Fix for issue #521: improve support for non-propagating scope paths

- fix for declaring type of parameter nodes - fix for instanceof with nested assignment
groovy · Mar 9, 2018 · d092ff0 · d092ff0
1 parent acaed84
commit d092ff0
Show file tree

Hide file tree

Showing 6 changed files with 259 additions and 124 deletions.
diff --git a/...sts.builder/src/org/eclipse/jdt/core/groovy/tests/search/DeclarationInferencingTests.java b/...sts.builder/src/org/eclipse/jdt/core/groovy/tests/search/DeclarationInferencingTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2017 the original author or authors.
+ * Copyright 2009-2018 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.
@@ -478,7 +478,7 @@ public void testLocalAndFieldWithSameName() {
             "}";
 
         int start = contents.lastIndexOf("xxx"), end = start + "xxx".length();
-        assertDeclaration(contents, start, end, "java.lang.Object", "xxx", DeclarationKind.VARIABLE);
+        assertDeclaration(contents, start, end, "Foo", "xxx", DeclarationKind.VARIABLE);
     }
 
     @Test
@@ -492,7 +492,7 @@ public void testParamAndFieldWithSameName() {
             "}";
 
         int start = contents.lastIndexOf("xxx"), end = start + "xxx".length();
-        assertDeclaration(contents, start, end, "java.lang.Object", "xxx", DeclarationKind.VARIABLE);
+        assertDeclaration(contents, start, end, "Foo", "xxx", DeclarationKind.VARIABLE);
     }
 
     @Test

diff --git a/...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java b/...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java
@@ -198,9 +198,9 @@ public void testLocalVar7a() {
         assertType(contents, offset, offset + 1, "java.lang.Number");
     }
 
-    @Test @Ignore("not yet implemented")
+    @Test
     public void testLocalVar8() {
-        String contents = "def m() {" +
+        String contents = "def m() {\n" +
             "  def x\n" +
             "  x = 1\n" +
             "  if (predicate()) {\n" +
@@ -223,9 +223,9 @@ public void testLocalVar8() {
         assertType(contents, offset, offset + 1, "java.lang.Integer");
     }
 
-    @Test @Ignore("not yet implemented")
+    @Test
     public void testLocalVar8a() {
-        String contents = "def m() {" +
+        String contents = "def m() {\n" +
             "  def x\n" +
             "  x = 1\n" +
             "  if (predicate())\n" +
@@ -383,20 +383,55 @@ public void testLocalVar11() {
 
     @Test
     public void testLocalVar12() {
+        String contents = "def x\n" +
+            "x = ''\n" +
+            "def cl = { ->\n" +
+            "  x = 1.0d\n" +
+            "}\n" +
+            "x\n" +
+            "x = 1.0\n" +
+            "x";
+
+        int offset = contents.indexOf("x");
+        assertType(contents, offset, offset + 1, "java.lang.Object");
+
+        // line 2
+        offset = contents.indexOf("x", offset + 1);
+        assertType(contents, offset, offset + 1, "java.lang.String");
+
+        // line 4
+        offset = contents.indexOf("x", offset + 1);
+        assertType(contents, offset, offset + 1, "java.lang.Double");
+
+        // line 6
+        offset = contents.indexOf("x", offset + 1);
+        assertType(contents, offset, offset + 1, "java.lang.String");
+
+        // line 7
+        offset = contents.indexOf("x", offset + 1);
+        assertType(contents, offset, offset + 1, "java.math.BigDecimal");
+
+        // line 8
+        offset = contents.indexOf("x", offset + 1);
+        assertType(contents, offset, offset + 1, "java.math.BigDecimal");
+    }
+
+    @Test
+    public void testLocalVar13() {
         String contents = "String x\n" +
             "x";
         assertExprType(contents, "x", "java.lang.String");
     }
 
     @Test
-    public void testLocalVar13() {
+    public void testLocalVar14() {
         String contents = "String x = 7\n" +
             "x";
         assertExprType(contents, "x", "java.lang.String");
     }
 
     @Test
-    public void testLocalVar14() {
+    public void testLocalVar15() {
         String contents = "String x\n" +
             "x = 7\n" + // GroovyCastException at runtime
             "x";

diff --git a/...ipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java b/...ipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java
@@ -21,6 +21,7 @@
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.FieldExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
@@ -89,7 +90,7 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
                 doCheck = true;
                 isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
                 // fully-qualified field expressions in static contexts will have an sloc of the entire qualified name
-                start = end - fieldName.length();
+                start = node.getEnd() - fieldName.length();
                 end = node.getEnd();
             }
         } else if (node instanceof FieldNode) {
@@ -104,12 +105,12 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
                 end = fnode.getNameEnd() + 1; // arrrgh...why +1?
             }
         } else if (node instanceof VariableExpression) {
-            String vname = ((VariableExpression) node).getName();
-            if (fieldName.equals(vname)) {
+            if (fieldName.equals(((VariableExpression) node).getName()) &&
+                    (result.declaration instanceof FieldNode || result.declaration instanceof PropertyNode)) {
                 doCheck = true;
                 isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
                 start = node.getStart();
-                end = start + vname.length();
+                end = node.getEnd();
             }
         }
 
@@ -121,20 +122,20 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
                 // GRECLIPSE-540: Still unresolved is that all field and variable references are considered reads. We don't know about writes.
                 if (isCompleteMatch && ((isAssignment && writeAccess) || (!isAssignment && readAccess) || (isDeclaration && findDeclarations))) {
                     SearchMatch match = null;
-
                     // must translate from synthetic source to binary if necessary
-                    IJavaElement realElement = enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy ? ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement) : enclosingElement;
+                    if (enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy)
+                        enclosingElement = ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement);
                     if (isDeclaration && findDeclarations) {
-                        match = new FieldDeclarationMatch(realElement, getAccuracy(result.confidence, isCompleteMatch), start, end - start, participant, realElement.getResource());
+                        match = new FieldDeclarationMatch(enclosingElement, getAccuracy(result.confidence, isCompleteMatch), start, end - start, participant, enclosingElement.getResource());
                     } else if (!isDeclaration && findReferences) {
-                        match = new FieldReferenceMatch(realElement, getAccuracy(result.confidence, isCompleteMatch), start, end - start, !isAssignment, isAssignment, false, participant, realElement.getResource());
+                        match = new FieldReferenceMatch(enclosingElement, getAccuracy(result.confidence, isCompleteMatch), start, end - start, !isAssignment, isAssignment, false, participant, enclosingElement.getResource());
                     }
                     if (match != null) {
                         try {
                             requestor.acceptSearchMatch(match);
                             acceptedPositions.add(position);
                         } catch (CoreException e) {
-                            Util.log(e, "Error reporting search match inside of " + realElement + " in resource " + realElement.getResource());
+                            Util.log(e, "Error reporting search match inside of " + enclosingElement + " in resource " + enclosingElement.getResource());
                         }
                     }
                 }