From 19a21fd43734f5fd9cea5111c4b4ff12f1e1f44b Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 7 Nov 2013 09:55:18 +0100 Subject: [PATCH] new "Non-varargs method overrides varargs method" inspection --- .../src/META-INF/InspectionGadgets.xml | 4 + .../siyeh/InspectionGadgetsBundle.properties | 10 +- .../ig/fixes/ConvertToVarargsMethodFix.java | 125 ++++++++++++++++++ ...ematicVarargsMethodOverrideInspection.java | 81 ++++++++++++ ...hodCanBeVariableArityMethodInspection.java | 47 +------ .../ProblematicVarargsMethodOverride.html | 7 + .../One.after.java | 14 ++ .../One.java | 14 ++ ...oblematicVarargsMethodOverrideFixTest.java | 37 ++++++ ...icVarargsMethodOverrideInspectionTest.java | 41 ++++++ 10 files changed, 331 insertions(+), 49 deletions(-) create mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/ConvertToVarargsMethodFix.java create mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspection.java create mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/ProblematicVarargsMethodOverride.html create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/inheritance/ProblematicVarargsMethodOverrideFixTest.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspectionTest.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml index 18654c807735..c002a11088b1 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml @@ -1014,6 +1014,10 @@ bundle="com.siyeh.InspectionGadgetsBundle" key="non.protected.constructor.in.abstract.class.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.inheritance.issues" enabledByDefault="false" level="WARNING" implementationClass="com.siyeh.ig.inheritance.NonProtectedConstructorInAbstractClassInspection"/> + #ref() can be converted to variable arity method #loc method.can.be.variable.arity.method.ignore.byte.short.option=Ignore methods with a last parameter of type byte[] or short[] -convert.to.variable.arity.method.quickfix=Convert to variable arity method +convert.to.variable.arity.method.quickfix=Convert to varargs method mismatched.string.builder.query.update.display.name=Mismatched query and update of StringBuilder mismatched.string.builder.updated.problem.descriptor=Contents of {0} #ref are updated, but never queried #loc mismatched.string.builder.queried.problem.descriptor=Contents of {0} #ref are queried, but never updated #loc @@ -2036,8 +2036,8 @@ inner.class.referenced.via.subclass.display.name=Inner class referenced via subc inner.class.referenced.via.subclass.problem.descriptor=Inner class #ref declared in class ''{0}'' but referenced via subclass ''{1}'' #loc inner.class.referenced.via.subclass.quickfix=Rationalize inner class access boolean.parameter.display.name='public' method with 'boolean' parameter -boolean.parameter.problem.descriptor='public' method #ref with 'boolean' parameter -boolean.parameters.problem.descriptor='public' method #ref with 'boolean' parameters +boolean.parameter.problem.descriptor='public' method #ref() with 'boolean' parameter +boolean.parameters.problem.descriptor='public' method #ref() with 'boolean' parameters boolean.parameter.constructor.problem.descriptor='public' constructor #ref with 'boolean' parameter boolean.parameters.constructor.problem.descriptor='public' constructor #ref with 'boolean' parameters boolean.parameter.only.report.multiple.option=Only report methods with multiple boolean parameters @@ -2056,4 +2056,6 @@ auto.closeable.resource.returned.option=Ignore AutoCloseable instances returned problematic.whitespace.display.name=Problematic whitespace problematic.whitespace.tabs.problem.descriptor=File ''{0}'' uses tabs for indentation problematic.whitespace.spaces.problem.descriptor=File ''{0}'' uses spaces for indentation -problematic.whitespace.show.whitespaces.quickfix=Toggle show whitespace in the editor \ No newline at end of file +problematic.whitespace.show.whitespaces.quickfix=Toggle show whitespace in the editor +problematic.varargs.method.display.name=Non-varargs method overrides varargs method +problematic.varargs.method.override.problem.descriptor=Non-varargs method #ref() overrides varargs method \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/ConvertToVarargsMethodFix.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/ConvertToVarargsMethodFix.java new file mode 100644 index 000000000000..2c0212fafc2f --- /dev/null +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/fixes/ConvertToVarargsMethodFix.java @@ -0,0 +1,125 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.fixes; + +import com.intellij.codeInsight.FileModificationService; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.util.IncorrectOperationException; +import com.intellij.util.Query; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.InspectionGadgetsFix; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Collection; + +/** + * @author Bas Leijdekkers + */ +public class ConvertToVarargsMethodFix extends InspectionGadgetsFix { + + @Override + @NotNull + public String getFamilyName() { + return getName(); + } + + @NotNull + @Override + public String getName() { + return InspectionGadgetsBundle.message("convert.to.variable.arity.method.quickfix"); + } + + @Override + protected boolean prepareForWriting() { + return false; + } + + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement(); + final PsiElement parent = element.getParent(); + if (!(parent instanceof PsiMethod)) { + return; + } + final PsiMethod method = (PsiMethod)parent; + final Collection writtenElements = new ArrayList(); + final Collection methodCalls = new ArrayList(); + writtenElements.add(method); + for (final PsiReference reference : ReferencesSearch.search(method, method.getUseScope(), false)) { + final PsiElement referenceElement = reference.getElement(); + if (referenceElement instanceof PsiReferenceExpression) { + writtenElements.add(referenceElement); + methodCalls.add((PsiReferenceExpression)referenceElement); + } + } + if (!FileModificationService.getInstance().preparePsiElementsForWrite(writtenElements)) { + return; + } + makeMethodVarargs(method); + makeMethodCallsVarargs(methodCalls); + } + + private static void makeMethodVarargs(PsiMethod method) { + final PsiParameterList parameterList = method.getParameterList(); + if (parameterList.getParametersCount() == 0) { + return; + } + final PsiParameter[] parameters = parameterList.getParameters(); + final PsiParameter lastParameter = parameters[parameters.length - 1]; + final PsiType type = lastParameter.getType(); + if (!(type instanceof PsiArrayType)) { + return; + } + final PsiArrayType arrayType = (PsiArrayType)type; + final PsiType componentType = arrayType.getComponentType(); + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(method.getProject()); + final PsiType ellipsisType = PsiEllipsisType.createEllipsis(componentType, type.getAnnotations()); + final PsiTypeElement newTypeElement = factory.createTypeElement(ellipsisType); + final PsiTypeElement typeElement = lastParameter.getTypeElement(); + if (typeElement != null) { + typeElement.replace(newTypeElement); + } + } + + private static void makeMethodCallsVarargs(Collection referenceExpressions) { + for (final PsiReferenceExpression referenceExpression : referenceExpressions) { + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)referenceExpression.getParent(); + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length == 0) { + continue; + } + final PsiExpression lastArgument = arguments[arguments.length - 1]; + if (!(lastArgument instanceof PsiNewExpression)) { + continue; + } + final PsiNewExpression newExpression = (PsiNewExpression)lastArgument; + final PsiArrayInitializerExpression arrayInitializerExpression = newExpression.getArrayInitializer(); + if (arrayInitializerExpression == null) { + continue; + } + final PsiExpression[] initializers = arrayInitializerExpression.getInitializers(); + for (final PsiExpression initializer : initializers) { + argumentList.add(initializer); + } + lastArgument.delete(); + } + } +} diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspection.java new file mode 100644 index 000000000000..bf121ff8c08a --- /dev/null +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspection.java @@ -0,0 +1,81 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.inheritance; + +import com.intellij.psi.*; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.fixes.ConvertToVarargsMethodFix; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * @author Bas Leijdekkers + */ +public class ProblematicVarargsMethodOverrideInspection extends BaseInspection { + + @Nls + @NotNull + @Override + public String getDisplayName() { + return InspectionGadgetsBundle.message("problematic.varargs.method.display.name"); + } + + @NotNull + @Override + protected String buildErrorString(Object... infos) { + return InspectionGadgetsBundle.message("problematic.varargs.method.override.problem.descriptor"); + } + + @Nullable + @Override + protected InspectionGadgetsFix buildFix(Object... infos) { + return new ConvertToVarargsMethodFix(); + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new NonVarargsMethodOverridesVarArgsMethodVisitor(); + } + + private static class NonVarargsMethodOverridesVarArgsMethodVisitor extends BaseInspectionVisitor { + + @Override + public void visitMethod(PsiMethod method) { + super.visitMethod(method); + final PsiParameterList parameterList = method.getParameterList(); + final PsiParameter[] parameters = parameterList.getParameters(); + if (parameters.length == 0) { + return; + } + final PsiParameter parameter = parameters[parameters.length - 1]; + final PsiType type = parameter.getType(); + if (!(type instanceof PsiArrayType)) { + return; + } + final PsiMethod[] superMethods = method.findDeepestSuperMethods(); + for (final PsiMethod superMethod : superMethods) { + if (superMethod.isVarArgs()) { + registerMethodError(method); + return; + } + } + } + } +} diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/MethodCanBeVariableArityMethodInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/MethodCanBeVariableArityMethodInspection.java index ef1591864c0e..74a0a9b5a06e 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/MethodCanBeVariableArityMethodInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/MethodCanBeVariableArityMethodInspection.java @@ -15,15 +15,14 @@ */ package com.siyeh.ig.migration; -import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel; -import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.fixes.ConvertToVarargsMethodFix; import com.siyeh.ig.psiutils.LibraryUtil; import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.Nls; @@ -64,49 +63,7 @@ public class MethodCanBeVariableArityMethodInspection extends BaseInspection { @Override protected InspectionGadgetsFix buildFix(Object... infos) { - return new MethodCanBeVariableArityMethodFix(); - } - - private static class MethodCanBeVariableArityMethodFix extends InspectionGadgetsFix { - @Override - @NotNull - public String getFamilyName() { - return getName(); - } - - @NotNull - @Override - public String getName() { - return InspectionGadgetsBundle.message("convert.to.variable.arity.method.quickfix"); - } - - @Override - protected void doFix(Project project, ProblemDescriptor descriptor) { - final PsiElement element = descriptor.getPsiElement(); - final PsiElement parent = element.getParent(); - if (!(parent instanceof PsiMethod)) { - return; - } - final PsiMethod method = (PsiMethod)parent; - final PsiParameterList parameterList = method.getParameterList(); - if (parameterList.getParametersCount() == 0) { - return; - } - final PsiParameter[] parameters = parameterList.getParameters(); - final PsiParameter lastParameter = parameters[parameters.length - 1]; - final PsiType type = lastParameter.getType(); - if (!(type instanceof PsiArrayType)) { - return; - } - final PsiArrayType arrayType = (PsiArrayType)type; - final PsiType componentType = arrayType.getComponentType(); - final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - final PsiTypeElement newTypeElement = factory.createTypeElementFromText(componentType.getCanonicalText() + "...", method); - final PsiTypeElement typeElement = lastParameter.getTypeElement(); - if (typeElement != null) { - typeElement.replace(newTypeElement); - } - } + return new ConvertToVarargsMethodFix(); } @Override diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ProblematicVarargsMethodOverride.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ProblematicVarargsMethodOverride.html new file mode 100644 index 000000000000..75621e4a01d3 --- /dev/null +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ProblematicVarargsMethodOverride.html @@ -0,0 +1,7 @@ + + +Reports methods overriding a variable arity (varargs) method with an array parameter. While this is legal Java, it can be confusing. +

