diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/search/SemanticallyEqualTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/search/SemanticallyEqualTest.java new file mode 100644 index 0000000000..a39115aaa7 --- /dev/null +++ b/rewrite-groovy/src/test/java/org/openrewrite/groovy/search/SemanticallyEqualTest.java @@ -0,0 +1,74 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * 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 + *

+ * https://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. + */ +package org.openrewrite.groovy.search; + +import org.junit.jupiter.api.Test; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.Issue; +import org.openrewrite.groovy.GroovyParser; +import org.openrewrite.groovy.tree.G; +import org.openrewrite.java.search.SemanticallyEqual; +import org.openrewrite.java.tree.J; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The Groovy parser does not attribute {@link J.Identifier#getFieldType()} on references, so these cases exercise + * what {@code SemanticallyEqual} may conclude when type information is systemically absent rather than merely missing. + */ +class SemanticallyEqualTest { + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/953") + @Test + void fieldNotEqualToShadowedNameVariable() { + J.Assignment assignment = firstAssignment(""" + class A { + private final boolean readOnly + A(boolean readOnly) { + this.readOnly = readOnly + } + } + """); + assertThat(SemanticallyEqual.areEqual(assignment.getVariable(), assignment.getAssignment())).isFalse(); + assertThat(SemanticallyEqual.areEqual(assignment.getAssignment(), assignment.getVariable())).isFalse(); + } + + @Test + void identifiersWithSameNameRemainEqual() { + J.Assignment assignment = firstAssignment(""" + class A { + void m(boolean readOnly) { + readOnly = readOnly + } + } + """); + assertThat(SemanticallyEqual.areEqual(assignment.getVariable(), assignment.getAssignment())).isTrue(); + } + + private J.Assignment firstAssignment(String source) { + G.CompilationUnit cu = GroovyParser.builder().build() + .parse(new InMemoryExecutionContext(), source) + .findFirst() + .map(G.CompilationUnit.class::cast) + .orElseThrow(); + J.ClassDeclaration clazz = (J.ClassDeclaration) cu.getStatements().getFirst(); + J.MethodDeclaration method = (J.MethodDeclaration) clazz.getBody().getStatements().stream() + .filter(J.MethodDeclaration.class::isInstance) + .findFirst() + .orElseThrow(); + return (J.Assignment) method.getBody().getStatements().getFirst(); + } +} 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 7ddb517749..ad6ba7e0cf 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 @@ -20,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.junitpioneer.jupiter.ExpectedToFail; import org.junitpioneer.jupiter.cartesian.CartesianTest; +import org.openrewrite.Issue; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaParser; import org.openrewrite.java.tree.J; @@ -436,6 +437,33 @@ void m(int a) { assertFalse(SemanticallyEqual.areEqual(((J.FieldAccess) a.getVariable()).getName(), a.getAssignment())); } + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/953") + @Test + void fieldNotEqualToShadowedNameVariableWithoutTypeAttribution() { + J.CompilationUnit cu = javaParser.parse( + """ + class T { + int a = 1; + void m(int a) { + this.a = a; + } + } + """ + ).findFirst() + .map(J.CompilationUnit.class::cast) + .get(); + J.CompilationUnit unattributed = (J.CompilationUnit) new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, Integer p) { + return identifier.withFieldType(null).withType(null); + } + }.visitNonNull(cu, 0); + J.MethodDeclaration m = (J.MethodDeclaration) unattributed.getClasses().getFirst().getBody().getStatements().get(1); + J.Assignment a = (J.Assignment) m.getBody().getStatements().getFirst(); + assertFalse(SemanticallyEqual.areEqual(a.getVariable(), a.getAssignment())); + assertFalse(SemanticallyEqual.areEqual(a.getAssignment(), a.getVariable())); + } + @Test void fieldsFromDifferentInstances() { J.CompilationUnit cu = javaParser.parse( 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 27452c2a55..53adc03166 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 @@ -608,10 +608,14 @@ public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, J j) { !isOfType(fieldType, ((J.Identifier) j).getFieldType()) || !fieldAccess.getSimpleName().equals(((J.Identifier) j).getSimpleName())) { isEqual.set(false); - } else { - if (fieldType != null && !fieldType.hasFlags(Flag.Static)) { + } else if (fieldType == null) { + // A null field type is either a type reference (`java.util.regex.Pattern` vs `Pattern`) + // or a variable whose type attribution is missing; only the former is safe to compare by name. + if (!isTypeReference(fieldAccess.getName()) || !isTypeReference((J.Identifier) j)) { isEqual.set(false); } + } else if (!fieldType.hasFlags(Flag.Static)) { + isEqual.set(false); } return fieldAccess; } @@ -710,6 +714,10 @@ public J.Identifier visitIdentifier(J.Identifier identifier, J j) { // Consider implicit-this "a" equivalent to "this.a" // Definitely does not account for every possible edge case around "this", but handles the common case if (fieldTarget instanceof J.Identifier && "this".equals(((J.Identifier) fieldTarget).getSimpleName())) { + if (identifier.getFieldType() == null || field.getName().getFieldType() == null) { + isEqual.set(false); + return identifier; + } visit(identifier, field.getName()); return identifier; } @@ -1561,6 +1569,12 @@ public N visitTypeName(N firstTypeName, J j) { return firstTypeName; } + private static boolean isTypeReference(J.Identifier identifier) { + return identifier.getFieldType() == null && + identifier.getType() instanceof JavaType.FullyQualified && + !(identifier.getType() instanceof JavaType.Unknown); + } + protected boolean isOfType(JavaType target, JavaType source) { return TypeUtils.isOfType(target, source); }