Add import should not remove preceding comment

moderne-meeseeks[bot] commented 1 year ago

Example diff

From: src/main/java/org/apache/maven/report/projectinfo/MailingListsReport.java

 package org.apache.maven.report.projectinfo;

-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
+import org.apache.commons.lang3.StringUtils;
import org.apache.maven.doxia.sink.Sink;
import org.apache.maven.model.MailingList;
import org.apache.maven.model.Model;
import org.apache.maven.plugin.logging.Log;
import org.apache.maven.plugins.annotations.Mojo;
import org.codehaus.plexus.i18n.I18N;
-import org.codehaus.plexus.util.StringUtils;

import java.net.URI;
import java.util.ArrayList;

Recipes in example diff:

org.openrewrite.java.ChangeMethodTargetToStatic

References:

View original result
Recipe ID: org.openrewrite.java.ChangeMethodTargetToStatic
Recipe Name: Change method target to static
Repository: apache/maven-project-info-reports-plugin/master
Created at Mon Apr 03 2023 21:38:35 GMT+0200 (Central European Summer Time)

This issue originally created by @timtebeek on: https://github.com/moderneinc/support-public/issues/37

moderne-meeseeks[bot] commented 1 year ago

Also seen a variant where a whitespace prefix is lost; expect that to be a similar case as this one.

Example diff

From: src/main/java/org/apache/maven/plugins/jdeprscan/AbstractJDeprScanMojo.java

 import java.util.List;
import java.util.Map;
import java.util.Properties;
-import java.util.StringTokenizer;
-
+import java.util.StringTokenizer;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.AbstractMojo;

 import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.toolchain.Toolchain;
import org.apache.maven.toolchain.ToolchainManager;
-import org.codehaus.plexus.util.StringUtils;
import org.codehaus.plexus.util.cli.CommandLineException;
import org.codehaus.plexus.util.cli.CommandLineUtils;
import org.codehaus.plexus.util.cli.Commandline;

Recipes in example diff:

org.openrewrite.java.ChangeMethodTargetToStatic

References:

View original result
Recipe ID: org.openrewrite.java.ChangeMethodTargetToStatic
Recipe Name: Change method target to static
Repository: apache/maven-jdeprscan-plugin/master
Created at Mon Apr 03 2023 21:43:22 GMT+0200 (Central European Summer Time)

This comment originially written by @timtebeek on: https://github.com/moderneinc/support-public/issues/37#issuecomment-1494879668

knutwannheden commented 1 year ago

I was trying to create a test for this and I ended up with something which I think is even weirder: maybeRemoveImport() removes the import and "reattaches" its comment to the next import and then maybeAddImport() adds a new import and it receives the same comment, so that in the end the comment is duplicated rather than lost.

openrewrite / rewrite