From cb2e86701d6bd636234178e81aa9958d80d89e36 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 10 Apr 2019 14:47:07 +0700 Subject: [PATCH] IterableUsedAsVararg: fixes according to review IDEA-CR-45949 --- .../IterableUsedAsVarargInspection.java | 12 ++++++--- .../IterableUsedAsVararg.html | 12 +++++---- .../iterableUsedAsVararg/afterErrorCall.java | 26 +++++++++++++++++++ .../iterableUsedAsVararg/beforeErrorCall.java | 26 +++++++++++++++++++ 4 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/afterErrorCall.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/beforeErrorCall.java diff --git a/java/java-impl/src/com/intellij/codeInspection/miscGenerics/IterableUsedAsVarargInspection.java b/java/java-impl/src/com/intellij/codeInspection/miscGenerics/IterableUsedAsVarargInspection.java index b9947a81e671..7326277c9423 100644 --- a/java/java-impl/src/com/intellij/codeInspection/miscGenerics/IterableUsedAsVarargInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/miscGenerics/IterableUsedAsVarargInspection.java @@ -55,9 +55,13 @@ public class IterableUsedAsVarargInspection extends AbstractBaseJavaLocalInspect String replacement = "new " + className + "[0]"; argCopy.replace(factory.createExpressionFromText(replacement, argCopy)); JavaResolveResult copyResult = callCopy.getMethodExpression().advancedResolve(false); - if (copyResult.getElement() != method) return; - PsiType substitutionWithArray = copyResult.getSubstitutor().substitute(componentType); - if (substitutionWithArray == null || TypeUtils.isJavaLangObject(substitutionWithArray)) return; + if (copyResult.getElement() == method) { + PsiType substitutionWithArray = copyResult.getSubstitutor().substitute(componentType); + if (substitutionWithArray == null || TypeUtils.isJavaLangObject(substitutionWithArray)) return; + } else { + PsiMethod newMethod = (PsiMethod)copyResult.getElement(); + if (newMethod == null || !newMethod.isVarArgs() || newMethod.getParameterList().getParametersCount() != argCount) return; + } LocalQuickFix fix = null; if (InheritanceUtil.isInheritor(varArgExpression.getType(), CommonClassNames.JAVA_UTIL_COLLECTION)) { fix = new AddToArrayFix(className); @@ -86,7 +90,7 @@ public class IterableUsedAsVarargInspection extends AbstractBaseJavaLocalInspect if (!InheritanceUtil.isInheritor(expression.getType(), CommonClassNames.JAVA_UTIL_COLLECTION)) return; PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); String fullReplacementText = - ParenthesesUtils.getText(expression, ParenthesesUtils.METHOD_CALL_PRECEDENCE) + ".toArray(new " + myClassName + "[0])"; + ParenthesesUtils.getText(expression, ParenthesesUtils.METHOD_CALL_PRECEDENCE + 1) + ".toArray(new " + myClassName + "[0])"; expression.replace(factory.createExpressionFromText(fullReplacementText, expression)); } } diff --git a/java/java-impl/src/inspectionDescriptions/IterableUsedAsVararg.html b/java/java-impl/src/inspectionDescriptions/IterableUsedAsVararg.html index 0e8d9223e566..6debc01ffc0c 100644 --- a/java/java-impl/src/inspectionDescriptions/IterableUsedAsVararg.html +++ b/java/java-impl/src/inspectionDescriptions/IterableUsedAsVararg.html @@ -1,10 +1,12 @@ -Reports suspicious usages of collection or iterables in vararg method calls. E.g. given method - <T> boolean contains(T needle, T... haystack) the call like - if(contains("item", listOfStrings)) {...} looks suspicious as the list will be - wrapped into single element array. Such code can be correctly compiled and likely run without - exceptions, but it's unlikely intended. +Reports suspicious usages of Collection or an Iterable in vararg method calls. E.g. given method + 
<T> boolean contains(T needle, T... haystack) {...}
+

a call like

+ 
if(contains("item", listOfStrings)) {...}
+

looks suspicious as the list will be wrapped into a single element array. + Such code can be successfully compiled and likely run without + exceptions, but it's unlikely intended.

New in 2019.2

diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/afterErrorCall.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/afterErrorCall.java new file mode 100644 index 000000000000..fef2edbf5430 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/afterErrorCall.java @@ -0,0 +1,26 @@ +// "Call 'toArray(new String[0])'" "true" +import java.util.List; +import java.util.Objects; + +class Test { + static boolean contains(T needle, T... haystack) { + for (final T t : haystack) { + if (Objects.equals(t, needle)) { + return true; + } + } + return false; + } + + static boolean contains(String needle, String... haystack) { + return contains((Object)needle, (Object[])haystack); + } + + void use(String s) { + if (contains(s, getList().toArray(new String[0]))) { + + } + } + + native List getList(); +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/beforeErrorCall.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/beforeErrorCall.java new file mode 100644 index 000000000000..5c029727b6d1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/iterableUsedAsVararg/beforeErrorCall.java @@ -0,0 +1,26 @@ +// "Call 'toArray(new String[0])'" "true" +import java.util.List; +import java.util.Objects; + +class Test { + static boolean contains(T needle, T... haystack) { + for (final T t : haystack) { + if (Objects.equals(t, needle)) { + return true; + } + } + return false; + } + + static boolean contains(String needle, String... haystack) { + return contains((Object)needle, (Object[])haystack); + } + + void use(String s) { + if (contains(s, getList())) { + + } + } + + native List getList(); +} \ No newline at end of file