[groovy] fix duplicate fields highlighting (IDEA-184971)

Before the change fields were fed to processor from
CollectClassMembersUtil, if there was a field with explicit visibility
modifiers, then only this field remained in the candidate list and fed
to the processor.

After the change the CompilationPhaseHint is used, and all code fields
are fed to the processor one by one. Also now processor returns after
the first duplicate is found, ignoring the rest.

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/psiTreeUtil.kt

@@ -8,6 +8,7 @@ import com.intellij.psi.scope.PsiScopeProcessor
 import com.intellij.psi.util.parents
 import com.intellij.util.withPrevious
 import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult
+import org.jetbrains.plugins.groovy.lang.resolve.ElementGroovyResult
 import org.jetbrains.plugins.groovy.lang.resolve.GrResolverProcessor
 import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil.DECLARATION_SCOPE_PASSED
 
@@ -34,6 +35,14 @@ fun <T : GroovyResolveResult> PsiElement.treeWalkUpAndGet(processor: GrResolverP
   return processor.results
 }
 
+fun <T : GroovyResolveResult> PsiElement.treeWalkUpAndGetSingleResult(processor: GrResolverProcessor<T>): T? {
+  return treeWalkUpAndGet(processor).singleOrNull()
+}
+
+fun <T : PsiElement> PsiElement.treeWalkUpAndGetSingleElement(processor: GrResolverProcessor<ElementGroovyResult<T>>): T? {
+  return treeWalkUpAndGetSingleResult(processor)?.element
+}
+
 inline fun <reified T : PsiElement> PsiElement.skipParentsOfType() = skipParentsOfType(true, T::class.java)
 
 fun PsiElement.skipParentsOfType(strict: Boolean = false, vararg types: Class<*>): Pair<PsiElement, PsiElement?>? {

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java

@@ -1,7 +1,4 @@
-/*
- * Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
- */
-
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.plugins.groovy.lang.resolve;
 
 import com.intellij.openapi.progress.ProgressManager;
@@ -29,7 +26,6 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement;
 import org.jetbrains.plugins.groovy.lang.psi.api.GroovyMethodResult;
 import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult;
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrListOrMap;
-import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifierList;
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotation;
 import org.jetbrains.plugins.groovy.lang.psi.api.signatures.GrSignature;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.*;
@@ -56,7 +52,6 @@ import org.jetbrains.plugins.groovy.lang.psi.impl.GroovyResolveResultImpl;
 import org.jetbrains.plugins.groovy.lang.psi.impl.PsiImplUtil;
 import org.jetbrains.plugins.groovy.lang.psi.impl.signatures.GrClosureSignatureUtil;
 import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
-import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrBindingVariable;
 import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrLightParameter;
 import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrScriptField;
 import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GroovyScriptClass;
@@ -71,6 +66,7 @@ import java.util.*;
 import static com.intellij.util.containers.ContainerUtil.count;
 import static com.intellij.util.containers.ContainerUtil.filter;
 import static org.jetbrains.plugins.groovy.lang.psi.impl.GrAnnotationUtilKt.hasAnnotation;
+import static org.jetbrains.plugins.groovy.lang.psi.util.PsiTreeUtilKt.treeWalkUpAndGetSingleElement;
 import static org.jetbrains.plugins.groovy.lang.resolve.ResolveUtilKt.getDefaultConstructor;
 import static org.jetbrains.plugins.groovy.lang.resolve.ResolveUtilKt.initialState;
 
@@ -896,14 +892,14 @@ public class ResolveUtil {
       return duplicates.size() > 0 ? duplicates.get(0) : null;
     }
     else {
-      PsiNamedElement duplicate = resolveExistingElement(variable, new DuplicateVariablesProcessor(variable), GrVariable.class);
+      PsiNamedElement duplicate = treeWalkUpAndGetSingleElement(variable, new DuplicateVariableProcessor(variable));
       final PsiElement context1 = variable.getContext();
       if (duplicate == null && variable instanceof GrParameter && context1 != null) {
         final PsiElement context = context1.getContext();
         if (context instanceof GrClosableBlock ||
             context instanceof GrMethod && !(context.getParent() instanceof GroovyFile) ||
             context instanceof GrTryCatchStatement) {
-          duplicate = resolveExistingElement(context.getParent(), new DuplicateVariablesProcessor(variable), GrVariable.class);
+          duplicate = treeWalkUpAndGetSingleElement(context.getParent(), new DuplicateVariableProcessor(variable));
         }
       }
       if (duplicate instanceof GrLightParameter && "args".equals(duplicate.getName())) {
@@ -1051,42 +1047,6 @@ public class ResolveUtil {
     return params[0];
   }
 
-  private static class DuplicateVariablesProcessor extends PropertyResolverProcessor {
-    private boolean myBorderPassed;
-    private final boolean myHasVisibilityModifier;
-
-    public DuplicateVariablesProcessor(GrVariable variable) {
-      super(variable.getName(), variable);
-      myBorderPassed = false;
-      myHasVisibilityModifier = hasExplicitVisibilityModifiers(variable);
-    }
-
-    private static boolean hasExplicitVisibilityModifiers(GrVariable variable) {
-      final GrModifierList modifierList = variable.getModifierList();
-      return modifierList != null && modifierList.hasExplicitVisibilityModifiers();
-    }
-
-    @Override
-    public boolean execute(@NotNull PsiElement element, @NotNull ResolveState state) {
-      if (myBorderPassed) {
-        return false;
-      }
-      if (element instanceof GrVariable && hasExplicitVisibilityModifiers((GrVariable)element) != myHasVisibilityModifier) {
-        return true;
-      }
-      if (element instanceof GrBindingVariable) return true;
-      return super.execute(element, state);
-    }
-
-    @Override
-    public void handleEvent(@NotNull Event event, Object associated) {
-      if (event == DECLARATION_SCOPE_PASSED) {
-        myBorderPassed = true;
-      }
-      super.handleEvent(event, associated);
-    }
-  }
-
   public static boolean isAccessible(@NotNull PsiElement place, @NotNull PsiNamedElement namedElement) {
     if (namedElement instanceof GrField) {
       final GrField field = (GrField)namedElement;

plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/DuplicateVariableProcessor.kt

@@ -0,0 +1,49 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package org.jetbrains.plugins.groovy.lang.resolve.processors
+
+import com.intellij.psi.PsiAnonymousClass
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiElement
+import com.intellij.psi.ResolveState
+import com.intellij.psi.scope.PsiScopeProcessor
+import org.jetbrains.plugins.groovy.lang.psi.GroovyFile
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod
+import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrBindingVariable
+import org.jetbrains.plugins.groovy.lang.resolve.CompilationPhaseHint
+import org.jetbrains.plugins.groovy.lang.resolve.ElementGroovyResult
+import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil.DECLARATION_SCOPE_PASSED
+
+class DuplicateVariableProcessor(private val variable: GrVariable) : FindFirstProcessor<ElementGroovyResult<GrVariable>>(variable.name) {
+
+  companion object {
+    private fun GrVariable.hasExplicitVisibilityModifiers(): Boolean = modifierList?.hasExplicitVisibilityModifiers() ?: false
+  }
+
+  init {
+    hint(CompilationPhaseHint.HINT_KEY, CompilationPhaseHint { CompilationPhaseHint.Phase.CONVERSION })
+  }
+
+  private val hasVisibilityModifier = variable.hasExplicitVisibilityModifiers()
+
+  override fun result(element: PsiElement, state: ResolveState): ElementGroovyResult<GrVariable>? {
+    if (element !is GrVariable || element is GrBindingVariable) return null
+    if (element == variable) return null
+    if (element.hasExplicitVisibilityModifiers() != hasVisibilityModifier) return null
+    return ElementGroovyResult(element)
+  }
+
+  private var myBorderPassed: Boolean = false
+
+  override fun handleEvent(event: PsiScopeProcessor.Event, associated: Any?) {
+    if (event != DECLARATION_SCOPE_PASSED || associated !is PsiElement) return
+    if (associated is GrClosableBlock && GrClosableBlock.OWNER_NAME == name ||
+        associated is PsiClass && associated !is PsiAnonymousClass ||
+        associated is GrMethod && associated.parent is GroovyFile) {
+      myBorderPassed = true
+    }
+  }
+
+  override fun shouldStop(): Boolean = myBorderPassed
+}

plugins/groovy/testdata/highlighting/DuplicateFields.groovy

@@ -1,4 +1,10 @@
 class Bar {
-  def foo = 1
+  def <error descr="Field 'foo' already defined">foo</error> = 1
   def <error descr="Field 'foo' already defined">foo</error> = 2
+}
+
+class Baz {
+  def <error descr="Field 'foo' already defined">foo</error> = 1
+  def <error descr="Field 'foo' already defined">foo</error> = 2
+  public def foo = 3
 }