Merge pull request #24 from Azure/no-comment-repetition

KaiqianYang · web-flow · commit 9e445dfe0088 · 2023-03-28T16:15:26.000+08:00
fix issue of adding repetitive comment when run recipe multiple times
diff --git a/src/main/java/com/azure/spring/migration/openrewrite/java/search/AddComment.java b/src/main/java/com/azure/spring/migration/openrewrite/java/search/AddComment.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2023 the original author or authors.
+ * <p>
+ * Licensed 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
+ * <p>
+ * https://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.
+ */
+
+package com.azure.spring.migration.openrewrite.java.search;
+
+import java.util.List;
+import java.util.Objects;
+import org.openrewrite.internal.lang.Nullable;
+
+import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.TextComment;
+import org.openrewrite.marker.SearchResult;
+
+import static org.openrewrite.Tree.randomId;
+
+public class AddComment {
+    public static <T extends J> T addIfAbsent(@Nullable T t, String description) {
+        if (t == null) {
+            //noinspection ConstantConditions
+            return null;
+        }
+        List<SearchResult> existingSearchResult = t.getMarkers().findAll(SearchResult.class);
+        if (existingSearchResult.stream().anyMatch(sr -> Objects.equals(sr.getDescription(), description))) {
+            return t;
+        }
+        if (t.getPrefix().getComments().stream().anyMatch(comment -> comment instanceof TextComment && ((TextComment) comment).getText().contains(description))) {
+            return t;
+        }
+        return t.withMarkers(t.getMarkers().computeByType(new SearchResult(randomId(), description), (s1, s2) -> s1 == null ? s2 : s1));
+    }
+
+}
diff --git a/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindLiterals.java b/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindLiterals.java
@@ -29,7 +29,6 @@
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaType.Primitive;
-import org.openrewrite.marker.SearchResult;
 
 @EqualsAndHashCode(callSuper = true)
 @Value
