From 21d1b968b014e019096e40438b5007d55338a306 Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Thu, 3 Dec 2015 10:42:59 +0300 Subject: [PATCH] type migration: RootTypeConversionRule delete unnecessarily qualified static method qualifier IDEA-CR-6898 --- .../rules/RootTypeConversionRule.java | 16 +- .../refactoring/TypeMigrationTest.java | 6 + .../after/Test.items | 11 + .../after/test.java | 10 + .../before/test.java | 10 + ...ssarilyQualifiedStaticUsageInspection.java | 191 +++++++++--------- 6 files changed, 145 insertions(+), 99 deletions(-) create mode 100644 java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/Test.items create mode 100644 java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/before/test.java diff --git a/java/java-impl/src/com/intellij/refactoring/typeMigration/rules/RootTypeConversionRule.java b/java/java-impl/src/com/intellij/refactoring/typeMigration/rules/RootTypeConversionRule.java index 2894795736ac..df9e42d8f814 100644 --- a/java/java-impl/src/com/intellij/refactoring/typeMigration/rules/RootTypeConversionRule.java +++ b/java/java-impl/src/com/intellij/refactoring/typeMigration/rules/RootTypeConversionRule.java @@ -23,6 +23,8 @@ import com.intellij.psi.util.TypeConversionUtil; import com.intellij.refactoring.typeMigration.TypeConversionDescriptorBase; import com.intellij.refactoring.typeMigration.TypeMigrationLabeler; import com.intellij.util.IncorrectOperationException; +import com.siyeh.ig.style.UnnecessarilyQualifiedStaticUsageInspection; +import com.siyeh.ig.style.UnnecessarilyQualifiedStaticallyImportedElementInspection; import org.jetbrains.annotations.NotNull; /** @@ -141,16 +143,20 @@ public class RootTypeConversionRule extends TypeConversionRule { final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; final PsiExpression qualifierExpression = methodCallExpression.getMethodExpression().getQualifierExpression(); final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(expression.getProject()); - final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(expression.getProject()); + final PsiMethodCallExpression newMethodCall; if (qualifierExpression != null) { - codeStyleManager.shortenClassReferences(qualifierExpression.replace(elementFactory.createExpressionFromText(myTargetClassQName, expression))); - return expression; + JavaCodeStyleManager.getInstance(expression.getProject()) + .shortenClassReferences(qualifierExpression.replace(elementFactory.createExpressionFromText(myTargetClassQName, expression))); + newMethodCall = methodCallExpression; } else { - final PsiElement replacedExpression = expression.replace( + newMethodCall = (PsiMethodCallExpression)expression.replace( elementFactory.createExpressionFromText(myTargetClassQName + "." + expression.getText(), expression)); - return (PsiExpression)codeStyleManager.shortenClassReferences(replacedExpression); } + if (UnnecessarilyQualifiedStaticUsageInspection.isUnnecessarilyQualifiedAccess(newMethodCall.getMethodExpression(), false, false, false)) { + newMethodCall.getMethodExpression().getQualifierExpression().delete(); + } + return newMethodCall; } @Override diff --git a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java index b1ed3f42396e..0cd0d3a2b70a 100644 --- a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java +++ b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java @@ -896,6 +896,12 @@ public class TypeMigrationTest extends TypeMigrationTestBase { myFactory.createTypeFromText("Test.AnInterface2", null)); } + public void testRemoveStaticMethodQualifier() { + doTestFirstParamType("meth", + myFactory.createTypeFromText("java.lang.Long", null), + myFactory.createTypeFromText("Test", null)); + } + public void testPropagateViaEquals() { doTestFirstParamType("meth", myFactory.createTypeFromText("java.lang.String", null), myFactory.createTypeFromText("java.lang.Long", null)); } diff --git a/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/Test.items new file mode 100644 index 000000000000..c1191fec5eb7 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/Test.items @@ -0,0 +1,11 @@ +Types: +PsiMethod:meth : Test +PsiParameter:l : Test +PsiReferenceExpression:l : Test +PsiReferenceExpression:l : Test + +Conversions: +Long.valueOf("256") -> Static method qualifier conversion -> Test + +New expression type changes: +Fails: diff --git a/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/test.java new file mode 100644 index 000000000000..5bbf13462764 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/after/test.java @@ -0,0 +1,10 @@ +class Test { + public Test meth(Test l) { + l = valueOf("256"); + return l; + } + + public static Test valueOf(String s) { + return null; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/before/test.java new file mode 100644 index 000000000000..5a315fe308b8 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/removeStaticMethodQualifier/before/test.java @@ -0,0 +1,10 @@ +class Test { + public Long meth(Long l) { + l = Long.valueOf("256"); + return l; + } + + public static Test valueOf(String s) { + return null; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessarilyQualifiedStaticUsageInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessarilyQualifiedStaticUsageInspection.java index 9ac7bf528a95..0eb7869de33e 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessarilyQualifiedStaticUsageInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessarilyQualifiedStaticUsageInspection.java @@ -121,7 +121,7 @@ public class UnnecessarilyQualifiedStaticUsageInspection extends BaseInspection if (qualifier == null) { return; } - if (!isUnnecessarilyQualifiedAccess(reference)) { + if (!isUnnecessarilyQualifiedAccess(reference, m_ignoreStaticAccessFromStaticContext, m_ignoreStaticFieldAccesses, m_ignoreStaticMethodCalls)) { return; } registerError(qualifier, ProblemHighlightType.LIKE_UNUSED_SYMBOL, reference); @@ -131,102 +131,105 @@ public class UnnecessarilyQualifiedStaticUsageInspection extends BaseInspection public void visitReferenceExpression(PsiReferenceExpression expression) { visitReferenceElement(expression); } + } - private boolean isUnnecessarilyQualifiedAccess(@NotNull PsiJavaCodeReferenceElement referenceElement) { - if (referenceElement instanceof PsiMethodReferenceExpression) { - return false; - } - final PsiElement parent = referenceElement.getParent(); - if (parent instanceof PsiImportStatementBase) { - return false; - } - final PsiElement qualifierElement = referenceElement.getQualifier(); - if (!(qualifierElement instanceof PsiJavaCodeReferenceElement)) { - return false; - } - final PsiJavaCodeReferenceElement qualifier = (PsiJavaCodeReferenceElement)qualifierElement; - if (isGenericReference(referenceElement, qualifier)) { - return false; - } - final PsiElement target = referenceElement.resolve(); - if ((!(target instanceof PsiField) || m_ignoreStaticFieldAccesses) && (!(target instanceof PsiMethod) || m_ignoreStaticMethodCalls)) { - return false; - } - if (m_ignoreStaticAccessFromStaticContext) { - final PsiMember containingMember = PsiTreeUtil.getParentOfType(referenceElement, PsiMember.class); - if (containingMember != null && !containingMember.hasModifierProperty(PsiModifier.STATIC)) { - return false; - } - } - final String referenceName = referenceElement.getReferenceName(); - if (referenceName == null) { - return false; - } - final PsiElement resolvedQualifier = qualifier.resolve(); - if (!(resolvedQualifier instanceof PsiClass)) { - return false; - } - final PsiClass containingClass = PsiTreeUtil.getParentOfType(referenceElement, PsiClass.class); - final PsiClass qualifyingClass = (PsiClass)resolvedQualifier; - if (containingClass == null || !PsiTreeUtil.isAncestor(qualifyingClass, containingClass, false)) { - return false; - } - final Project project = referenceElement.getProject(); - final JavaPsiFacade manager = JavaPsiFacade.getInstance(project); - final PsiResolveHelper resolveHelper = manager.getResolveHelper(); - final PsiMember member = (PsiMember)target; - final PsiClass memberClass; - if (target instanceof PsiField) { - final PsiVariable variable = resolveHelper.resolveReferencedVariable(referenceName, referenceElement); - if (variable == null || !variable.equals(member)) { - return false; - } - final TextRange referenceElementTextRange = referenceElement.getTextRange(); - if (referenceElementTextRange == null) { - return false; - } - final TextRange variableTextRange = variable.getTextRange(); - if (variableTextRange == null) { - return false; - } - //illegal forward ref - if (referenceElementTextRange.getStartOffset() < variableTextRange.getEndOffset()) { - return false; - } - final PsiMember memberVariable = (PsiMember)variable; - memberClass = memberVariable.getContainingClass(); - } - else if (target instanceof PsiClass) { - final PsiClass aClass = resolveHelper.resolveReferencedClass(referenceName, referenceElement); - if (aClass == null || !aClass.equals(member)) { - return false; - } - memberClass = aClass.getContainingClass(); - } - else { - return isMethodAccessibleWithoutQualifier(referenceElement, qualifyingClass); - } - return resolvedQualifier.equals(memberClass); - } - - private boolean isMethodAccessibleWithoutQualifier(PsiJavaCodeReferenceElement referenceElement, PsiClass qualifyingClass) { - final String referenceName = referenceElement.getReferenceName(); - if (referenceName == null) { - return false; - } - PsiClass containingClass = ClassUtils.getContainingClass(referenceElement); - while (containingClass != null) { - final PsiMethod[] methods = containingClass.findMethodsByName(referenceName, true); - for (final PsiMethod method : methods) { - final String name = method.getName(); - if (referenceName.equals(name)) { - return containingClass.equals(qualifyingClass); - } - } - containingClass = ClassUtils.getContainingClass(containingClass); - } + public static boolean isUnnecessarilyQualifiedAccess(@NotNull PsiJavaCodeReferenceElement referenceElement, + boolean ignoreStaticAccessFromStaticContext, + boolean ignoreStaticFieldAccesses, + boolean ignoreStaticMethodCalls) { + if (referenceElement instanceof PsiMethodReferenceExpression) { return false; } + final PsiElement parent = referenceElement.getParent(); + if (parent instanceof PsiImportStatementBase) { + return false; + } + final PsiElement qualifierElement = referenceElement.getQualifier(); + if (!(qualifierElement instanceof PsiJavaCodeReferenceElement)) { + return false; + } + final PsiJavaCodeReferenceElement qualifier = (PsiJavaCodeReferenceElement)qualifierElement; + if (isGenericReference(referenceElement, qualifier)) { + return false; + } + final PsiElement target = referenceElement.resolve(); + if ((!(target instanceof PsiField) || ignoreStaticFieldAccesses) && (!(target instanceof PsiMethod) || ignoreStaticMethodCalls)) { + return false; + } + if (ignoreStaticAccessFromStaticContext) { + final PsiMember containingMember = PsiTreeUtil.getParentOfType(referenceElement, PsiMember.class); + if (containingMember != null && !containingMember.hasModifierProperty(PsiModifier.STATIC)) { + return false; + } + } + final String referenceName = referenceElement.getReferenceName(); + if (referenceName == null) { + return false; + } + final PsiElement resolvedQualifier = qualifier.resolve(); + if (!(resolvedQualifier instanceof PsiClass)) { + return false; + } + final PsiClass containingClass = PsiTreeUtil.getParentOfType(referenceElement, PsiClass.class); + final PsiClass qualifyingClass = (PsiClass)resolvedQualifier; + if (containingClass == null || !PsiTreeUtil.isAncestor(qualifyingClass, containingClass, false)) { + return false; + } + final Project project = referenceElement.getProject(); + final JavaPsiFacade manager = JavaPsiFacade.getInstance(project); + final PsiResolveHelper resolveHelper = manager.getResolveHelper(); + final PsiMember member = (PsiMember)target; + final PsiClass memberClass; + if (target instanceof PsiField) { + final PsiVariable variable = resolveHelper.resolveReferencedVariable(referenceName, referenceElement); + if (variable == null || !variable.equals(member)) { + return false; + } + final TextRange referenceElementTextRange = referenceElement.getTextRange(); + if (referenceElementTextRange == null) { + return false; + } + final TextRange variableTextRange = variable.getTextRange(); + if (variableTextRange == null) { + return false; + } + //illegal forward ref + if (referenceElementTextRange.getStartOffset() < variableTextRange.getEndOffset()) { + return false; + } + final PsiMember memberVariable = (PsiMember)variable; + memberClass = memberVariable.getContainingClass(); + } + else if (target instanceof PsiClass) { + final PsiClass aClass = resolveHelper.resolveReferencedClass(referenceName, referenceElement); + if (aClass == null || !aClass.equals(member)) { + return false; + } + memberClass = aClass.getContainingClass(); + } + else { + return isMethodAccessibleWithoutQualifier(referenceElement, qualifyingClass); + } + return resolvedQualifier.equals(memberClass); + } + + private static boolean isMethodAccessibleWithoutQualifier(PsiJavaCodeReferenceElement referenceElement, PsiClass qualifyingClass) { + final String referenceName = referenceElement.getReferenceName(); + if (referenceName == null) { + return false; + } + PsiClass containingClass = ClassUtils.getContainingClass(referenceElement); + while (containingClass != null) { + final PsiMethod[] methods = containingClass.findMethodsByName(referenceName, true); + for (final PsiMethod method : methods) { + final String name = method.getName(); + if (referenceName.equals(name)) { + return containingClass.equals(qualifyingClass); + } + } + containingClass = ClassUtils.getContainingClass(containingClass); + } + return false; } static boolean isGenericReference(PsiJavaCodeReferenceElement referenceElement, PsiJavaCodeReferenceElement qualifierElement) {