From b0c4dabe4800e0e5cf3d3a5ac15d6aa8a5bc0f81 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Sun, 15 Sep 2024 09:02:23 +0200 Subject: [PATCH 1/2] Fix minor test bug Caused a minor test attribution error. --- .../openrewrite/java/service/AnnotationServiceTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/service/AnnotationServiceTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/service/AnnotationServiceTest.java index 0bcc10a3d1d..4c7896dbce1 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/service/AnnotationServiceTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/service/AnnotationServiceTest.java @@ -34,7 +34,7 @@ void classAnnotations() { java( """ import javax.annotation.processing.Generated; - + @SuppressWarnings("all") public @Generated("foo") class T {} """, @@ -68,7 +68,7 @@ void annotatedType() { import java.lang.annotation.*; import static java.lang.annotation.ElementType.*; - + class T { public @A1 Integer @A2 [] arg; } @@ -159,9 +159,9 @@ void fieldAccessAnnotations() { import java.lang.annotation.*; import static java.lang.annotation.ElementType.*; - + class T { - java. lang. @Ann Map arg; + java. lang. @Ann Integer arg; } @Retention(RetentionPolicy.RUNTIME) From 9aad528c4c4b812b3a040548d651d003aa90c146 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Sun, 15 Sep 2024 09:40:18 +0200 Subject: [PATCH 2/2] `SemanticallyEqual` should allow for different lambda parameter names While comparing two lambdas for semantical equality, the lambda parameter names should be ignored. This actually also applies to other variables (e.g. variables declared in a block such as a lambda body), but that is out of scope for this PR. This will allow Refaster recipes to match a lambda against other lambdas using different parameter names. --- .../java/search/SemanticallyEqualTest.java | 22 ++++++++ .../java/JavaTemplateSemanticallyEqual.java | 16 +++--- .../java/search/SemanticallyEqual.java | 53 ++++++++++++++++--- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/search/SemanticallyEqualTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/search/SemanticallyEqualTest.java index b4de19e17ba..0e4f98aa4f1 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/search/SemanticallyEqualTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/search/SemanticallyEqualTest.java @@ -299,6 +299,28 @@ class T { ); } + @Test + void lambdaParameterNames() { + assertExpressionsEqual( + """ + import java.util.Comparator; + class T { + Comparator a = (x1, y1) -> x1 - y1; + Comparator b = (x2, y2) -> x2 - y2; + } + """ + ); + assertExpressionsNotEqual( + """ + import java.util.Comparator; + class T { + Comparator a = (x1, y1) -> x1 - y1; + Comparator b = (x2, y2) -> y2 - x2; + } + """ + ); + } + @Nested class Generics { @Test diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaTemplateSemanticallyEqual.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaTemplateSemanticallyEqual.java index 2215355e8ce..4416c6c0a24 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/JavaTemplateSemanticallyEqual.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/JavaTemplateSemanticallyEqual.java @@ -73,18 +73,22 @@ private static J[] createTemplateParameters(String code) { substituted = propertyPlaceholderHelper.replacePlaceholders(substituted, key -> { String s; if (!key.isEmpty()) { - TemplateParameterParser parser = new TemplateParameterParser(new CommonTokenStream(new TemplateParameterLexer( - CharStreams.fromString(key)))); - - parser.removeErrorListeners(); - parser.addErrorListener(new BaseErrorListener() { + BaseErrorListener errorListener = new BaseErrorListener() { @Override public void syntaxError(Recognizer recognizer, Object offendingSymbol, int line, int charPositionInLine, String msg, RecognitionException e) { throw new IllegalArgumentException( String.format("Syntax error at line %d:%d %s.", line, charPositionInLine, msg), e); } - }); + }; + + TemplateParameterLexer lexer = new TemplateParameterLexer(CharStreams.fromString(key)); + lexer.removeErrorListeners(); + lexer.addErrorListener(errorListener); + + TemplateParameterParser parser = new TemplateParameterParser(new CommonTokenStream(lexer)); + parser.removeErrorListeners(); + parser.addErrorListener(errorListener); TemplateParameterParser.MatcherPatternContext ctx = parser.matcherPattern(); if (ctx.typedPattern() == null) { diff --git a/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java b/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java index e2631a72234..0bcf9b648c9 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java @@ -21,9 +21,7 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.tree.*; -import java.util.EnumSet; -import java.util.List; -import java.util.Objects; +import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -59,6 +57,8 @@ protected static class SemanticallyEqualVisitor extends JavaIsoVisitor { private final boolean compareMethodArguments; protected final AtomicBoolean isEqual = new AtomicBoolean(true); + private final Deque> variableScope = new ArrayDeque<>(); + private final Set seen = new HashSet<>(); public SemanticallyEqualVisitor(boolean compareMethodArguments) { this.compareMethodArguments = compareMethodArguments; @@ -120,6 +120,29 @@ protected void visitList(@Nullable List list1, @Nullable List()); + } + return tree; + } + + @Override + public @Nullable J postVisit(J tree, J j) { + if (declaresVariableScope(tree)) { + variableScope.pop(); + } + return tree; + } + + protected boolean declaresVariableScope(J tree) { + if (tree instanceof J.Lambda) { + return true; + } + return false; + } + @Override public Expression visitExpression(Expression expression, J j) { if (isEqual.get()) { @@ -683,6 +706,16 @@ public J.Identifier visitIdentifier(J.Identifier identifier, J j) { } J.Identifier compareTo = (J.Identifier) j; + if (identifier.getFieldType() != null) { + Map scope = variableScope.peek(); + if (scope != null && scope.containsKey(identifier.getSimpleName()) && scope.get(identifier.getSimpleName()).equals(compareTo.getSimpleName())) { + return identifier; + } + } + if (TypeUtils.isWellFormedType(identifier.getType(), seen) && !TypeUtils.isOfType(identifier.getType(), compareTo.getType())) { + isEqual.set(false); + return identifier; + } if (!identifier.getSimpleName().equals(compareTo.getSimpleName())) { isEqual.set(false); return identifier; @@ -784,8 +817,8 @@ public J.Lambda visitLambda(J.Lambda lambda, J j) { isEqual.set(false); return lambda; } + visitList(lambda.getParameters().getParameters(), compareTo.getParameters().getParameters()); visit(lambda.getBody(), compareTo.getBody()); - this.visitList(lambda.getParameters().getParameters(), compareTo.getParameters().getParameters()); } return lambda; } @@ -911,7 +944,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, J j) JavaType.FullyQualified methodDeclaringType = method.getMethodType().getDeclaringType(); JavaType.FullyQualified compareToDeclaringType = compareTo.getMethodType().getDeclaringType(); if (!TypeUtils.isAssignableTo(methodDeclaringType instanceof JavaType.Parameterized ? - ((JavaType.Parameterized) methodDeclaringType).getType() : methodDeclaringType, + ((JavaType.Parameterized) methodDeclaringType).getType() : methodDeclaringType, compareToDeclaringType instanceof JavaType.Parameterized ? ((JavaType.Parameterized) compareToDeclaringType).getType() : compareToDeclaringType)) { isEqual.set(false); @@ -1360,8 +1393,14 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations } J.VariableDeclarations.NamedVariable compareTo = (J.VariableDeclarations.NamedVariable) j; - if (!variable.getSimpleName().equals(compareTo.getSimpleName()) || - !TypeUtils.isOfType(variable.getType(), compareTo.getType()) || + Map scope = variableScope.peek(); + if (scope != null) { + scope.put(variable.getSimpleName(), compareTo.getSimpleName()); + } else if (!variable.getSimpleName().equals(compareTo.getSimpleName())) { + isEqual.set(false); + return variable; + } + if (!TypeUtils.isOfType(variable.getType(), compareTo.getType()) || nullMissMatch(variable.getInitializer(), compareTo.getInitializer())) { isEqual.set(false); return variable;