@@ -69,10 +68,10 @@ public FindLiterals(final String pattern, @Nullable final String mark) {
             public J. @NonNull Literal visitLiteral(J. @NonNull Literal literal, @NonNull ExecutionContext ctx) {
                 if (literal.getValueSource() != null) {
                     if (literal.getType() == Primitive.String && compiledPattern.matcher(literal.getValueSource().substring(1, literal.getValueSource().length() - 1)).matches()) {
-                        return SearchResult.found(literal,mark);
+                        return AddComment.addIfAbsent(literal,mark);
                     }
                     if (compiledPattern.matcher(literal.getValueSource()).matches()) {
-                        return SearchResult.found(literal,mark);
+                        return AddComment.addIfAbsent(literal,mark);
                     }
                 }
                 return literal;
diff --git a/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindMethods.java b/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindMethods.java
@@ -31,7 +31,6 @@
 import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaSourceFile;
-import org.openrewrite.marker.SearchResult;
 
 /**
  * Finds matching method invocations.
@@ -107,7 +106,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver
                                 method.printTrimmed(getCursor())
                             ));
                         }
-                        m = SearchResult.found(m, mark);
+                        m = AddComment.addIfAbsent(m, mark);
                     } else {
                         doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(method)));
                     }
@@ -127,7 +126,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver
                                 memberRef.printTrimmed(getCursor())
                             ));
                         }
-                        m = m.withReference(SearchResult.found(m.getReference(), mark));
+                        m = m.withReference(AddComment.addIfAbsent(m.getReference(), mark));
                     } else {
                         doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(memberRef)));
                     }
@@ -147,7 +146,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver
                                 newClass.printTrimmed(getCursor())
                             ));
                         }
-                        n = SearchResult.found(n, mark);
+                        n = AddComment.addIfAbsent(n, mark);
                     } else {
                         doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(newClass)));
                     }
diff --git a/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindTypes.java b/src/main/java/com/azure/spring/migration/openrewrite/java/search/FindTypes.java
@@ -30,7 +30,6 @@
 import org.openrewrite.java.tree.JavaType;
 import org.openrewrite.java.tree.NameTree;
 import org.openrewrite.java.tree.TypeUtils;
-import org.openrewrite.marker.SearchResult;
 
 import lombok.EqualsAndHashCode;
 
@@ -85,7 +84,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch
                     JavaType.FullyQualified type = TypeUtils.asFullyQualified(ident.getType());
                     if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&
                         ident.getSimpleName().equals(type.getClassName())) {
-                        return SearchResult.found(ident, mark);
+                        return AddComment.addIfAbsent(ident, mark);
                     }
                 }
                 return super.visitIdentifier(ident, executionContext);
@@ -97,7 +96,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch
                 JavaType.FullyQualified type = TypeUtils.asFullyQualified(n.getType());
                 if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&
                     getCursor().firstEnclosing(J.Import.class) == null) {
-                    return SearchResult.found(n, mark);
+                    return AddComment.addIfAbsent(n, mark);
                 }
                 return n;
             }
@@ -108,7 +107,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch
                 JavaType.FullyQualified type = TypeUtils.asFullyQualified(fa.getTarget().getType());
                 if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&
                     fa.getName().getSimpleName().equals("class")) {
-                    return SearchResult.found(fa, mark);
+                    return AddComment.addIfAbsent(fa, mark);
                 }
                 return fa;
             }
diff --git a/src/main/java/com/azure/spring/migration/openrewrite/yaml/FindProperty.java b/src/main/java/com/azure/spring/migration/openrewrite/yaml/FindProperty.java
@@ -61,8 +61,9 @@ public class FindProperty extends Recipe {
     }
 
     String HASH_TAG = "#";
-    String wrapComment(String commentText, String indent) {
-        return HASH_TAG + commentText + indent;
+    String LINE_BREAK = "\n";
+    String wrapComment(String commentText, String prefix) {
+        return prefix + HASH_TAG + commentText + (prefix.contains(LINE_BREAK) ? prefix : LINE_BREAK + prefix);
     }
 
     @Override
@@ -76,9 +77,9 @@ String wrapComment(String commentText, String indent) {
                 if (!Boolean.FALSE.equals(relaxedBinding) ?
                     NameCaseConvention.matchesGlobRelaxedBinding(prop, propertyKey) :
                     StringUtils.matchesGlob(prop, propertyKey)) {
-                    String wrappedComment = wrapComment(commentText, e.getPrefix());
-                    if (!e.getKey().getPrefix().startsWith(wrappedComment)) {
-                        e = e.withKey(e.getKey().withPrefix(wrappedComment));
+                    if (!e.getPrefix().contains(commentText) && !e.getKey().getPrefix().contains(commentText)) {
+                        String wrappedComment = wrapComment(commentText, e.getPrefix());
+                        e = e.withPrefix(wrappedComment);
                     }
                 }
                 return e;
diff --git a/src/test/java/com/azure/spring/migration/openrewrite/java/search/FindMethodsTest.java b/src/test/java/com/azure/spring/migration/openrewrite/java/search/FindMethodsTest.java
@@ -22,6 +22,7 @@
 import org.openrewrite.test.RewriteTest;
 
 public final class FindMethodsTest implements RewriteTest {
+
     @Test
     void testFindMethods() {
         rewriteRun(
diff --git a/src/test/java/com/azure/spring/migration/openrewrite/yaml/FindPropertyTest.java b/src/test/java/com/azure/spring/migration/openrewrite/yaml/FindPropertyTest.java
@@ -28,13 +28,16 @@ void testFindPassword() {
             spec -> spec.recipe(new FindProperty("*password","TODO ASA-FindPassword: Don't save passwords or login information in files",true)),
             yaml(
                 """
+                        password: test
                         application:
                           Password: 1111
                         application:
                           password:
                             test: 111
                       """,
                 """
+                    #TODO ASA-FindPassword: Don't save passwords or login information in files
+                    password: test
                     application:
                       #TODO ASA-FindPassword: Don't save passwords or login information in files
                       Password: 1111

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,6 @@`
`29`	`29`	`import org.openrewrite.java.JavaVisitor;`
`30`	`30`	`import org.openrewrite.java.tree.J;`
`31`	`31`	`import org.openrewrite.java.tree.JavaType.Primitive;`
`32`		`-import org.openrewrite.marker.SearchResult;`
`33`	`32`
`34`	`33`	`@EqualsAndHashCode(callSuper = true)`
`35`	`34`	`@Value`
`@@ -69,10 +68,10 @@ public FindLiterals(final String pattern, @Nullable final String mark) {`
`69`	`68`	`public J. @NonNull Literal visitLiteral(J. @NonNull Literal literal, @NonNull ExecutionContext ctx) {`
`70`	`69`	`if (literal.getValueSource() != null) {`
`71`	`70`	`if (literal.getType() == Primitive.String && compiledPattern.matcher(literal.getValueSource().substring(1, literal.getValueSource().length() - 1)).matches()) {`
`72`		`- return SearchResult.found(literal,mark);`
	`71`	`+ return AddComment.addIfAbsent(literal,mark);`
`73`	`72`	`}`
`74`	`73`	`if (compiledPattern.matcher(literal.getValueSource()).matches()) {`
`75`		`- return SearchResult.found(literal,mark);`
	`74`	`+ return AddComment.addIfAbsent(literal,mark);`
`76`	`75`	`}`
`77`	`76`	`}`
`78`	`77`	`return literal;`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@`
`31`	`31`	`import org.openrewrite.java.tree.Expression;`
`32`	`32`	`import org.openrewrite.java.tree.J;`
`33`	`33`	`import org.openrewrite.java.tree.JavaSourceFile;`
`34`		`-import org.openrewrite.marker.SearchResult;`
`35`	`34`
`36`	`35`	`/**`
`37`	`36`	`* Finds matching method invocations.`
`@@ -107,7 +106,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver`
`107`	`106`	`method.printTrimmed(getCursor())`
`108`	`107`	`));`
`109`	`108`	`}`
`110`		`- m = SearchResult.found(m, mark);`
	`109`	`+ m = AddComment.addIfAbsent(m, mark);`
`111`	`110`	`} else {`
`112`	`111`	`doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(method)));`
`113`	`112`	`}`
`@@ -127,7 +126,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver`
`127`	`126`	`memberRef.printTrimmed(getCursor())`
`128`	`127`	`));`
`129`	`128`	`}`
`130`		`- m = m.withReference(SearchResult.found(m.getReference(), mark));`
	`129`	`+ m = m.withReference(AddComment.addIfAbsent(m.getReference(), mark));`
`131`	`130`	`} else {`
`132`	`131`	`doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(memberRef)));`
`133`	`132`	`}`
`@@ -147,7 +146,7 @@ public FindMethods(final String methodPattern, @Nullable final Boolean matchOver`
`147`	`146`	`newClass.printTrimmed(getCursor())`
`148`	`147`	`));`
`149`	`148`	`}`
`150`		`- n = SearchResult.found(n, mark);`
	`149`	`+ n = AddComment.addIfAbsent(n, mark);`
`151`	`150`	`} else {`
`152`	`151`	`doAfterVisit(new FindLocalFlowPaths<>(getFlowSpec(newClass)));`
`153`	`152`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,6 @@`
`30`	`30`	`import org.openrewrite.java.tree.JavaType;`
`31`	`31`	`import org.openrewrite.java.tree.NameTree;`
`32`	`32`	`import org.openrewrite.java.tree.TypeUtils;`
`33`		`-import org.openrewrite.marker.SearchResult;`
`34`	`33`
`35`	`34`	`import lombok.EqualsAndHashCode;`
`36`	`35`
`@@ -85,7 +84,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch`
`85`	`84`	`JavaType.FullyQualified type = TypeUtils.asFullyQualified(ident.getType());`
`86`	`85`	`if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&`
`87`	`86`	`ident.getSimpleName().equals(type.getClassName())) {`
`88`		`- return SearchResult.found(ident, mark);`
	`87`	`+ return AddComment.addIfAbsent(ident, mark);`
`89`	`88`	`}`
`90`	`89`	`}`
`91`	`90`	`return super.visitIdentifier(ident, executionContext);`
`@@ -97,7 +96,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch`
`97`	`96`	`JavaType.FullyQualified type = TypeUtils.asFullyQualified(n.getType());`
`98`	`97`	`if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&`
`99`	`98`	`getCursor().firstEnclosing(J.Import.class) == null) {`
`100`		`- return SearchResult.found(n, mark);`
	`99`	`+ return AddComment.addIfAbsent(n, mark);`
`101`	`100`	`}`
`102`	`101`	`return n;`
`103`	`102`	`}`
`@@ -108,7 +107,7 @@ public FindTypes(final String fullyQualifiedTypeName, @Nullable final Boolean ch`
`108`	`107`	`JavaType.FullyQualified type = TypeUtils.asFullyQualified(fa.getTarget().getType());`
`109`	`108`	`if (typeMatches(Boolean.TRUE.equals(checkAssignability), fullyQualifiedType, type) &&`
`110`	`109`	`fa.getName().getSimpleName().equals("class")) {`
`111`		`- return SearchResult.found(fa, mark);`
	`110`	`+ return AddComment.addIfAbsent(fa, mark);`
`112`	`111`	`}`
`113`	`112`	`return fa;`
`114`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,9 @@ public class FindProperty extends Recipe {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`String HASH_TAG = "#";`
`64`		`- String wrapComment(String commentText, String indent) {`
`65`		`- return HASH_TAG + commentText + indent;`
	`64`	`+ String LINE_BREAK = "\n";`
	`65`	`+ String wrapComment(String commentText, String prefix) {`
	`66`	`+ return prefix + HASH_TAG + commentText + (prefix.contains(LINE_BREAK) ? prefix : LINE_BREAK + prefix);`
`66`	`67`	`}`
`67`	`68`
`68`	`69`	`@Override`
`@@ -76,9 +77,9 @@ String wrapComment(String commentText, String indent) {`
`76`	`77`	`if (!Boolean.FALSE.equals(relaxedBinding) ?`
`77`	`78`	`NameCaseConvention.matchesGlobRelaxedBinding(prop, propertyKey) :`
`78`	`79`	`StringUtils.matchesGlob(prop, propertyKey)) {`
`79`		`- String wrappedComment = wrapComment(commentText, e.getPrefix());`
`80`		`- if (!e.getKey().getPrefix().startsWith(wrappedComment)) {`
`81`		`- e = e.withKey(e.getKey().withPrefix(wrappedComment));`
	`80`	`+ if (!e.getPrefix().contains(commentText) && !e.getKey().getPrefix().contains(commentText)) {`
	`81`	`+ String wrappedComment = wrapComment(commentText, e.getPrefix());`
	`82`	`+ e = e.withPrefix(wrappedComment);`
`82`	`83`	`}`
`83`	`84`	`}`
`84`	`85`	`return e;`