From cbdda56f99f3ceddf49a8de1d7af0bf3d4db373b Mon Sep 17 00:00:00 2001 From: Georgii Ustinov Date: Thu, 12 Oct 2023 09:43:18 +0300 Subject: [PATCH] IDEA-332957 GuavaInspection false positive review refactor GitOrigin-RevId: 32b819439b341b8b3170ae304304c339cfc9e753 --- .../siyeh/ig/psiutils/ExpressionUtils.java | 11 ---- .../inspections/GuavaInspection.java | 23 ++++---- .../guava/GuavaPredicateConversionRule.java | 56 ++++++++++++------- .../inspections/GuavaInspectionTest.java | 6 ++ .../inspections/guava/predicatesVarArg1.java | 13 +++++ .../guava/predicatesVarArg1_after.java | 12 ++++ .../inspections/guava/predicatesVarArg2.java | 13 +++++ .../guava/predicatesVarArg2_after.java | 12 ++++ .../inspections/guava/predicatesVarArg3.java | 13 +++++ .../guava/predicatesVarArg3_after.java | 12 ++++ 10 files changed, 130 insertions(+), 41 deletions(-) create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg1.java create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg1_after.java create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg2.java create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg2_after.java create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg3.java create mode 100644 java/typeMigration/testData/inspections/guava/predicatesVarArg3_after.java diff --git a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/ExpressionUtils.java index e7c3b61df72b..9d7e198b56a7 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -91,17 +91,6 @@ public final class ExpressionUtils { public static PsiExpression getFirstExpressionInList(@Nullable PsiExpressionList expressionList) { return PsiTreeUtil.getChildOfType(expressionList, PsiExpression.class); } - - public static int getExpressionPosition(PsiExpressionList expressionList, PsiExpression expression) { - var expressions = expressionList.getExpressions(); - for (int i = 0; i < expressions.length; ++i) { - if (expressions[i].equals(expression)) { - return i; - } - } - return -1; - } - @Nullable public static PsiExpression getOnlyExpressionInList(@Nullable PsiExpressionList expressionList) { return ControlFlowUtils.getOnlyChildOfType(expressionList, PsiExpression.class); diff --git a/java/typeMigration/src/com/intellij/refactoring/typeMigration/inspections/GuavaInspection.java b/java/typeMigration/src/com/intellij/refactoring/typeMigration/inspections/GuavaInspection.java index 810f9a0f96af..4505e3cfd885 100644 --- a/java/typeMigration/src/com/intellij/refactoring/typeMigration/inspections/GuavaInspection.java +++ b/java/typeMigration/src/com/intellij/refactoring/typeMigration/inspections/GuavaInspection.java @@ -114,12 +114,8 @@ public final class GuavaInspection extends AbstractBaseJavaLocalInspectionTool { checkPredicatesUtilityMethod(expression); } - private void checkPredicatesUtilityMethod(PsiMethodCallExpression expression) { - if (GuavaPredicateConversionRule.isPredicates(expression) && ( - isReceiverOfEnclosingCallAFluentIterable(expression) || - GuavaPredicateConversionRule.isEnclosingCallPredicate(expression) || - GuavaPredicateConversionRule.isPredicateConvertibleInsideEnclosingMethod(expression) - )) { + private void checkPredicatesUtilityMethod(@NotNull PsiMethodCallExpression expression) { + if (GuavaPredicateConversionRule.isPredicates(expression) && isPossibleToConvertPredicate(expression)) { final PsiClassType initialType = (PsiClassType)expression.getType(); if (initialType == null) return; PsiClassType targetType = createTargetType(initialType); @@ -132,6 +128,16 @@ public final class GuavaInspection extends AbstractBaseJavaLocalInspectionTool { } } + private static boolean isPossibleToConvertPredicate(@NotNull PsiMethodCallExpression expression) { + PsiMethodCallExpression enclosingMethodCallExpression = + PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class); + + return enclosingMethodCallExpression == null || + isReceiverOfEnclosingCallAFluentIterable(enclosingMethodCallExpression) || + GuavaPredicateConversionRule.isEnclosingCallAPredicate(enclosingMethodCallExpression) || + GuavaPredicateConversionRule.isPredicateConvertibleInsideEnclosingMethod(expression, enclosingMethodCallExpression); + } + private void checkFluentIterableGenerationMethod(PsiMethodCallExpression expression) { if (!checkChains) return; if (!isFluentIterableFromCall(expression)) return; @@ -191,10 +197,7 @@ public final class GuavaInspection extends AbstractBaseJavaLocalInspectionTool { return result; } - public static boolean isReceiverOfEnclosingCallAFluentIterable(PsiMethodCallExpression expression) { - PsiMethodCallExpression enclosingMethodCallExpression = - PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class); - if (enclosingMethodCallExpression == null) return false; + public static boolean isReceiverOfEnclosingCallAFluentIterable(@NotNull PsiMethodCallExpression enclosingMethodCallExpression) { PsiMethod method = enclosingMethodCallExpression.resolveMethod(); if (method == null) return false; return isFluentIterable(method.getContainingClass()); diff --git a/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/guava/GuavaPredicateConversionRule.java b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/guava/GuavaPredicateConversionRule.java index eae776fa3338..d4c075052fb6 100644 --- a/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/guava/GuavaPredicateConversionRule.java +++ b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/guava/GuavaPredicateConversionRule.java @@ -8,13 +8,11 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.typeMigration.TypeConversionDescriptorBase; import com.intellij.refactoring.typeMigration.TypeEvaluator; import com.intellij.refactoring.typeMigration.TypeMigrationLabeler; -import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Arrays; import java.util.Collections; -import java.util.Objects; import java.util.Set; /** @@ -73,26 +71,31 @@ public class GuavaPredicateConversionRule extends GuavaLambdaConversionRule { return false; } - public static boolean isEnclosingCallPredicate(PsiMethodCallExpression expression) { - PsiMethodCallExpression enclosingMethodCallExpression = - PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class); - if (enclosingMethodCallExpression == null) return false; + public static boolean isEnclosingCallAPredicate(@NotNull PsiMethodCallExpression enclosingMethodCallExpression) { return isPredicates(enclosingMethodCallExpression); } + private static int getExpressionPosition(@NotNull PsiExpressionList expressionList, @NotNull PsiExpression expression) { + var expressions = expressionList.getExpressions(); + for (int i = 0; i < expressions.length; ++i) { + if (expressions[i].equals(expression)) { + return i; + } + } + return -1; + } - public static boolean isPredicateConvertibleInsideEnclosingMethod(PsiMethodCallExpression methodCallExpression) { - PsiExpressionList expressionList = PsiTreeUtil.getParentOfType(methodCallExpression, PsiExpressionList.class); - PsiMethodCallExpression enclosingMethodCallExpression = - PsiTreeUtil.getParentOfType(methodCallExpression, PsiMethodCallExpression.class); - if (expressionList == null || enclosingMethodCallExpression == null) return true; + public static boolean isPredicateConvertibleInsideEnclosingMethod(@NotNull PsiMethodCallExpression innerMethodCallExpression, + @NotNull PsiMethodCallExpression enclosingMethodCallExpression) { + PsiExpressionList expressionList = PsiTreeUtil.getParentOfType(innerMethodCallExpression, PsiExpressionList.class); + if (expressionList == null) return false; PsiMethod enclosingMethod = enclosingMethodCallExpression.resolveMethod(); - if (enclosingMethod == null) return true; + if (enclosingMethod == null) return false; PsiClass aClass = enclosingMethod.getContainingClass(); - if (aClass == null) return true; - int position = ExpressionUtils.getExpressionPosition(expressionList, methodCallExpression); + if (aClass == null) return false; + int position = getExpressionPosition(expressionList, innerMethodCallExpression); return Arrays.stream(aClass.findMethodsByName(enclosingMethod.getName(), true)) - .filter(method -> PsiUtil.isMemberAccessibleAt(method, methodCallExpression)) + .filter(method -> PsiUtil.isMemberAccessibleAt(method, innerMethodCallExpression)) .anyMatch(method -> areExpressionsConvertibleToMethodParameters(expressionList.getExpressionTypes(), method.getParameterList().getParameters(), position)); @@ -101,20 +104,33 @@ public class GuavaPredicateConversionRule extends GuavaLambdaConversionRule { private static boolean areExpressionsConvertibleToMethodParameters(PsiType[] expressionTypes, PsiParameter[] parameters, int predicateExpressionPosition) { - if (parameters.length != expressionTypes.length) { + if (parameters.length == 0 || (parameters.length > expressionTypes.length && !parameters[expressionTypes.length].isVarArgs())) { return false; } boolean isJavaPredicatePresented = false; - for (int i = 0; i < parameters.length; ++i) { - PsiType parameterType = Objects.requireNonNull(parameters[i].getType()); - if (!parameterType.isConvertibleFrom(expressionTypes[i])) return false; + int parametersLastIndex = parameters.length - 1; + for (int expressionTypeIndex = 0; expressionTypeIndex < expressionTypes.length; ++expressionTypeIndex) { + PsiType parameterType; + if (expressionTypeIndex < parameters.length) { + if (parameters[expressionTypeIndex].isVarArgs()) { + parameterType = ((PsiArrayType)parameters[expressionTypeIndex].getType()).getComponentType(); + } + else { + parameterType = parameters[expressionTypeIndex].getType(); + } + } + else { + if (!parameters[parametersLastIndex].isVarArgs()) return false; + parameterType = ((PsiArrayType)parameters[parametersLastIndex].getType()).getComponentType(); + } + if (!parameterType.isConvertibleFrom(expressionTypes[expressionTypeIndex])) return false; PsiClass parameterClass = PsiTypesUtil.getPsiClass(parameterType); if (parameterClass == null) continue; String qualifiedClassName = parameterClass.getQualifiedName(); if (qualifiedClassName == null) continue; - if (i == predicateExpressionPosition && qualifiedClassName.equals("java.util.function.Predicate")) { + if (expressionTypeIndex == predicateExpressionPosition && qualifiedClassName.equals(CommonClassNames.JAVA_UTIL_FUNCTION_PREDICATE)) { isJavaPredicatePresented = true; } } diff --git a/java/typeMigration/test/com/intellij/codeInsight/inspections/GuavaInspectionTest.java b/java/typeMigration/test/com/intellij/codeInsight/inspections/GuavaInspectionTest.java index b2fc4e117317..3053349823f5 100644 --- a/java/typeMigration/test/com/intellij/codeInsight/inspections/GuavaInspectionTest.java +++ b/java/typeMigration/test/com/intellij/codeInsight/inspections/GuavaInspectionTest.java @@ -263,6 +263,12 @@ public class GuavaInspectionTest extends JavaCodeInsightFixtureTestCase { public void testPredicates6() { doTestAllFile(); } + public void testPredicatesVarArg1() { doTestAllFile(); } + + public void testPredicatesVarArg2() { doTestAllFile(); } + + public void testPredicatesVarArg3() { doTestAllFile(); } + public void testFluentIterableElementTypeChanged() { doTest(); } diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg1.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg1.java new file mode 100644 index 000000000000..904fff15d20a --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg1.java @@ -0,0 +1,13 @@ +package org.example; + +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(Predicates.not(p0), Predicates.not(p1)); + } + public static void call(java.util.function.Predicate... pr) { + } + +} \ No newline at end of file diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg1_after.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg1_after.java new file mode 100644 index 000000000000..cc09d9e6c7bf --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg1_after.java @@ -0,0 +1,12 @@ +package org.example; + +import java.util.function.Predicate; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(p0.negate(), p1.negate()); + } + public static void call(Predicate... pr) { + } + +} \ No newline at end of file diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg2.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg2.java new file mode 100644 index 000000000000..0781019fa616 --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg2.java @@ -0,0 +1,13 @@ +package org.example; + +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(10, Predicates.not(p0), Predicates.not(p1)); + } + + public static void call(int x, java.util.function.Predicate, java.util.function.Predicate... pr) { + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg2_after.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg2_after.java new file mode 100644 index 000000000000..91167122ade5 --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg2_after.java @@ -0,0 +1,12 @@ +package org.example; + +import java.util.function.Predicate; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(10, p0.negate(), p1.negate()); + } + + public static void call(int x, Predicate, Predicate... pr) { + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg3.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg3.java new file mode 100644 index 000000000000..5fa5b9fde8f9 --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg3.java @@ -0,0 +1,13 @@ +package org.example; + +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(10, Predicates.not(p0), Predicates.not(p1)); + } + + public static void call(int x, java.util.function.Predicate, java.util.function.Predicate, java.util.function.Predicate... pr) { + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/inspections/guava/predicatesVarArg3_after.java b/java/typeMigration/testData/inspections/guava/predicatesVarArg3_after.java new file mode 100644 index 000000000000..cd4a7f14eeff --- /dev/null +++ b/java/typeMigration/testData/inspections/guava/predicatesVarArg3_after.java @@ -0,0 +1,12 @@ +package org.example; + +import java.util.function.Predicate; + +public class Main { + void foo(Predicate p0, Predicate p1) { + call(10, p0.negate(), p1.negate()); + } + + public static void call(int x, Predicate, Predicate, Predicate... pr) { + } +} \ No newline at end of file