From fb9a5e8ff7a7ee77e76f6c2548d0f292c8c92f09 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Mon, 12 Jan 2015 21:13:33 +0100 Subject: [PATCH] extract method parameters suggester: change method copy context so calls inside method body could be resolved (aClass..getTypeParameterList() passed inside MethodElement.copyElement && aClass.processDeclarations implementations together cause that method calls to same class members can't be resolved); teach DuplicatesFinder to work on non-physical elements with correct context --- .../util/duplicates/DuplicatesFinder.java | 22 +++++++++---------- .../ExtractMethodSignatureSuggester.java | 7 +++++- ...tChangeSignatureCallToSameClassMethod.java | 12 ++++++++++ ...eSignatureCallToSameClassMethod_after.java | 15 +++++++++++++ .../refactoring/ExtractMethodTest.java | 4 ++++ 5 files changed, 48 insertions(+), 12 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java index 38c0cf4abbf4..b6b076f9064c 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java @@ -192,7 +192,7 @@ public class DuplicatesFinder { final PsiType patternType = ((PsiExpression)myPattern[0]).getType(); final PsiType candidateType = candidateExpression.getType(); PsiSubstitutor substitutor = PsiSubstitutor.EMPTY; - final PsiMethod method = PsiTreeUtil.getParentOfType(myPattern[0], PsiMethod.class); + final PsiMethod method = PsiTreeUtil.getContextOfType(myPattern[0], PsiMethod.class); if (method != null) { final PsiResolveHelper resolveHelper = JavaPsiFacade.getInstance(candidate.getProject()).getResolveHelper(); substitutor = resolveHelper.inferTypeArguments(method.getTypeParameters(), new PsiType[]{patternType}, @@ -435,7 +435,7 @@ public class DuplicatesFinder { final PsiExpression patternQualifier = patternRefExpr.getQualifierExpression(); final PsiExpression candidateQualifier = candidateRefExpr.getQualifierExpression(); if (patternQualifier == null) { - PsiClass contextClass = PsiTreeUtil.getParentOfType(pattern, PsiClass.class); + PsiClass contextClass = PsiTreeUtil.getContextOfType(pattern, PsiClass.class); if (candidateQualifier instanceof PsiReferenceExpression) { final PsiElement resolved = ((PsiReferenceExpression)candidateQualifier).resolve(); if (resolved instanceof PsiClass && contextClass != null && InheritanceUtil.isInheritorOrSelf(contextClass, (PsiClass)resolved, true)) { @@ -448,7 +448,7 @@ public class DuplicatesFinder { if (patternQualifier instanceof PsiThisExpression) { final PsiJavaCodeReferenceElement qualifier = ((PsiThisExpression)patternQualifier).getQualifier(); if (candidate instanceof PsiReferenceExpression) { - PsiElement contextClass = qualifier == null ? PsiTreeUtil.getParentOfType(pattern, PsiClass.class) : qualifier.resolve(); + PsiElement contextClass = qualifier == null ? PsiTreeUtil.getContextOfType(pattern, PsiClass.class) : qualifier.resolve(); return contextClass instanceof PsiClass && match.registerInstanceExpression(((PsiReferenceExpression)candidate).getQualifierExpression(), (PsiClass)contextClass); } @@ -473,7 +473,7 @@ public class DuplicatesFinder { } else if (patternQualifier instanceof PsiReferenceExpression) { final PsiElement resolved = ((PsiReferenceExpression)patternQualifier).resolve(); if (resolved instanceof PsiClass) { - final PsiClass classContext = PsiTreeUtil.getParentOfType(candidate, PsiClass.class); + final PsiClass classContext = PsiTreeUtil.getContextOfType(candidate, PsiClass.class); if (classContext != null && InheritanceUtil.isInheritorOrSelf(classContext, (PsiClass)resolved, true)) { return true; } @@ -489,31 +489,31 @@ public class DuplicatesFinder { } else { if (patternQualifier instanceof PsiThisExpression && candidateQualifier instanceof PsiThisExpression) { final PsiJavaCodeReferenceElement thisPatternQualifier = ((PsiThisExpression)patternQualifier).getQualifier(); - final PsiElement patternContextClass = thisPatternQualifier == null ? PsiTreeUtil.getParentOfType(patternQualifier, PsiClass.class) : thisPatternQualifier.resolve(); + final PsiElement patternContextClass = thisPatternQualifier == null ? PsiTreeUtil.getContextOfType(patternQualifier, PsiClass.class) : thisPatternQualifier.resolve(); final PsiJavaCodeReferenceElement thisCandidateQualifier = ((PsiThisExpression)candidateQualifier).getQualifier(); - final PsiElement candidateContextClass = thisCandidateQualifier == null ? PsiTreeUtil.getParentOfType(candidateQualifier, PsiClass.class) : thisCandidateQualifier.resolve(); + final PsiElement candidateContextClass = thisCandidateQualifier == null ? PsiTreeUtil.getContextOfType(candidateQualifier, PsiClass.class) : thisCandidateQualifier.resolve(); return patternContextClass == candidateContextClass; } } } } else if (pattern instanceof PsiThisExpression) { final PsiJavaCodeReferenceElement qualifier = ((PsiThisExpression)pattern).getQualifier(); - final PsiElement contextClass = qualifier == null ? PsiTreeUtil.getParentOfType(pattern, PsiClass.class) : qualifier.resolve(); + final PsiElement contextClass = qualifier == null ? PsiTreeUtil.getContextOfType(pattern, PsiClass.class) : qualifier.resolve(); if (candidate instanceof PsiReferenceExpression) { final PsiElement parent = candidate.getParent(); return parent instanceof PsiReferenceExpression && contextClass instanceof PsiClass && match.registerInstanceExpression(((PsiReferenceExpression)parent).getQualifierExpression(), (PsiClass)contextClass); } else if (candidate instanceof PsiThisExpression) { final PsiJavaCodeReferenceElement candidateQualifier = ((PsiThisExpression)candidate).getQualifier(); - final PsiElement candidateContextClass = candidateQualifier == null ? PsiTreeUtil.getParentOfType(candidate, PsiClass.class) : candidateQualifier.resolve(); + final PsiElement candidateContextClass = candidateQualifier == null ? PsiTreeUtil.getContextOfType(candidate, PsiClass.class) : candidateQualifier.resolve(); return contextClass == candidateContextClass; } } else if (pattern instanceof PsiSuperExpression) { final PsiJavaCodeReferenceElement qualifier = ((PsiSuperExpression)pattern).getQualifier(); - final PsiElement contextClass = qualifier == null ? PsiTreeUtil.getParentOfType(pattern, PsiClass.class) : qualifier.resolve(); + final PsiElement contextClass = qualifier == null ? PsiTreeUtil.getContextOfType(pattern, PsiClass.class) : qualifier.resolve(); if (candidate instanceof PsiSuperExpression) { final PsiJavaCodeReferenceElement candidateQualifier = ((PsiSuperExpression)candidate).getQualifier(); - return contextClass == (candidateQualifier != null ? candidateQualifier.resolve() : PsiTreeUtil.getParentOfType(candidate, PsiClass.class)); + return contextClass == (candidateQualifier != null ? candidateQualifier.resolve() : PsiTreeUtil.getContextOfType(candidate, PsiClass.class)); } } @@ -599,7 +599,7 @@ public class DuplicatesFinder { return match.registerReturnValue(new ConditionalReturnStatementValue(returnValue)); } else { - final PsiElement classOrLambda = PsiTreeUtil.getParentOfType(returnValue, PsiClass.class, PsiLambdaExpression.class); + final PsiElement classOrLambda = PsiTreeUtil.getContextOfType(returnValue, PsiClass.class, PsiLambdaExpression.class); final PsiElement commonParent = PsiTreeUtil.findCommonParent(match.getMatchStart(), match.getMatchEnd()); if (classOrLambda == null || !PsiTreeUtil.isAncestor(commonParent, classOrLambda, false)) { if (returnValue != null && !match.registerReturnValue(ReturnStatementReturnValue.INSTANCE)) return false; //do not register return value for return; statement diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java index ea0ba38520b3..c682e11e6c86 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java @@ -81,7 +81,9 @@ public class ExtractMethodSignatureSuggester { myProject = project; myElementFactory = JavaPsiFacade.getElementFactory(project); - myExtractedMethod = (PsiMethod)extractedMethod.copy(); + final PsiClass containingClass = extractedMethod.getContainingClass(); + LOG.assertTrue(containingClass != null); + myExtractedMethod = myElementFactory.createMethodFromText(extractedMethod.getText(), containingClass.getLBrace()); myMethodCall = methodCall; myVariableData = variableDatum; } @@ -147,6 +149,9 @@ public class ExtractMethodSignatureSuggester { if (duplicates != null && !duplicates.isEmpty()) { restoreRenamedParams(copies); inlineSameArguments(method, copies, variables, duplicates); + if (!myMethodCall.isValid()) { + return null; + } myMethodCall = (PsiMethodCallExpression)myMethodCall.copy(); for (PsiExpression expression : copies) { myMethodCall.getArgumentList().add(expression); diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod.java new file mode 100644 index 000000000000..6f8f57f23dd8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod.java @@ -0,0 +1,12 @@ +class B { + void readFile(String s, Class c) {} + + { + + readFile("foo", B.class); + this.readFile("foo", B.class); + + readFile("bar", B.class); + this.readFile("bar", B.class); + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod_after.java new file mode 100644 index 000000000000..27589a380aa8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureCallToSameClassMethod_after.java @@ -0,0 +1,15 @@ +class B { + void readFile(String s, Class c) {} + + { + + newMethod("foo"); + + newMethod("bar"); + } + + private void newMethod(String foo) { + readFile(foo, B.class); + this.readFile(foo, B.class); + } +} diff --git a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java index b4ea5f056928..00359a356b63 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java @@ -642,6 +642,10 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testSuggestChangeSignatureCallToSameClassMethod() throws Exception { + doDuplicatesTest(); + } + public void testSuggestChangeSignatureInitialParameterUnused() throws Exception { doDuplicatesTest(); }