From 7042612210ee27b701d69a6d40fcd71203e14b91 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 22 Feb 2019 14:52:27 +0100 Subject: [PATCH] IPP: allow flipping method call with implicit this qualifier (IDEA-207785) --- .../FlipCommutativeMethodCallIntention.java | 61 ++++++--------- .../FlipCommutativeMethodCallPredicate.java | 74 +++++++++---------- .../ipp/commutative/flip/NoQualifier.java | 7 ++ .../commutative/flip/NoQualifier_after.java | 7 ++ .../commutative/flip/Substitution_after.java | 4 +- ...lipCommutativeMethodCallIntentionTest.java | 1 + 6 files changed, 76 insertions(+), 78 deletions(-) create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier_after.java diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntention.java index 9d97c15939b0..d08ee1bab198 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntention.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2018 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2019 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,6 @@ package com.siyeh.ipp.commutative; import com.intellij.psi.*; import com.siyeh.IntentionPowerPackBundle; -import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.CommentTracker; import com.siyeh.ig.psiutils.ParenthesesUtils; import com.siyeh.ipp.base.MutablyNamedIntention; @@ -30,12 +29,10 @@ public class FlipCommutativeMethodCallIntention extends MutablyNamedIntention { @Override protected String getTextForElement(PsiElement element) { final PsiMethodCallExpression call = (PsiMethodCallExpression)element; - final PsiReferenceExpression methodExpression = - call.getMethodExpression(); + final PsiReferenceExpression methodExpression = call.getMethodExpression(); @NonNls final String methodName = methodExpression.getReferenceName(); assert methodName != null; - if ("equals".equals(methodName) || - "equalsIgnoreCase".equals(methodName)) { + if ("equals".equals(methodName) || "equalsIgnoreCase".equals(methodName)) { return IntentionPowerPackBundle.message( "flip.commutative.method.call.intention.name", methodName); } @@ -52,39 +49,27 @@ public class FlipCommutativeMethodCallIntention extends MutablyNamedIntention { } @Override - public void processIntention(PsiElement element) { - final PsiMethodCallExpression call = (PsiMethodCallExpression)element; - final PsiReferenceExpression methodExpression = call.getMethodExpression(); - final String methodName = methodExpression.getReferenceName(); - final PsiExpression target = methodExpression.getQualifierExpression(); - if (target == null) { + public void processIntention(@NotNull PsiElement element) { + final PsiMethodCallExpression expression = (PsiMethodCallExpression)element; + final PsiExpressionList argumentList = expression.getArgumentList(); + final PsiExpression argument = argumentList.getExpressions()[0]; + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); + final PsiExpression strippedQualifier = ParenthesesUtils.stripParentheses(qualifier); + final PsiExpression strippedArgument = ParenthesesUtils.stripParentheses(argument); + if (strippedArgument == null) { return; } - final PsiExpressionList argumentList = call.getArgumentList(); - final PsiExpression arg = argumentList.getExpressions()[0]; - final PsiExpression strippedTarget = - ParenthesesUtils.stripParentheses(target); - if (strippedTarget == null) { - return; - } - final PsiExpression strippedArg = - ParenthesesUtils.stripParentheses(arg); - if (strippedArg == null) { - return; - } - final String callString; - if (ParenthesesUtils.getPrecedence(strippedArg) > - ParenthesesUtils.METHOD_CALL_PRECEDENCE) { - callString = '(' + strippedArg.getText() + ")." + methodName + '(' + - strippedTarget.getText() + ')'; - } - else { - callString = strippedArg.getText() + '.' + methodName + '(' + - strippedTarget.getText() + ')'; - } - CommentTracker commentTracker = new CommentTracker(); - commentTracker.markUnchanged(strippedArg); - commentTracker.markUnchanged(strippedTarget); - PsiReplacementUtil.replaceExpression(call, callString, commentTracker); + CommentTracker tracker = new CommentTracker(); + if (qualifier != null) tracker.grabComments(qualifier); + if (strippedQualifier != null) tracker.markUnchanged(strippedQualifier); + tracker.grabComments(argument); + tracker.markUnchanged(strippedArgument); + final PsiElement newArgument = strippedQualifier == null + ? JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText("this", expression) + : strippedQualifier.copy(); + methodExpression.setQualifierExpression(strippedArgument); + argument.replace(newArgument); + tracker.insertCommentsBefore(expression); } } \ No newline at end of file diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallPredicate.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallPredicate.java index 4aa1d592e975..b5d062dd4bc4 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallPredicate.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/commutative/FlipCommutativeMethodCallPredicate.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2005 Dave Griffith + * Copyright 2003-2019 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,58 +30,58 @@ class FlipCommutativeMethodCallPredicate implements PsiElementPredicate { if (ErrorUtil.containsError(element)) { return false; } - final PsiMethodCallExpression expression = - (PsiMethodCallExpression)element; + final PsiMethodCallExpression expression = (PsiMethodCallExpression)element; // do it only when there is just one argument. - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] args = argumentList.getExpressions(); - if (args.length != 1) { + final PsiExpression[] arguments = expression.getArgumentList().getExpressions(); + if (arguments.length != 1) { return false; } - final PsiReferenceExpression methodExpression = - expression.getMethodExpression(); - final PsiExpression qualifier = - methodExpression.getQualifierExpression(); - // make sure that there is a caller and a caller + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); + final PsiType callerType; if (qualifier == null) { - return false; + final PsiMethod method = expression.resolveMethod(); + if (method == null) { + return false; + } + final PsiClass aClass = method.getContainingClass(); + if (aClass == null) { + return false; + } + callerType = JavaPsiFacade.getElementFactory(aClass.getProject()).createType(aClass); + } + else { + callerType = qualifier.getType(); + if (!(callerType instanceof PsiClassType)) { + return false; + } } final String methodName = methodExpression.getReferenceName(); - // the logic is... - // if the argument takes a method of the same name with the caller - // as parameter then we can switch the argument and the caller. - final PsiType callerType = qualifier.getType(); - final PsiType argumentType = args[0].getType(); + final PsiType argumentType = arguments[0].getType(); if (!(argumentType instanceof PsiClassType)) { return false; } - if (!(callerType instanceof PsiClassType)) { - return false; + if (callerType.equals(argumentType)) { + return true; } final PsiClassType.ClassResolveResult resolveResult = ((PsiClassType)argumentType).resolveGenerics(); final PsiClass argumentClass = resolveResult.getElement(); if (argumentClass == null) { return false; } - final PsiMethod[] methods = - argumentClass.findMethodsByName(methodName, true); + final PsiMethod[] methods = argumentClass.findMethodsByName(methodName, true); for (final PsiMethod testMethod : methods) { - final String testMethodName = testMethod.getName(); - if (testMethodName.equals(methodName)) { - final PsiParameterList parameterList = - testMethod.getParameterList(); - final PsiParameter[] parameters = parameterList.getParameters(); - if (parameters.length == 1) { - final PsiParameter parameter = parameters[0]; - final PsiClass containingClass = testMethod.getContainingClass(); - if (containingClass != null) { - final PsiSubstitutor substitutor = - TypeConversionUtil.getClassSubstitutor(containingClass, argumentClass, resolveResult.getSubstitutor()); - if (substitutor != null) { - final PsiType type = substitutor.substitute(parameter.getType()); - if (type != null && type.isAssignableFrom(callerType)) { - return true; - } + final PsiParameterList parameterList = testMethod.getParameterList(); + if (parameterList.getParametersCount() == 1) { + final PsiParameter parameter = parameterList.getParameters()[0]; + final PsiClass containingClass = testMethod.getContainingClass(); + if (containingClass != null) { + final PsiSubstitutor substitutor = + TypeConversionUtil.getClassSubstitutor(containingClass, argumentClass, resolveResult.getSubstitutor()); + if (substitutor != null) { + final PsiType type = substitutor.substitute(parameter.getType()); + if (type != null && type.isAssignableFrom(callerType)) { + return true; } } } diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier.java new file mode 100644 index 000000000000..75dfec380649 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier.java @@ -0,0 +1,7 @@ +class X { + + void foo(X x) { + foo((/*1*/(new X()))); + } + +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier_after.java new file mode 100644 index 000000000000..c25de85be171 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/NoQualifier_after.java @@ -0,0 +1,7 @@ +class X { + + void foo(X x) { + /*1*/new X().foo(this); + } + +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/Substitution_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/Substitution_after.java index 4a9374e2b40e..fc837696856a 100644 --- a/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/Substitution_after.java +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/commutative/flip/Substitution_after.java @@ -2,9 +2,7 @@ class A { void foo(A a){} void bar(B b, B b1) { - /*in method call*/ - /*comment in arg*/ - b1.foo(b);//end line comment + b1./*in method call*/foo(b/*comment in arg*/);//end line comment } } diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntentionTest.java index 5b4e5ed59b6f..9faa4e2569d5 100644 --- a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntentionTest.java +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/commutative/FlipCommutativeMethodCallIntentionTest.java @@ -20,6 +20,7 @@ import com.siyeh.ipp.IPPTestCase; public class FlipCommutativeMethodCallIntentionTest extends IPPTestCase { public void testSubstitution() { doTest(); } + public void testNoQualifier() { doTest(); } @Override protected String getIntentionName() {