diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java index 943ecf00b16..6b8d5b62381 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitorTest.java @@ -22,6 +22,7 @@ import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.RewriteTest.toRecipe; @@ -61,6 +62,21 @@ public class A { ); } + @DocumentExample + @Test + void foo() { + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()), + java( + """ + public class A { + boolean a = null instanceof Unknown || null instanceof java.lang.Unknown; + } + """ + ) + ); + } + @Test void simplifyEqualsLiteralFalseIf() { rewriteRun( 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 5d0697b676c..6260ae2ec1a 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 @@ -681,42 +681,49 @@ public J.ForLoop.Control visitForControl(J.ForLoop.Control control, J j) { @Override public J.Identifier visitIdentifier(J.Identifier identifier, J j) { if (isEqual.get()) { - Map scope = variableScope.peek(); if (j instanceof J.FieldAccess) { J.FieldAccess field = (J.FieldAccess) j; Expression fieldTarget = field.getTarget(); // 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 (fieldTarget instanceof J.Identifier && "this".equals(((J.Identifier) fieldTarget).getSimpleName())) { visit(identifier, field.getName()); return identifier; } // Identifier name and type aren't the same as the field access they are obviously different - if(!identifier.getSimpleName().equals(field.getSimpleName()) || !TypeUtils.isOfType(identifier.getFieldType(), field.getName().getFieldType())) { + if (!identifier.getSimpleName().equals(field.getSimpleName()) || !TypeUtils.isOfType(identifier.getFieldType(), field.getName().getFieldType())) { + isEqual.set(false); + return identifier; + } + + // Either both are variables or both are not (e.g. types) + if ((identifier.getFieldType() == null) ^ (field.getName().getFieldType() == null)) { isEqual.set(false); return identifier; } - String identifierTypeFqn = identifier.getType() instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified)identifier.getType()).getFullyQualifiedName() : ""; + String identifierTypeFqn = identifier.getType() instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified) identifier.getType()).getFullyQualifiedName() : ""; // If the field access is a static field access, the identifier must be a class reference // e.g.: Pattern is semantically equal to java.util.regex.Pattern if (field.isFullyQualifiedClassReference(identifierTypeFqn)) { return identifier; } - // Similarly, might be comparing a statically imported field to a fully qualified - // e.g. java.util.regex.Pattern.CASE_INSENSITIVE is semantically equal to CASE_INSENSITIVE when the latter is a static import of the former - JavaType identifierOwner = identifier.getFieldType().getOwner(); - String identifierOwnerFqn = identifierOwner instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified) identifierOwner).getFullyQualifiedName() : ""; - if(field.getTarget() instanceof J.FieldAccess && ((J.FieldAccess) field.getTarget()).isFullyQualifiedClassReference(identifierOwnerFqn)) { - return identifier; - } + if (identifier.getFieldType() != null) { + // Similarly, might be comparing a statically imported field to a fully qualified + // e.g. java.util.regex.Pattern.CASE_INSENSITIVE is semantically equal to CASE_INSENSITIVE when the latter is a static import of the former + JavaType identifierOwner = identifier.getFieldType().getOwner(); + String identifierOwnerFqn = identifierOwner instanceof JavaType.FullyQualified ? ((JavaType.FullyQualified) identifierOwner).getFullyQualifiedName() : ""; + if (field.getTarget() instanceof J.FieldAccess && ((J.FieldAccess) field.getTarget()).isFullyQualifiedClassReference(identifierOwnerFqn)) { + return identifier; + } - // Identifier is statically imported field name "CASE_INSENSITIVE", field is field access "Pattern.CASE_INSENSITIVE" - if (identifierOwner instanceof JavaType.FullyQualified && ((JavaType.FullyQualified) identifierOwner).getClassName().equals(fieldTarget.toString()) && TypeUtils.isOfType(identifier.getFieldType().getOwner(), fieldTarget.getType())) { - return identifier; + // Identifier is statically imported field name "CASE_INSENSITIVE", field is field access "Pattern.CASE_INSENSITIVE" + if (identifierOwner instanceof JavaType.FullyQualified && ((JavaType.FullyQualified) identifierOwner).getClassName().equals(fieldTarget.toString()) && TypeUtils.isOfType(identifier.getFieldType().getOwner(), fieldTarget.getType())) { + return identifier; + } } isEqual.set(false); @@ -729,6 +736,7 @@ 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; } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java index 070d38ea124..7afe8698797 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/tree/J.java @@ -1979,21 +1979,26 @@ public List getSideEffects() { } public boolean isFullyQualifiedClassReference(String className) { - return isFullyQualifiedClassReference(this, className); + if (!className.contains(".")) { + return false; + } + return isFullyQualifiedClassReference(this, className, className.length()); } - private boolean isFullyQualifiedClassReference(J.FieldAccess fieldAccess, String className) { - if (!className.contains(".")) { + private boolean isFullyQualifiedClassReference(J.FieldAccess fieldAccess, String className, int prevDotIndex) { + int dotIndex = className.lastIndexOf('.', prevDotIndex - 1); + if (dotIndex < 0) { return false; } - if (!fieldAccess.getName().getSimpleName().equals(className.substring(className.lastIndexOf('.') + 1))) { + String simpleName = fieldAccess.getName().getSimpleName(); + if (!simpleName.regionMatches(0, className, dotIndex + 1, simpleName.length())) { return false; } if (fieldAccess.getTarget() instanceof J.FieldAccess) { - return isFullyQualifiedClassReference((J.FieldAccess) fieldAccess.getTarget(), className.substring(0, className.lastIndexOf('.'))); + return isFullyQualifiedClassReference((J.FieldAccess) fieldAccess.getTarget(), className, dotIndex); } if (fieldAccess.getTarget() instanceof Identifier) { - return ((Identifier) fieldAccess.getTarget()).getSimpleName().equals(className.substring(0, className.lastIndexOf('.'))); + return ((Identifier) fieldAccess.getTarget()).getSimpleName().equals(className.substring(0, dotIndex)); } return false; }