+New in 13 + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.after.java new file mode 100644 index 000000000000..79dd8da98a29 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.after.java @@ -0,0 +1,14 @@ +package com.siyeh.igfixes.inheritance.problematic_varargs_method_override; + +class One { + + public void m(String... ss) {} +} +class Two extends One { + public void m(String... ss) {} +} +class Three { + public static void main(String... args) { + new Two().m("1", "2", "3"); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.java new file mode 100644 index 000000000000..d74e7d0bb3d1 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/inheritance/problematic_varargs_method_override/One.java @@ -0,0 +1,14 @@ +package com.siyeh.igfixes.inheritance.problematic_varargs_method_override; + +class One { + + public void m(String... ss) {} +} +class Two extends One { + public void m(String[] ss) {} +} +class Three { + public static void main(String... args) { + new Two().m(new String[]{"1", "2", "3"}); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/inheritance/ProblematicVarargsMethodOverrideFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/inheritance/ProblematicVarargsMethodOverrideFixTest.java new file mode 100644 index 000000000000..0e94dc6c2883 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/inheritance/ProblematicVarargsMethodOverrideFixTest.java @@ -0,0 +1,37 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.fixes.inheritance; + +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.IGQuickFixesTestCase; +import com.siyeh.ig.inheritance.ProblematicVarargsMethodOverrideInspection; + +/** + * @author Bas Leijdekkers + */ +public class ProblematicVarargsMethodOverrideFixTest extends IGQuickFixesTestCase { + + public void testOne() { doTest(); } + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new ProblematicVarargsMethodOverrideInspection()); + myRelativePath = "inheritance/problematic_varargs_method_override"; + myDefaultHint = InspectionGadgetsBundle.message("convert.to.variable.arity.method.quickfix"); + } + +} diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspectionTest.java new file mode 100644 index 000000000000..46ec84b1c9fb --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/inheritance/ProblematicVarargsMethodOverrideInspectionTest.java @@ -0,0 +1,41 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.inheritance; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightInspectionTestCase; + +/** + * @author Bas Leijdekkers + */ +public class ProblematicVarargsMethodOverrideInspectionTest extends LightInspectionTestCase { + + public void testSimple() { + doTest("class One {" + + " void m(String... ss) {" + + " }" + + "}" + + "class Two extends One {" + + " void /*Non-varargs method 'm()' overrides varargs method*/m/**/(String[] ss) {" + + " }" + + "}"); + } + + @Override + protected InspectionProfileEntry getInspection() { + return new ProblematicVarargsMethodOverrideInspection(); + } +}