From 4d92992b7bb668350826e018d02be1f61a4c336d Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Thu, 18 Jan 2018 19:48:41 +0300 Subject: [PATCH] [groovy] resolve fields before transformations Before the change the `foo` reference inside of a closure was resolved to a field. Now it's properly resolved to an accessor method (as done by Groovy itself). ``` class A { def foo def bar = { foo } } ``` Before the change the `foo` reference inside of a class was resolved to a field even when it was qualified. Now it is resolved to an accessor method. ``` class A { def foo def bar() { new A().foo } } ``` This change also speeds up resolution as it stops when first field is found, i.e. transformations and non-code processors will not be run. --- .../lang/resolve/GrReferenceResolveRunner.kt | 62 +++++++- .../plugins/groovy/lang/resolve/owner.kt | 27 ++++ .../resolve/processors/CodeFieldProcessor.kt | 26 ++++ .../GroovyResolverProcessorImpl.java | 22 +-- .../refactoring/GroovyRefactoringUtil.java | 30 ++-- .../GrClassMemberReferenceVisitor.java | 35 ++--- .../java2groovy/OldReferencesResolver.java | 20 +-- .../memberPullUp/GrPullUpHelper.java | 78 ++++------ .../rename/RenameGrFieldProcessor.java | 37 ++--- .../resolve/ResolveFieldVsAccessorTest.groovy | 137 ++++++++++++++++++ .../lang/resolve/ResolvePropertyTest.groovy | 69 +-------- .../GrIntroduceParameterInClosureTest.groovy | 21 +-- ...ReplaceWithGetter.groovy => Getter.groovy} | 0 ...etter_after.groovy => Getter_after.groovy} | 4 +- 14 files changed, 342 insertions(+), 226 deletions(-) create mode 100644 plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/owner.kt create mode 100644 plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/CodeFieldProcessor.kt create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveFieldVsAccessorTest.groovy rename plugins/groovy/testdata/refactoring/introduceParameterInClosure/{DontReplaceWithGetter.groovy => Getter.groovy} (100%) rename plugins/groovy/testdata/refactoring/introduceParameterInClosure/{DontReplaceWithGetter_after.groovy => Getter_after.groovy} (61%) diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/GrReferenceResolveRunner.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/GrReferenceResolveRunner.kt index 53b8a7c781cc..ef326ed0d01b 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/GrReferenceResolveRunner.kt +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/GrReferenceResolveRunner.kt @@ -10,16 +10,20 @@ import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotationArrayInitializer import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotationNameValuePair +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrMethodCall import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression import org.jetbrains.plugins.groovy.lang.psi.impl.GroovyResolveResultImpl import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil +import org.jetbrains.plugins.groovy.lang.psi.util.isThisExpression import org.jetbrains.plugins.groovy.lang.psi.util.treeWalkUpAndGet import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil.canResolveToMethod import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil.isDefinitelyKeyOfMap import org.jetbrains.plugins.groovy.lang.resolve.processors.ClassHint +import org.jetbrains.plugins.groovy.lang.resolve.processors.CodeFieldProcessor import org.jetbrains.plugins.groovy.lang.resolve.processors.GroovyResolverProcessorBuilder import org.jetbrains.plugins.groovy.lang.resolve.processors.LocalVariableProcessor @@ -100,8 +104,8 @@ fun GrReferenceExpression.getCallVariants(upToArgument: GrExpression?): Array { resolvePackageOrClass()?.let { return listOf(it) } - val localVariableResults = resolveLocalVariable() - if (localVariableResults.isNotEmpty()) return localVariableResults + val staticResults = resolveStatic() + if (staticResults.isNotEmpty()) return staticResults if (!canResolveToMethod(this) && isDefinitelyKeyOfMap(this)) return emptyList() val processor = GroovyResolverProcessorBuilder.builder() @@ -142,8 +146,58 @@ private fun GrReferenceExpression.doResolvePackageOrClass(): PsiElement? { return null } -private fun GrReferenceExpression.resolveLocalVariable(): Collection { - if (isQualified) return emptyList() +/** + * Resolves elements that exist before transformations are run. + * + * @see org.codehaus.groovy.control.ResolveVisitor + */ +private fun GrReferenceExpression.resolveStatic(): Collection { val name = referenceName ?: return emptyList() + + val qualifier = qualifier + + if (qualifier == null) { + val locals = resolveToLocalVariable(name) + if (locals.size == 1) return locals + } + + if (qualifier == null || qualifier.isThisExpression()) { + val fields = resolveToField(name) + val field = fields.singleOrNull() + if (field != null && checkCurrentClass(field.element, this)) return fields + } + + return emptyList() +} + +/** + * Walks up the tree and returns when the first [local variable][GrVariable] is found. + * + * @name local variable name + * @receiver call site + * @return empty collection or a collection with 1 local variable result + */ +private fun PsiElement.resolveToLocalVariable(name: String): Collection> { return treeWalkUpAndGet(LocalVariableProcessor(name)) } + +/** + * Walks up the tree and returns when the first code [field][GrField] is found. + * + * @name field name + * @receiver call site + * @return empty collection or a collection with 1 code field result + */ +private fun PsiElement.resolveToField(name: String): Collection> { + return treeWalkUpAndGet(CodeFieldProcessor(name, this)) +} + +/** + * Checks if resolved [field] is a field of current class owner. + * + * @see org.codehaus.groovy.control.ResolveVisitor.currentClass + */ +private fun checkCurrentClass(field: GrField, place: PsiElement): Boolean { + val containingClass = field.containingClass ?: return false + return containingClass == place.getOwner() +} diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/owner.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/owner.kt new file mode 100644 index 000000000000..6a0b40dc37f8 --- /dev/null +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/owner.kt @@ -0,0 +1,27 @@ +// 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.psi.PsiElement +import org.jetbrains.plugins.groovy.lang.psi.GroovyFile +import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition +import org.jetbrains.plugins.groovy.lang.psi.util.contexts + +private fun PsiElement.isOwner(): Boolean = when (this) { + is GrTypeDefinition, is GrClosableBlock -> true + is GroovyFile -> context != null + else -> false +} + +/** + * Returns an immediate owner, which can be one of the following: + * - class + * - closure + * - file + * + * @receiver element which owner is needed + * @return immediate owner + */ +fun PsiElement.getOwner(): PsiElement? = contexts().firstOrNull { + it.isOwner() +} diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/CodeFieldProcessor.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/CodeFieldProcessor.kt new file mode 100644 index 000000000000..8c303291d107 --- /dev/null +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/CodeFieldProcessor.kt @@ -0,0 +1,26 @@ +// 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.PsiElement +import com.intellij.psi.PsiSubstitutor +import com.intellij.psi.ResolveState +import com.intellij.psi.scope.ElementClassHint +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField +import org.jetbrains.plugins.groovy.lang.resolve.BaseGroovyResolveResult +import org.jetbrains.plugins.groovy.lang.resolve.CompilationPhaseHint +import org.jetbrains.plugins.groovy.lang.resolve.CompilationPhaseHint.Phase.TRANSFORMATION +import org.jetbrains.plugins.groovy.lang.resolve.ElementGroovyResult + +class CodeFieldProcessor(name: String, private val place: PsiElement) : FindFirstProcessor>(name) { + + init { + hint(ElementClassHint.KEY, ElementClassHint { false }) + hint(GroovyResolveKind.HINT_KEY, GroovyResolveKind.Hint { it == GroovyResolveKind.FIELD }) + hint(CompilationPhaseHint.HINT_KEY, CompilationPhaseHint { TRANSFORMATION }) + } + + override fun result(element: PsiElement, state: ResolveState): ElementGroovyResult? { + val field = element as? GrField ?: return null + return BaseGroovyResolveResult(field, place, state.get(ClassHint.RESOLVE_CONTEXT), state.get(PsiSubstitutor.KEY)) + } +} diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/GroovyResolverProcessorImpl.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/GroovyResolverProcessorImpl.java index da623092f615..8d34aee57ca2 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/GroovyResolverProcessorImpl.java +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/processors/GroovyResolverProcessorImpl.java @@ -1,11 +1,7 @@ -/* - * Copyright 2000-2017 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.processors; -import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiField; import com.intellij.psi.PsiType; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; @@ -13,8 +9,6 @@ import org.jetbrains.annotations.Nullable; 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.statements.expressions.GrReferenceExpression; -import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrBindingVariable; -import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.lang.resolve.GrMethodComparator; import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; @@ -51,20 +45,6 @@ class GroovyResolverProcessorImpl extends GroovyResolverProcessor implements GrM return candidates; } - candidates = getCandidates(GroovyResolveKind.FIELD); - if (!candidates.isEmpty()) { - assert candidates.size() == 1; - final GroovyResolveResult candidate = candidates.get(0); - final PsiElement element = candidate.getElement(); - if (element instanceof PsiField) { - final PsiClass containingClass = ((PsiField)element).getContainingClass(); - if (containingClass != null && PsiUtil.getContextClass(myRef) == containingClass) return candidates; - } - else if (!(element instanceof GrBindingVariable)) { - return candidates; - } - } - if (myIsPartOfFqn) { candidates = getCandidates(GroovyResolveKind.PACKAGE, GroovyResolveKind.CLASS); if (!candidates.isEmpty()) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java index e937f6a3d4cf..83983d3cf0b1 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/GroovyRefactoringUtil.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring; @@ -64,6 +50,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeArgumentList; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrVariableDeclarationOwner; +import org.jetbrains.plugins.groovy.lang.psi.util.GroovyPropertyUtils; import java.util.*; @@ -724,4 +711,17 @@ public abstract class GroovyRefactoringUtil { } }); } + + @NotNull + public static String getNewName(@NotNull PsiNamedElement element, boolean property) { + final String name; + if (element instanceof PsiMethod && property) { + name = GroovyPropertyUtils.getPropertyName((PsiMethod)element); + } + else { + name = element.getName(); + } + LOG.assertTrue(name != null, element); + return name; + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/classMembers/GrClassMemberReferenceVisitor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/classMembers/GrClassMemberReferenceVisitor.java index 5286c1cf5bd6..72d7822ba61b 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/classMembers/GrClassMemberReferenceVisitor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/classMembers/GrClassMemberReferenceVisitor.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring.classMembers; import com.intellij.psi.PsiClass; @@ -22,6 +8,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor; +import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition; @@ -51,23 +38,25 @@ public abstract class GrClassMemberReferenceVisitor extends GroovyRecursiveEleme } } - PsiElement resolved = ref.resolve(); + GroovyResolveResult resolveResult = ref.advancedResolve(); + PsiElement resolved = resolveResult.getElement(); if (resolved instanceof GrMember) { PsiClass containingClass = ((GrMember)resolved).getContainingClass(); if (isPartOf(myClass, containingClass)) { - visitClassMemberReferenceElement((GrMember)resolved, ref); + visitClassMemberReferenceElement(ref, (GrMember)resolved, resolveResult); } } } @Override public void visitCodeReferenceElement(@NotNull GrCodeReferenceElement reference) { - PsiElement referencedElement = reference.resolve(); + GroovyResolveResult resolveResult = reference.advancedResolve(); + PsiElement referencedElement = resolveResult.getElement(); if (referencedElement instanceof GrTypeDefinition) { final GrTypeDefinition referencedClass = (GrTypeDefinition)referencedElement; if (PsiTreeUtil.isAncestor(myClass, referencedElement, true) || isPartOf(myClass, referencedClass.getContainingClass())) { - visitClassMemberReferenceElement((GrMember)referencedElement, reference); + visitClassMemberReferenceElement(reference, (GrMember)referencedElement, resolveResult); } } } @@ -77,5 +66,11 @@ public abstract class GrClassMemberReferenceVisitor extends GroovyRecursiveEleme return aClass.equals(containingClass) || aClass.isInheritor(containingClass, true); } - protected abstract void visitClassMemberReferenceElement(GrMember resolved, GrReferenceElement ref); + protected void visitClassMemberReferenceElement(GrReferenceElement ref, GrMember member, GroovyResolveResult resolveResult) { + visitClassMemberReferenceElement(member, ref); + } + + protected void visitClassMemberReferenceElement(GrMember resolved, GrReferenceElement ref) { + throw new RuntimeException("Override one of visitClassMemberReferenceElement() methods"); + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/java2groovy/OldReferencesResolver.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/java2groovy/OldReferencesResolver.java index 829a7cd19341..348860fc71da 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/java2groovy/OldReferencesResolver.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/introduce/parameter/java2groovy/OldReferencesResolver.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring.introduce.parameter.java2groovy; @@ -51,6 +37,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import static org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil.getNewName; + /** * @author Maxim.Medvedev */ @@ -180,7 +168,7 @@ public class OldReferencesResolver { boolean isStatic = subj instanceof PsiField && ((PsiField)subj).hasModifierProperty(PsiModifier.STATIC) || subj instanceof PsiMethod && ((PsiMethod)subj).hasModifierProperty(PsiModifier.STATIC); - String name = ((PsiNamedElement)subj).getName(); + String name = getNewName((PsiNamedElement)subj, adv.isInvokedOnProperty()); boolean shouldBeAt = subj instanceof PsiField && !PsiTreeUtil.isAncestor(((PsiMember)subj).getContainingClass(), newExpr, true) && GroovyPropertyUtils.findGetterForField((PsiField)subj) != null; diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/memberPullUp/GrPullUpHelper.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/memberPullUp/GrPullUpHelper.java index c6de0be0cecf..cbd4f5400102 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/memberPullUp/GrPullUpHelper.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/memberPullUp/GrPullUpHelper.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring.memberPullUp; import com.intellij.codeInsight.AnnotationUtil; @@ -46,6 +32,7 @@ import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory; import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor; +import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; @@ -62,6 +49,8 @@ import org.jetbrains.plugins.groovy.util.GroovyChangeContextUtil; import java.util.*; +import static org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil.getNewName; + public class GrPullUpHelper implements PullUpHelper { private static final Logger LOG = Logger.getInstance(GrPullUpHelper.class); @@ -355,18 +344,15 @@ public class GrPullUpHelper implements PullUpHelper { private void fixReferencesToStatic(GroovyPsiElement classMember) throws IncorrectOperationException { final StaticReferencesCollector collector = new StaticReferencesCollector(myMembersToMove); classMember.accept(collector); - ArrayList refs = collector.getReferences(); - ArrayList members = collector.getReferees(); - ArrayList classes = collector.getRefereeClasses(); GroovyPsiElementFactory factory = GroovyPsiElementFactory.getInstance(myProject); - for (int i = 0; i < refs.size(); i++) { - GrReferenceElement ref = refs.get(i); - PsiElement namedElement = members.get(i); - PsiClass aClass = classes.get(i); - + for (StaticReferenceResult result : collector.results) { + GrReferenceElement ref = result.reference; + GrMember namedElement = result.referee; + PsiClass aClass = result.refereeClass; if (namedElement instanceof PsiNamedElement) { - GrReferenceExpression newRef = (GrReferenceExpression)factory.createExpressionFromText("a." + ((PsiNamedElement)namedElement).getName(), null); + String name = getNewName((PsiNamedElement)namedElement, result.invokedOnProperty); + GrReferenceExpression newRef = (GrReferenceExpression)factory.createExpressionFromText("a." + name, null); GrExpression qualifier = newRef.getQualifierExpression(); assert qualifier != null; qualifier = (GrExpression)qualifier.replace(factory.createReferenceExpressionFromText(aClass.getQualifiedName())); @@ -377,43 +363,43 @@ public class GrPullUpHelper implements PullUpHelper { } } + private static class StaticReferenceResult { + final GrReferenceElement reference; + final GrMember referee; + final PsiClass refereeClass; + final boolean invokedOnProperty; + + private StaticReferenceResult(GrReferenceElement reference, GrMember referee, PsiClass refereeClass, boolean invokedOnProperty) { + this.reference = reference; + this.referee = referee; + this.refereeClass = refereeClass; + this.invokedOnProperty = invokedOnProperty; + } + } private class StaticReferencesCollector extends GrClassMemberReferenceVisitor { - private final ArrayList myReferences = new ArrayList<>(); - private final ArrayList myReferees = new ArrayList<>(); - private final ArrayList myRefereeClasses = new ArrayList<>(); + private final Set myMovedMembers; + final List results = new ArrayList<>(); private StaticReferencesCollector(Set movedMembers) { super(mySourceClass); myMovedMembers = movedMembers; } - public ArrayList getReferees() { - return myReferees; - } - - public ArrayList getRefereeClasses() { - return myRefereeClasses; - } - - public ArrayList getReferences() { - return myReferences; - } - @Override - protected void visitClassMemberReferenceElement(GrMember classMember, GrReferenceElement classMemberReference) { + protected void visitClassMemberReferenceElement(GrReferenceElement ref, GrMember classMember, GroovyResolveResult resolveResult) { if (classMember.hasModifierProperty(PsiModifier.STATIC) /*&& classMemberReference.isQualified()*/) { if (!myMovedMembers.contains(classMember) && RefactoringHierarchyUtil.isMemberBetween(myTargetSuperClass, mySourceClass, classMember)) { - myReferences.add(classMemberReference); - myReferees.add(classMember); - myRefereeClasses.add(classMember.getContainingClass()); + results.add(new StaticReferenceResult( + ref, classMember, classMember.getContainingClass(), resolveResult.isInvokedOnProperty() + )); } else if (myMovedMembers.contains(classMember) || myMembersAfterMove.contains(classMember)) { - myReferences.add(classMemberReference); - myReferees.add(classMember); - myRefereeClasses.add(myTargetSuperClass); + results.add(new StaticReferenceResult( + ref, classMember, myTargetSuperClass, resolveResult.isInvokedOnProperty() + )); } } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/rename/RenameGrFieldProcessor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/rename/RenameGrFieldProcessor.java index 8c36d0799d29..2e1149f57cd3 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/rename/RenameGrFieldProcessor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/rename/RenameGrFieldProcessor.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring.rename; import com.intellij.psi.*; @@ -20,7 +6,10 @@ import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.searches.MethodReferencesSearch; import com.intellij.psi.search.searches.ReferencesSearch; -import com.intellij.psi.util.*; +import com.intellij.psi.util.PropertyUtilBase; +import com.intellij.psi.util.PsiFormatUtil; +import com.intellij.psi.util.PsiFormatUtilBase; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.listeners.RefactoringElementListener; import com.intellij.refactoring.rename.RenameJavaVariableProcessor; import com.intellij.refactoring.rename.UnresolvableCollisionUsageInfo; @@ -81,15 +70,21 @@ public class RenameGrFieldProcessor extends RenameJavaVariableProcessor { String newName, final UsageInfo[] usages, @Nullable RefactoringElementListener listener) throws IncorrectOperationException { + final Map oldResolvedRefs = ContainerUtil.newHashMap(); + for (UsageInfo usage : usages) { + final PsiReference ref = usage.getReference(); + if (ref instanceof GrReferenceExpression) { + oldResolvedRefs.put((GrReferenceExpression)ref, ref.resolve()); + } + } + GrField field = (GrField)psiElement; - Map handled = ContainerUtil.newHashMap(); for (UsageInfo usage : usages) { final PsiReference ref = usage.getReference(); if (ref instanceof GrReferenceExpression) { - PsiElement resolved = ref.resolve(); + PsiElement resolved = oldResolvedRefs.get(ref); ref.handleElementRename(getNewNameFromTransformations(resolved, newName)); - handled.put((GrReferenceExpression)ref, resolved); } else if (ref != null) { handleElementRename(newName, ref, field.getName()); @@ -99,8 +94,8 @@ public class RenameGrFieldProcessor extends RenameJavaVariableProcessor { field.setName(newName); PsiManager manager = psiElement.getManager(); - for (GrReferenceExpression expression : handled.keySet()) { - PsiElement oldResolved = handled.get(expression); + for (GrReferenceExpression expression : oldResolvedRefs.keySet()) { + PsiElement oldResolved = oldResolvedRefs.get(expression); if (oldResolved == null) continue; PsiElement resolved = expression.resolve(); if (resolved == null) continue; diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveFieldVsAccessorTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveFieldVsAccessorTest.groovy new file mode 100644 index 000000000000..22bdcc75132e --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveFieldVsAccessorTest.groovy @@ -0,0 +1,137 @@ +// 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.testFramework.LightProjectDescriptor +import groovy.transform.CompileStatic +import org.jetbrains.plugins.groovy.GroovyLightProjectDescriptor +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrAnonymousClassDefinition +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod + +import static org.jetbrains.plugins.groovy.util.ThrowingTransformation.disableTransformations + +@CompileStatic +class ResolveFieldVsAccessorTest extends GroovyResolveTestCase { + + final LightProjectDescriptor projectDescriptor = GroovyLightProjectDescriptor.GROOVY_LATEST + + void 'test implicit this'() { + disableTransformations testRootDisposable + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def implicitThis() { + prop + } +''', GrField + } + + void 'test explicit this'() { + disableTransformations testRootDisposable + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def explicitThis() { + this.prop + } +} +''', GrField + } + + void 'test qualified'() { + def method = resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def qualifiedUsage() { + new A().prop + } +} +''', GrMethod + assert method.name == 'getProp' + } + + void 'test inner class'() { + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def innerClass() { + new Runnable() { + void run() { + prop + } + } + } +} +''', GrMethod + } + + void 'test inner class explicit this'() { + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def innerExplicitThis = new Runnable() { + void run() { + println A.this.prop + } + } +} +''', GrMethod + } + + void 'test inner vs outer'() { + def method = resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } + + def innerProperty = new Runnable() { + def getProp() { "inner getter" } + + void run() { + println prop + } + } +''', GrMethod + assert method.containingClass instanceof GrAnonymousClassDefinition + } + + void 'test implicit super'() { + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } +} + +class B extends A { + def implicitSuper() { + prop + } +} +''', GrMethod + } + + void 'test explicit super'() { + resolveByText '''\ +class A { + def prop = "field" + def getProp() { "getter" } +} + +class B extends A { + def explicitSuper() { + super.prop + } +} +''', GrMethod + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolvePropertyTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolvePropertyTest.groovy index d18edbe2c56e..e1f3149cb829 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolvePropertyTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolvePropertyTest.groovy @@ -1,6 +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.psi.* @@ -52,6 +50,7 @@ class ResolvePropertyTest extends GroovyResolveTestCase { } void testField1() throws Exception { + disableTransformations testRootDisposable doTest("field1/A.groovy") } @@ -602,7 +601,7 @@ setFoo(2) } void testCommandExpressions() { - assertInstanceOf resolve("A.groovy"), GrField + assertInstanceOf resolve("A.groovy"), GrAccessorMethod } void testMetaClassIsNotResolvedWithMapQualifier() { @@ -770,7 +769,7 @@ class SomeMapClass extends HashMap { } void testResolveInsideWith1() { - def resolved = resolve('a.groovy', GrField) + def resolved = resolve('a.groovy', GrAccessorMethod) assertEquals(resolved.containingClass.name, 'B') } @@ -1339,66 +1338,6 @@ print A.a() ''', GrVariable } - void testPropertyVsAccessor() { - resolveByText('''\ -class ProductServiceImplTest { - BackendClient backendClient - - def setup() { - new ProductServiceImpl() { - protected BackendClient getBackendClient() { - return backendClient // <--- this expression is highlighted as member variable - } - } - } -} - -class BackendClient{} -class ProductServiceImpl{} -''', GrMethod) - } - - void testPropertyVsAccessor2() { - resolveByText('''\ -class ProductServiceImplTest { - def setup() { - new ProductServiceImpl() { - BackendClient backendClient - - protected BackendClient getBackendClient() { - return backendClient - } - } - } -} - -class BackendClient{} -class ProductServiceImpl{} -''', GrField) - } - - void testPropertyVsAccessor3() { - resolveByText('''\ -class ProductServiceImplTest { - BackendClient backendClient - - protected BackendClient getBackendClient() { - return backendClient - } - def setup() { - new ProductServiceImpl() { - def foo() { - return backendClient - } - } - } -} - -class BackendClient{} -class ProductServiceImpl{} -''', GrMethod) - } - void testTraitPublicField1() { resolveByText(''' trait T { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterInClosureTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterInClosureTest.groovy index c1ef8ee55fae..aebb784376cd 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterInClosureTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/introduceParameter/GrIntroduceParameterInClosureTest.groovy @@ -1,28 +1,17 @@ -/* - * Copyright 2000-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// 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.refactoring.introduceParameter import com.intellij.psi.impl.source.PostprocessReformattingAspect import com.intellij.refactoring.IntroduceParameterRefactoring import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase +import groovy.transform.CompileStatic import org.jetbrains.annotations.Nullable import org.jetbrains.plugins.groovy.util.TestUtils + /** * @author Max Medvedev */ +@CompileStatic class GrIntroduceParameterInClosureTest extends LightCodeInsightFixtureTestCase { protected String getBasePath() { return TestUtils.getTestDataPath() + "refactoring/introduceParameterInClosure/" @@ -82,7 +71,7 @@ class GrIntroduceParameterInClosureTest extends LightCodeInsightFixtureTestCase doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, true, null, false) } - void testDontReplaceWithGetter() { + void testGetter() { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_NONE, true, true, null, false) } diff --git a/plugins/groovy/testdata/refactoring/introduceParameterInClosure/DontReplaceWithGetter.groovy b/plugins/groovy/testdata/refactoring/introduceParameterInClosure/Getter.groovy similarity index 100% rename from plugins/groovy/testdata/refactoring/introduceParameterInClosure/DontReplaceWithGetter.groovy rename to plugins/groovy/testdata/refactoring/introduceParameterInClosure/Getter.groovy diff --git a/plugins/groovy/testdata/refactoring/introduceParameterInClosure/DontReplaceWithGetter_after.groovy b/plugins/groovy/testdata/refactoring/introduceParameterInClosure/Getter_after.groovy similarity index 61% rename from plugins/groovy/testdata/refactoring/introduceParameterInClosure/DontReplaceWithGetter_after.groovy rename to plugins/groovy/testdata/refactoring/introduceParameterInClosure/Getter_after.groovy index e008717a62a7..3552dea054a8 100644 --- a/plugins/groovy/testdata/refactoring/introduceParameterInClosure/DontReplaceWithGetter_after.groovy +++ b/plugins/groovy/testdata/refactoring/introduceParameterInClosure/Getter_after.groovy @@ -3,10 +3,10 @@ class X { def getFoo(){foo} - def bar = { final anObject -> + def bar = { final Object anObject -> print anObject } } final X x = new X() -print x.bar(x.@foo) +print x.bar(x.foo)