From bca8fc49faac7d9b9ecfa5e73d7692b3705a97a7 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Mon, 4 May 2020 11:28:41 +0200 Subject: [PATCH] java redundant cast: option to ignore casts for suspicious varargs (IDEA-221139) GitOrigin-RevId: e6a4712ddf0f3c9c5d52a9577bcff0fe14b2eefb --- .../messages/JavaAnalysisBundle.properties | 1 + .../RedundantCastInspection.java | 22 +++-- .../generics/SuspiciousVarargsCall.java | 12 +++ .../codeInspection/RedundantCast15Test.java | 1 + ...ArgumentToVariableArgMethodInspection.java | 95 +++++++++++-------- 5 files changed, 84 insertions(+), 47 deletions(-) create mode 100644 java/java-tests/testData/inspection/redundantCast/generics/SuspiciousVarargsCall.java diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index e33e2cbda309..f2784cea2c2e 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -173,6 +173,7 @@ exports.to.itself.delete.statement.fix=Delete directive html.classes.exposed.with.code.module.info.code.html=Classes exposed with module-info html.ignore.overrides.of.deprecated.abstract.methods=Ignore overrides of deprecated abstract methods from non-deprecated supers ignore.casts.in.suspicious.collections.method.calls=Ignore casts in suspicious collections method calls +ignore.casts.in.suspicious.varargs.method.calls=Ignore casts in suspicious varargs method calls ignore.exceptions.thrown.by.entry.points.methods=Ignore exceptions thrown by entry points methods ignore.in.the.same.outermost.class=Ignore in the same outermost class ignore.inside.deprecated.members=Ignore inside deprecated members diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/redundantCast/RedundantCastInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/redundantCast/RedundantCastInspection.java index c376fada2824..cc32e203f94f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/redundantCast/RedundantCastInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/redundantCast/RedundantCastInspection.java @@ -7,12 +7,12 @@ import com.intellij.codeInspection.miscGenerics.SuspiciousMethodCallUtil; import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel; import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.WriteExternalException; import com.intellij.psi.*; import com.intellij.psi.util.PsiExpressionTrimRenderer; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.RedundantCastUtil; -import org.jdom.Element; +import com.siyeh.ig.bugs.NullArgumentToVariableArgMethodInspection; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -26,7 +26,7 @@ public class RedundantCastInspection extends GenericsInspectionToolBase { @NonNls private static final String SHORT_NAME = "RedundantCast"; public boolean IGNORE_SUSPICIOUS_METHOD_CALLS; - + public boolean IGNORE_SUSPICIOUS_VARARG_METHOD_CALLS = true; public RedundantCastInspection() { myQuickFixAction = new AcceptSuggested(); @@ -52,17 +52,11 @@ public class RedundantCastInspection extends GenericsInspectionToolBase { return getDescriptions(field, manager, isOnTheFly); } - @Override - public void writeSettings(@NotNull Element node) throws WriteExternalException { - if (IGNORE_SUSPICIOUS_METHOD_CALLS) { - super.writeSettings(node); - } - } - @Override public JComponent createOptionsPanel() { final MultipleCheckboxOptionsPanel optionsPanel = new MultipleCheckboxOptionsPanel(this); optionsPanel.addCheckbox(JavaAnalysisBundle.message("ignore.casts.in.suspicious.collections.method.calls"), "IGNORE_SUSPICIOUS_METHOD_CALLS"); + optionsPanel.addCheckbox(JavaAnalysisBundle.message("ignore.casts.in.suspicious.varargs.method.calls"), "IGNORE_SUSPICIOUS_VARARG_METHOD_CALLS"); return optionsPanel; } @@ -81,6 +75,14 @@ public class RedundantCastInspection extends GenericsInspectionToolBase { return null; } } + + if (gParent instanceof PsiCallExpression && IGNORE_SUSPICIOUS_VARARG_METHOD_CALLS) { + PsiExpressionList expressionList = (PsiExpressionList)parent; + if (PsiTreeUtil.isAncestor(expressionList.getExpressions()[expressionList.getExpressionCount() - 1], operand, true) && + NullArgumentToVariableArgMethodInspection.isSuspiciousVararg((PsiCallExpression)gParent, operand.getType())) { + return null; + } + } } String message = JavaAnalysisBundle.message("inspection.redundant.cast.problem.descriptor", diff --git a/java/java-tests/testData/inspection/redundantCast/generics/SuspiciousVarargsCall.java b/java/java-tests/testData/inspection/redundantCast/generics/SuspiciousVarargsCall.java new file mode 100644 index 000000000000..37d86462059f --- /dev/null +++ b/java/java-tests/testData/inspection/redundantCast/generics/SuspiciousVarargsCall.java @@ -0,0 +1,12 @@ +class MyTest { + void v(Object... objects) { } + void v(String... objects) { } + + void m(String[] values){ + v((Object[]) values); + v((Object[]) null); + + v((String[]) values); + v((String[]) null); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java index 1c609838a525..007eb49a87f1 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantCast15Test.java @@ -81,6 +81,7 @@ public class RedundantCast15Test extends LightJavaCodeInsightFixtureTestCase { myFixture.testHighlighting(getTestName(false) + ".java"); } public void testDifferentNullness() { doTest();} + public void testSuspiciousVarargsCall() { doTest();} public void testPrimitiveWidening() { doTest(); } public void testCastLongLiteral() { doTest(); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/NullArgumentToVariableArgMethodInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/NullArgumentToVariableArgMethodInspection.java index a936eb11827f..6888eb03e465 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/NullArgumentToVariableArgMethodInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/NullArgumentToVariableArgMethodInspection.java @@ -73,6 +73,16 @@ public class NullArgumentToVariableArgMethodInspection extends BaseInspection { return new NullArgumentToVariableArgVisitor(); } + /** + * Checks if it's unclear from the first glance if a method is called as varargs or not + * + * @return true iff {@code call} is varargs method call and {@code lastArgumentType} is {@code null} type or array type, + * which is assignable from vararg parameter component type + */ + public static boolean isSuspiciousVararg(PsiCall call, PsiType lastArgumentType) { + return NullArgumentToVariableArgVisitor.getSuspiciousVarargType(call, lastArgumentType) != null; + } + private static class NullArgumentToVariableArgVisitor extends BaseInspectionVisitor { @Override @@ -87,6 +97,50 @@ public class NullArgumentToVariableArgMethodInspection extends BaseInspection { visitCall(call); } + private static PsiArrayType getSuspiciousVarargType(PsiCall call, PsiType type) { + final boolean checkArray; + if (PsiType.NULL.equals(type)) { + checkArray = false; + } + else if (type instanceof PsiArrayType) { + checkArray = true; + } + else { + return null; + } + final PsiMethod method = call.resolveMethod(); + if (method == null) { + return null; + } + final PsiParameterList parameterList = method.getParameterList(); + PsiExpressionList argumentList = call.getArgumentList(); + if (argumentList == null || parameterList.getParametersCount() != argumentList.getExpressionCount()) { + return null; + } + final PsiParameter[] parameters = parameterList.getParameters(); + final PsiParameter lastParameter = parameters[parameters.length - 1]; + if (!lastParameter.isVarArgs()) { + return null; + } + final PsiType type1 = lastParameter.getType(); + if (!(type1 instanceof PsiEllipsisType)) { + return null; + } + + final PsiEllipsisType ellipsisType = (PsiEllipsisType)type1; + final PsiArrayType arrayType = (PsiArrayType)ellipsisType.toArrayType(); + final PsiType componentType = arrayType.getComponentType(); + if (checkArray) { + if (!componentType.equals(TypeUtils.getObjectType(call))) { + return null; + } + if (type.isAssignableFrom(arrayType) || !arrayType.isAssignableFrom(type)) { + return null; + } + } + return arrayType; + } + private void visitCall(PsiCall call) { final PsiExpressionList argumentList = call.getArgumentList(); if (argumentList == null) { @@ -98,45 +152,12 @@ public class NullArgumentToVariableArgMethodInspection extends BaseInspection { } final PsiExpression lastArgument = arguments[arguments.length - 1]; final PsiType type = lastArgument.getType(); - final boolean checkArray; - if (PsiType.NULL.equals(type)) { - checkArray = false; - } - else if (type instanceof PsiArrayType) { - checkArray = true; - } - else { + + PsiArrayType arrayType = getSuspiciousVarargType(call, type); + if (arrayType == null) { return; } - final PsiMethod method = call.resolveMethod(); - if (method == null) { - return; - } - final PsiParameterList parameterList = method.getParameterList(); - if (parameterList.getParametersCount() != arguments.length) { - return; - } - final PsiParameter[] parameters = parameterList.getParameters(); - final PsiParameter lastParameter = parameters[parameters.length - 1]; - if (!lastParameter.isVarArgs()) { - return; - } - final PsiType type1 = lastParameter.getType(); - if (!(type1 instanceof PsiEllipsisType)) { - return; - } - final PsiEllipsisType ellipsisType = (PsiEllipsisType)type1; - final PsiType arrayType = ellipsisType.toArrayType(); - final PsiType componentType = ellipsisType.getComponentType(); - if (checkArray) { - if (!componentType.equals(TypeUtils.getObjectType(call))) { - return; - } - if (type.isAssignableFrom(arrayType) || !arrayType.isAssignableFrom(type)) { - return; - } - } - registerError(lastArgument, lastArgument, componentType, arrayType); + registerError(lastArgument, lastArgument, arrayType.getComponentType(), arrayType); } } } \ No newline at end of file