Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SemanticallyEqual should allow for different lambda parameter names #4494

Merged
merged 2 commits into from
Sep 15, 2024
Merged

SemanticallyEqual should allow for different lambda parameter names #4494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0c4dab
Fix minor test bug
knutwannheden Sep 15, 2024
9aad528
`SemanticallyEqual` should allow for different lambda parameter names
knutwannheden Sep 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions rewrite-java-test/src/test/java/org/openrewrite/java/search/SemanticallyEqualTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,28 @@ class T {
);
}

@Test
void lambdaParameterNames() {
assertExpressionsEqual(
"""
import java.util.Comparator;
class T {
Comparator<Integer> a = (x1, y1) -> x1 - y1;
Comparator<Integer> b = (x2, y2) -> x2 - y2;
}
"""
);
assertExpressionsNotEqual(
"""
import java.util.Comparator;
class T {
Comparator<Integer> a = (x1, y1) -> x1 - y1;
Comparator<Integer> b = (x2, y2) -> y2 - x2;
}
"""
);
}

@Nested
class Generics {
@Test
Expand Down
8 changes: 4 additions & 4 deletions rewrite-java-test/src/test/java/org/openrewrite/java/service/AnnotationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void classAnnotations() {
java(
"""
import javax.annotation.processing.Generated;

@SuppressWarnings("all")
public @Generated("foo") class T {}
""",
Expand Down Expand Up @@ -68,7 +68,7 @@ void annotatedType() {
import java.lang.annotation.*;

import static java.lang.annotation.ElementType.*;

class T {
public @A1 Integer @A2 [] arg;
}
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 10 additions & 6 deletions rewrite-java/src/main/java/org/openrewrite/java/JavaTemplateSemanticallyEqual.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
53 changes: 46 additions & 7 deletions rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -59,6 +57,8 @@ protected static class SemanticallyEqualVisitor extends JavaIsoVisitor<J> {
private final boolean compareMethodArguments;

protected final AtomicBoolean isEqual = new AtomicBoolean(true);
private final Deque<Map<String, String>> variableScope = new ArrayDeque<>();
private final Set<JavaType> seen = new HashSet<>();

public SemanticallyEqualVisitor(boolean compareMethodArguments) {
this.compareMethodArguments = compareMethodArguments;
Expand Down Expand Up @@ -120,6 +120,29 @@ protected void visitList(@Nullable List<? extends J> list1, @Nullable List<? ext
return (J) tree;
}

@Override
public @Nullable J preVisit(J tree, J j) {
if (declaresVariableScope(tree)) {
variableScope.push(new HashMap<>());
}
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()) {
Expand Down Expand Up @@ -683,6 +706,16 @@ public J.Identifier visitIdentifier(J.Identifier identifier, J j) {
}

J.Identifier compareTo = (J.Identifier) j;
if (identifier.getFieldType() != null) {
Map<String, String> 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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<String, String> 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;
Expand Down
Loading