From 5ea28d4a9a26742433eafdaa5f1b7ef6fa5c3dc8 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Mon, 15 Jan 2018 20:30:47 +0300 Subject: [PATCH] [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. --- .../groovy/lang/psi/util/psiTreeUtil.kt | 9 ++++ .../groovy/lang/resolve/ResolveUtil.java | 48 ++---------------- .../processors/DuplicateVariableProcessor.kt | 49 +++++++++++++++++++ .../highlighting/DuplicateFields.groovy | 8 ++- 4 files changed, 69 insertions(+), 45 deletions(-) create mode 100644 plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/DuplicateVariableProcessor.kt diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/psiTreeUtil.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/psiTreeUtil.kt index 81d678d50978..236a21703ace 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/psi/util/psiTreeUtil.kt +++ b/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 PsiElement.treeWalkUpAndGet(processor: GrResolverP return processor.results } +fun PsiElement.treeWalkUpAndGetSingleResult(processor: GrResolverProcessor): T? { + return treeWalkUpAndGet(processor).singleOrNull() +} + +fun PsiElement.treeWalkUpAndGetSingleElement(processor: GrResolverProcessor>): T? { + return treeWalkUpAndGetSingleResult(processor)?.element +} + inline fun PsiElement.skipParentsOfType() = skipParentsOfType(true, T::class.java) fun PsiElement.skipParentsOfType(strict: Boolean = false, vararg types: Class<*>): Pair? { diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java index 302538490c9b..91eb9008b886 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java +++ b/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; diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/DuplicateVariableProcessor.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/DuplicateVariableProcessor.kt new file mode 100644 index 000000000000..655e4867d9e9 --- /dev/null +++ b/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>(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? { + 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 +} diff --git a/plugins/groovy/testdata/highlighting/DuplicateFields.groovy b/plugins/groovy/testdata/highlighting/DuplicateFields.groovy index e9f8b3d405d3..4d4994162b9b 100644 --- a/plugins/groovy/testdata/highlighting/DuplicateFields.groovy +++ b/plugins/groovy/testdata/highlighting/DuplicateFields.groovy @@ -1,4 +1,10 @@ class Bar { - def foo = 1 + def foo = 1 def foo = 2 +} + +class Baz { + def foo = 1 + def foo = 2 + public def foo = 3 } \ No newline at end of file