RenameVariable fixes for member references (#5406)

* fix

* Cleanup

* fixes

Author: fmodesto

rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateMatchTest.java

@@ -18,7 +18,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
-import org.junitpioneer.jupiter.ExpectedToFail;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Issue;
@@ -1002,20 +1001,19 @@ void test() {
     }
 
     @Test
-    @ExpectedToFail("PR #5374 was reverted, due to regressions")
     void matchMemberReferenceContainingParameter() {
         rewriteRun(
           spec -> spec
             .expectedCyclesThatMakeChanges(1).cycles(1)
             .recipe(toRecipe(() -> new JavaIsoVisitor<>() {
-                JavaTemplate template = JavaTemplate.builder("java.util.Optional.ofNullable(#{any(java.lang.String)}).orElseGet(#{any(java.lang.Object)}::toString)").build();
+                JavaTemplate template = JavaTemplate.builder("java.util.function.Predicate.not(#{any(java.util.Set)}::contains)").build();
 
                 @Override
                 public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
                     JavaTemplate.Matcher matcher = template.matcher(getCursor());
                     if (matcher.find()) {
                         JavaTemplateSemanticallyEqual.TemplateMatchResult result = matcher.getMatchResult();
-                        assertThat(result.getMatchedParameters()).hasSize(2);
+                        assertThat(result.getMatchedParameters()).hasSize(1);
                         return SearchResult.found(template.apply(getCursor(), method.getCoordinates().replace(), result.getMatchedParameters().toArray()));
                     }
                     return super.visitMethodInvocation(method, executionContext);
@@ -1024,22 +1022,24 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
           //language=java
           java(
             """
-              import java.util.Optional;
+              import java.util.function.Predicate;
+              import java.util.Set;
 
               class Foo {
-                  @SuppressWarnings("all")
-                  void test() {
-                      Optional.ofNullable("foo").orElseGet("bar"::toString);
+                  Predicate<Object> test() {
+                      Set<String> set = Set.of("1", "2");
+                      return Predicate.not(set::contains);
                   }
               }
               """,
             """
-              import java.util.Optional;
+              import java.util.function.Predicate;
+              import java.util.Set;
 
               class Foo {
-                  @SuppressWarnings("all")
-                  void test() {
-                      /*~~>*/java.util.Optional.ofNullable("foo").orElseGet("bar"::toString);
+                  Predicate<Object> test() {
+                      Set<String> set = Set.of("1", "2");
+                      return /*~~>*/java.util.function.Predicate.not(set::contains);
                   }
               }
               """

rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java

@@ -16,7 +16,6 @@
 package org.openrewrite.java;
 
 import org.junit.jupiter.api.Test;
-import org.junitpioneer.jupiter.ExpectedToFail;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Issue;
@@ -1037,7 +1036,6 @@ public A setN(int n1) {
     }
 
     @Test
-    @ExpectedToFail("PR #5372 was reverted, due to regressions")
     void hiddenVariablesHierarchyRenameBase() {
         rewriteRun(
           spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@@ -1133,7 +1131,6 @@ public Extended test(Base hidden) {
     }
 
     @Test
-    @ExpectedToFail("PR #5372 was reverted, due to regressions")
     void hiddenVariablesHierarchyRenameExtended() {
         rewriteRun(
           spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@@ -1229,7 +1226,6 @@ public Extended test(Base hidden) {
     }
 
     @Test
-    @ExpectedToFail("PR #5372 was reverted, due to regressions")
     void hiddenVariablesHierarchyRenameLocal() {
         rewriteRun(
           spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {

rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java

@@ -90,7 +90,12 @@ public J.Identifier visitIdentifier(J.Identifier ident, P p) {
             }
             Cursor parent = getCursor().getParentTreeCursor();
             if (ident.getSimpleName().equals(renameVariable.getSimpleName())) {
-                if (parent.getValue() instanceof J.FieldAccess) {
+                if (ident.getFieldType() != null && ident.getFieldType().getOwner() instanceof JavaType.FullyQualified &&
+                        TypeUtils.isOfType(ident.getFieldType(), renameVariable.getVariableType())) {
+                    parent.putMessage("renamed", true);
+                    return ident.withFieldType(ident.getFieldType().withName(newName)).withSimpleName(newName);
+                } else if (parent.getValue() instanceof J.FieldAccess &&
+                        !ident.equals(((J.FieldAccess) parent.getValue()).getTarget())) {
                     if (fieldAccessTargetsVariable(parent.getValue())) {
                         if (ident.getFieldType() != null) {
                             ident = ident.withFieldType(ident.getFieldType().withName(newName));
@@ -149,50 +154,18 @@ private boolean isVariableName(Object value, J.Identifier ident) {
         }
 
         /**
-         * FieldAccess targets the variable if its target is an Identifier and either
-         * its target FieldType equals variable.Name.FieldType
-         * or its target Type equals variable.Name.FieldType.Owner
-         * or if FieldAccess targets a TypCast and either
-         * its type equals variable.Name.FieldType
-         * or its type equals variable.Name.FieldType.Owner.
-         * In case the FieldAccess targets another FieldAccess, the target is followed
-         * until it is either an Identifier or a TypeCast.
+         * FieldAccess targets the variable if its target type equals variable.Name.FieldType.Owner.
          */
         private boolean fieldAccessTargetsVariable(J.FieldAccess fieldAccess) {
-            if (renameVariable.getName().getFieldType() != null) {
-                Expression target = getTarget(fieldAccess);
-                JavaType targetType = resolveType(target.getType());
+            if (renameVariable.getName().getFieldType() != null &&
+                    fieldAccess.getTarget().getType() != null) {
+                JavaType targetType = resolveType(fieldAccess.getTarget().getType());
                 JavaType.Variable variableNameFieldType = renameVariable.getName().getFieldType();
-                if (TypeUtils.isOfType(variableNameFieldType.getOwner(), targetType)) {
-                    return true;
-                }
-                if (target instanceof J.TypeCast) {
-                    return TypeUtils.isOfType(variableNameFieldType, targetType);
-                } else if (target instanceof J.Identifier) {
-                    return TypeUtils.isOfType(variableNameFieldType, ((J.Identifier) target).getFieldType());
-                }
+                return TypeUtils.isOfType(resolveType(variableNameFieldType.getOwner()), targetType);
             }
             return false;
         }
 
-        private @Nullable Expression getTarget(J.FieldAccess fieldAccess) {
-            Expression target = fieldAccess.getTarget();
-            if (target instanceof J.Identifier) {
-                return target;
-            }
-            if (target instanceof J.FieldAccess) {
-                return getTarget((J.FieldAccess) target);
-            }
-            if (target instanceof J.Parentheses<?>) {
-                J tree = ((J.Parentheses<?>) target).getTree();
-                if (tree instanceof J.TypeCast) {
-                    return (J.TypeCast) tree;
-                }
-                return null;
-            }
-            return null;
-        }
-
         private @Nullable JavaType resolveType(@Nullable JavaType type) {
             return type instanceof JavaType.Parameterized ? ((JavaType.Parameterized) type).getType() : type;
         }

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

@@ -932,6 +932,7 @@ public J.MemberReference visitMemberReference(J.MemberReference memberRef, J j)
                     return memberRef;
                 }
 
+                visit(memberRef.getContaining(), compareTo.getContaining());
                 visitList(memberRef.getTypeParameters(), compareTo.getTypeParameters());
             }
             return memberRef;