Fix SemanticallyEqual, and by extension SimplifyBooleanExpression, co…

…nsidering identifiers to be equal to unrelated field accesses

rewrite-groovy/src/test/java/org/openrewrite/groovy/cleanup/SimplifyBooleanExpressionVisitorTest.java

@@ -414,6 +414,23 @@ def m() {
         );
     }
 
+    @Test
+    void mapFields() {
+        rewriteRun(
+          groovy(
+            """
+              class A {
+                  def foo(boolean a, Map m) {
+                      if (a || m.foo || m.bar) {
+                          System.out.println("")
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @ParameterizedTest
     @Issue("https://github.com/openrewrite/rewrite-templating/issues/28")
     // Mimic what would be inserted by a Refaster template using two nullable parameters, with the second one a literal
@@ -468,7 +485,6 @@ def m() {
       a == null || !a.isEmpty() && "b" != null && !"b".isEmpty() // a == null || !a.isEmpty()
       """)
     void simplifyLiteralNull(String before, String after) {
-        //language=java
         String template = """
           class A {
               void foo(String a) {

rewrite-java/src/main/java/org/openrewrite/java/search/SemanticallyEqual.java

@@ -41,24 +41,12 @@ public static boolean areEqual(J firstElem, J secondElem) {
         return semanticallyEqualVisitor.isEqual();
     }
 
-    /**
-     * Compares method invocations and new class constructors based on the `JavaType.Method` instead of checking
-     * each types of each parameter.
-     * I.E. void foo(Object obj) {} invoked with `java.lang.String` or `java.lang.Integer` will return true.
-     */
-    public static boolean areSemanticallyEqual(J firstElem, J secondElem) {
-        SemanticallyEqualVisitor semanticallyEqualVisitor = new SemanticallyEqualVisitor(false);
-        semanticallyEqualVisitor.visit(firstElem, secondElem);
-        return semanticallyEqualVisitor.isEqual.get();
-    }
-
     @SuppressWarnings("ConstantConditions")
     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;
@@ -137,10 +125,7 @@ protected void visitList(@Nullable List<? extends J> list1, @Nullable List<? ext
         }
 
         protected boolean declaresVariableScope(J tree) {
-            if (tree instanceof J.Lambda) {
-                return true;
-            }
-            return false;
+            return tree instanceof J.Lambda;
         }
 
         @Override
@@ -696,18 +681,54 @@ public J.ForLoop.Control visitForControl(J.ForLoop.Control control, J j) {
         @Override
         public J.Identifier visitIdentifier(J.Identifier identifier, J j) {
             if (isEqual.get()) {
-                if (!(j instanceof J.Identifier)) {
-                    if (!(j instanceof J.FieldAccess) || !TypeUtils.isOfType(identifier.getFieldType(), ((J.FieldAccess) j).getName().getFieldType())) {
-                        isEqual.set(false);
-                    } else if (identifier.getFieldType() != null && !identifier.getFieldType().hasFlags(Flag.Static)) {
+                Map<String, String> 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())) {
+                        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())) {
                         isEqual.set(false);
+                        return identifier;
                     }
+
+                    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;
+                    }
+
+                    // 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);
+                    return identifier;
+                }
+                if (!(j instanceof J.Identifier)) {
+                    isEqual.set(false);
                     return identifier;
                 }
 
                 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;
                     }
@@ -856,8 +877,7 @@ public J.MemberReference visitMemberReference(J.MemberReference memberRef, J j)
                     return memberRef;
                 }
 
-                visit(memberRef.getContaining(), compareTo.getContaining());
-                this.visitList(memberRef.getTypeParameters(), compareTo.getTypeParameters());
+                visitList(memberRef.getTypeParameters(), compareTo.getTypeParameters());
             }
             return memberRef;
         }