Fix for #1331: setter method precedence

groovy · Jan 10, 2022 · 4dacd66 · 4dacd66
1 parent 9fc6732
commit 4dacd66
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 7 deletions.
diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
@@ -646,7 +646,8 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
 
         // look for canonical accessor method
         Optional<MethodNode> accessor = findPropertyAccessorMethod(name, declaringType, isLhsExpression, isStaticExpression, methodCallArgumentTypes).filter(it -> !isSynthetic(it));
-        if (accessor.isPresent() && directFieldAccess == 0) {
+        boolean nonPrivateAccessor = accessor.filter(it -> !it.isPrivate() || declaringType.equals(it.getDeclaringClass())).isPresent();
+        if (nonPrivateAccessor && directFieldAccess == 0) {
             return accessor.get();
         }
 
@@ -678,7 +679,7 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
             return field;
         }
 
-        if (dynamicProperty) {
+        if (dynamicProperty && !(isLhsExpression && nonPrivateAccessor)) { // GROOVY-5491
             return createDynamicProperty(name, getMapPropertyType(declaringType), declaringType, isStaticExpression);
         }
 

diff --git a/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java b/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 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.
@@ -1444,14 +1444,20 @@ public void visitMapExpression(final MapExpression node) {
                     VariableScope scope = scopes.getLast();
                     // look for a non-synthetic setter followed by a property or field
                     String setterName = AccessorSupport.SETTER.createAccessorName(key.getText());
-                    scope.setMethodCallArgumentTypes(getMethodCallArgumentTypes(GeneralUtils.callX(ctorType, setterName, value)));
-                    TypeLookupResult result = lookupExpressionType(GeneralUtils.constX(setterName), ctorType, false, scope);
+                    MethodCallExpression setterCall = GeneralUtils.callX(GeneralUtils.varX("this", ctorType), setterName, value);
+                    scope.setCurrentNode(setterCall); scope.setMethodCallArgumentTypes(getMethodCallArgumentTypes(setterCall));
+                    scope.setCurrentNode(setterCall.getMethod()); // scope.getEnclosingNode() should return setterCall
+                    TypeLookupResult result = lookupExpressionType(setterCall.getMethod(), ctorType, false, scope);
+                    scope.forgetCurrentNode();
                     if (result.confidence == TypeConfidence.UNKNOWN || !(result.declaration instanceof MethodNode) ||
                             ((MethodNode) result.declaration).isSynthetic()) {
+                        scope.setCurrentNode(key);
                         scope.getWormhole().put("lhs", key);
                         scope.setMethodCallArgumentTypes(null);
                         result = lookupExpressionType(key, ctorType, false, scope);
+                        scope.forgetCurrentNode();
                     }
+                    scope.forgetCurrentNode();
 
                     // pre-visit entry so keys are highlighted as keys, not fields/methods/properties
                     ClassNode mapType = Optional.ofNullable(exprType).orElseGet(() -> createParameterizedMap(key.getType(), value.getType()));

diff --git a/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy b/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 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.
@@ -1032,6 +1032,36 @@ final class SemanticHighlightingTests extends GroovyEclipseTestSuite {
 
     @Test
     void testNamedParams3() {
+        addGroovySource '''\
+            |class C {
+            |  private void setX(x) {
+            |  }
+            |}
+            |'''.stripMargin()
+
+        String contents = '''\
+            |class D extends C {
+            |  private void setY(y) {
+            |  }
+            |}
+            |new D(x: 'x', y: 'y')
+            |'''.stripMargin()
+
+        assertHighlighting(contents,
+            new HighlightedTypedPosition(contents.indexOf('D'), 1, CLASS),
+            new HighlightedTypedPosition(contents.indexOf('C'), 1, CLASS),
+            new HighlightedTypedPosition(contents.indexOf('setY'), 4, METHOD),
+            new HighlightedTypedPosition(contents.indexOf('y'), 1, PARAMETER),
+            new HighlightedTypedPosition(contents.lastIndexOf('D'), 1, CLASS),
+            new HighlightedTypedPosition(contents.lastIndexOf('D'), 1, CTOR_CALL),
+            new HighlightedTypedPosition(contents.lastIndexOf('x:'), 1, MAP_KEY),
+            new HighlightedTypedPosition(contents.lastIndexOf('x:'), 1, UNKNOWN),
+            new HighlightedTypedPosition(contents.lastIndexOf('y:'), 1, MAP_KEY),
+            new HighlightedTypedPosition(contents.lastIndexOf('y:'), 1, METHOD_CALL))
+    }
+
+    @Test
+    void testNamedParams4() {
         String contents = 'def map = Collections.singletonMap(key: "k", value: "v")'
 
         assertHighlighting(contents,
@@ -1812,7 +1842,7 @@ final class SemanticHighlightingTests extends GroovyEclipseTestSuite {
             |    this();
             |  }
             |}
-            |def x = new X();
+            |def x = new X()
             |'''.stripMargin()
 
         assertHighlighting(contents,
@@ -3100,6 +3130,53 @@ final class SemanticHighlightingTests extends GroovyEclipseTestSuite {
             new HighlightedTypedPosition(contents.lastIndexOf('three'), 5, MAP_KEY))
     }
 
+    @Test // GROOVY-5491
+    void testMapKey7() {
+        addGroovySource '''\
+            |import groovy.transform.PackageScope
+            |abstract class A {
+            |  public        void setFour(x) {}
+            |  protected     void setFive(x) {}
+            |  @PackageScope void setSixx(x) {}
+            |  private       void setSeven(x) {}
+            |}
+            |'''.stripMargin()
+
+        String contents = '''\
+            |class C extends A implements Map {
+            |  private void setEight(x) {}
+            |  void test() {
+            |    four = null // set
+            |    five = null // set
+            |    sixx = null // set
+            |    seven = null // put
+            |    eight = null // set
+            |  }
+            |}
+            |new C().seven = null // put
+            |new C().eight = null // set
+            |'''.stripMargin()
+
+        assertHighlighting(contents,
+            new HighlightedTypedPosition(contents.indexOf('C'), 1, CLASS),
+            new HighlightedTypedPosition(contents.indexOf('A'), 1, ABSTRACT_CLASS),
+            new HighlightedTypedPosition(contents.indexOf('Map'), 3, INTERFACE),
+            new HighlightedTypedPosition(contents.indexOf('setEight'), 8, METHOD),
+            new HighlightedTypedPosition(contents.indexOf('x)'), 1, PARAMETER),
+            new HighlightedTypedPosition(contents.indexOf('test'), 4, METHOD),
+            new HighlightedTypedPosition(contents.indexOf('four'), 4, METHOD_CALL),
+            new HighlightedTypedPosition(contents.indexOf('five'), 4, METHOD_CALL),
+            new HighlightedTypedPosition(contents.indexOf('sixx'), 4, METHOD_CALL),
+            new HighlightedTypedPosition(contents.indexOf('seven'), 5, MAP_KEY),
+            new HighlightedTypedPosition(contents.indexOf('eight'), 5, METHOD_CALL),
+            new HighlightedTypedPosition(contents.indexOf('C().s'), 1, CLASS),
+            new HighlightedTypedPosition(contents.indexOf('C().s'), 1, CTOR_CALL),
+            new HighlightedTypedPosition(contents.lastIndexOf('seven'), 5, MAP_KEY),
+            new HighlightedTypedPosition(contents.indexOf('C().e'), 1, CLASS),
+            new HighlightedTypedPosition(contents.indexOf('C().e'), 1, CTOR_CALL),
+            new HighlightedTypedPosition(contents.lastIndexOf('eight'), 5, METHOD_CALL))
+    }
+
     @Test
     void testSpread() {
         String contents = '''\