IDEA-52997: Groovy: good code is red. A field and a property of the same name are...
authorMaxim Medvedev <maxim.medvedev@jetbrains.com>
Sat, 3 Apr 2010 10:16:09 +0000 (14:16 +0400)
committerMaxim Medvedev <maxim.medvedev@jetbrains.com>
Sat, 3 Apr 2010 10:18:45 +0000 (14:18 +0400)
plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java
plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/GrVariable.java
plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/CollectClassMembersUtil.java
plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java
plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolvePropertyTest.groovy
plugins/groovy/testdata/highlighting/PropertyAndFieldDeclaration.groovy [new file with mode: 0644]

index 45e2952d75c2025a2db9c1db349f836bea85728c..d114ec046a7d5918bfa3fc48afd293cb4a118b03 100644 (file)
@@ -1386,25 +1386,39 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator {
   }
 
   private static class DuplicateVariablesProcessor extends PropertyResolverProcessor {
-    boolean borderPassed;
+    private boolean myBorderPassed;
+    private final boolean myHasVisibilityModifier;
 
     public DuplicateVariablesProcessor(GrVariable variable) {
       super(variable.getName(), variable);
-      borderPassed = false;
+      myBorderPassed = false;
+      myHasVisibilityModifier = hasExplicitVisibilityModifiers(variable);
+    }
+
+    private static boolean hasExplicitVisibilityModifiers(GrVariable variable) {
+      final PsiModifierList modifierList = variable.getModifierList();
+      if (modifierList instanceof GrModifierList) return ((GrModifierList)modifierList).hasExplicitVisibilityModifiers();
+      if (modifierList == null) return false;
+      return modifierList.hasExplicitModifier(GrModifier.PUBLIC) ||
+             modifierList.hasExplicitModifier(GrModifier.PROTECTED) ||
+             modifierList.hasExplicitModifier(GrModifier.PRIVATE);
     }
 
     @Override
     public boolean execute(PsiElement element, ResolveState state) {
-      if (borderPassed) {
+      if (myBorderPassed) {
         return false;
       }
+      if (element instanceof GrVariable && hasExplicitVisibilityModifiers((GrVariable)element) != myHasVisibilityModifier) {
+        return true;
+      }
       return super.execute(element, state);
     }
 
     @Override
     public void handleEvent(Event event, Object associated) {
       if (event == ResolveUtil.DECLARATION_SCOPE_PASSED) {
-        borderPassed = true;
+        myBorderPassed = true;
       }
       super.handleEvent(event, associated);
     }
index bf9ea116a6d3211bd27c42466d2db3d9957eaa1f..352fc2f1978ca5a6ad0eccf1a2cf3e9ea66231cb 100644 (file)
 
 package org.jetbrains.plugins.groovy.lang.psi.api.statements;
 
-import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiType;
-import com.intellij.psi.PsiVariable;
 import com.intellij.util.IncorrectOperationException;
-import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
 import org.jetbrains.plugins.groovy.lang.psi.GrNamedElement;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeElement;
@@ -32,7 +28,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeElement;
  * @date: 11.04.2007
  */
 public interface GrVariable extends GrVariableBase, GrNamedElement {
-  public static final GrVariable[] EMPTY_ARRAY = new GrVariable[0];
+  GrVariable[] EMPTY_ARRAY = new GrVariable[0];
 
   @Nullable
   GrExpression getInitializerGroovy();
index 2b55e7647e721de8648923fd762ecaf19cefcc96..df3823da00ff4101793cf60c19a935029c14fbd6 100644 (file)
@@ -23,6 +23,8 @@ import com.intellij.psi.util.*;
 import com.intellij.util.containers.HashMap;
 import com.intellij.util.containers.HashSet;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifierList;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
 
 import java.util.ArrayList;
@@ -89,6 +91,14 @@ public class CollectClassMembersUtil {
       String name = field.getName();
       if (!allFields.containsKey(name)) {
         allFields.put(name, new CandidateInfo(field, substitutor));
+      } else if (hasExplicitVisibilityModifiers(field)) {
+        final CandidateInfo candidateInfo = allFields.get(name);
+        final PsiElement element = candidateInfo.getElement();
+        if (element instanceof GrField &&
+            !(((GrField)element).getModifierList()).hasExplicitVisibilityModifiers() &&
+            aClass == ((GrField)element).getContainingClass()) { //replace property-field with field with explicit visibilityModifier 
+          allFields.put(name, new CandidateInfo(field, substitutor));
+        }
       }
     }
 
@@ -112,6 +122,15 @@ public class CollectClassMembersUtil {
     }
   }
 
+  private static boolean hasExplicitVisibilityModifiers(PsiField field) {
+    if (field instanceof GrField) {
+      return ((GrModifierList)field.getModifierList()).hasExplicitVisibilityModifiers();
+    }
+    else {
+      return true;
+    }
+  }
+
   private static void addMethod(Map<String, List<CandidateInfo>> allMethods, PsiMethod method, PsiSubstitutor substitutor) {
     String name = method.getName();
     List<CandidateInfo> methods = allMethods.get(name);
index ff6c5e95e50584cf9b4fe388a7f3f5646d1ac0e5..3cf292ba7117ac74dacb80bea4d913b0d161fb08 100644 (file)
@@ -227,4 +227,8 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase {
   public void testIndexPropertyAccess() throws Exception {
     doTest();
   }
+
+  public void testPropertyAndFieldDeclaration() throws Exception {
+    doTest();
+  }
 }
\ No newline at end of file
index d75a7e2f73650422584c509225e6e1de10388191..4e04155adbeee073ff7c8c2effcc8562afaf8690 100644 (file)
@@ -352,8 +352,112 @@ print ba<caret>r
 
   public void testStaticFieldAndNonStaticGetter() {
     myFixture.configureByText("Abc.groovy", "print Float.N<caret>aN")
-    def ref=findReference()
-    def resolved=ref.resolve()
+    def ref = findReference()
+    def resolved = ref.resolve()
     assertInstanceOf resolved, PsiField.class
   }
+
+  public void testPropertyAndFieldDeclarationInsideClass() {
+    myFixture.configureByText("a.groovy", """class Foo {
+  def foo
+  public def foo
+
+  def bar() {
+    print fo<caret>o
+  }
+}""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrField.class
+    assertTrue resolved.getModifierList().hasExplicitVisibilityModifiers()
+  }
+
+  public void testPropertyAndFieldDeclarationOutsideClass() {
+    myFixture.configureByText("a.groovy", """class Foo {
+  def foo
+  public def foo
+
+  def bar() {
+    print foo
+  }
+}
+print new Foo().fo<caret>o""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrAccessorMethod.class
+  }
+
+  public void testPropertyAndFieldDeclarationWithSuperClass1() {
+    myFixture.configureByText("a.groovy", """
+class Bar{
+  def foo
+}
+class Foo extends Bar {
+  public def foo
+
+  def bar() {
+    print foo
+  }
+}
+print new Foo().fo<caret>o""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrAccessorMethod.class
+  }
+
+  public void testPropertyAndFieldDeclarationWithSuperClass2() {
+    myFixture.configureByText("a.groovy", """
+class Bar{
+  def foo
+}
+class Foo extends Bar {
+  public def foo
+
+  def bar() {
+    print f<caret>oo
+  }
+}
+print new Foo().foo""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrField.class
+    assertTrue resolved.getModifierList().hasExplicitVisibilityModifiers()
+  }
+
+  public void testPropertyAndFieldDeclarationWithSuperClass3() {
+    myFixture.configureByText("a.groovy", """
+class Bar{
+  public def foo
+}
+class Foo extends Bar {
+  def foo
+
+  def bar() {
+    print foo
+  }
+}
+print new Foo().fo<caret>o""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrAccessorMethod.class
+  }
+  
+  public void testPropertyAndFieldDeclarationWithSuperClass4() {
+    myFixture.configureByText("a.groovy", """
+class Bar{
+  public def foo
+}
+class Foo extends Bar {
+  def foo
+
+  def bar() {
+    print f<caret>oo
+  }
+}
+print new Foo().foo""")
+    def ref = findReference()
+    def resolved = ref.resolve();
+    assertInstanceOf resolved, GrField.class
+    assertTrue !resolved.getModifierList().hasExplicitVisibilityModifiers()
+  }
 }
\ No newline at end of file
diff --git a/plugins/groovy/testdata/highlighting/PropertyAndFieldDeclaration.groovy b/plugins/groovy/testdata/highlighting/PropertyAndFieldDeclaration.groovy
new file mode 100644 (file)
index 0000000..81e9995
--- /dev/null
@@ -0,0 +1,4 @@
+class Foo {
+  def foo
+  public foo
+}
\ No newline at end of file