From ad2cd0bdcced5481360bda796b375b3b979ef55d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Mon, 8 Sep 2014 11:20:45 +0200 Subject: [PATCH] IDEA-129589 (C-style array declaration in method return type not detected) --- .../siyeh/InspectionGadgetsBundle.properties | 3 +- .../CStyleArrayDeclarationInspection.java | 102 ++++++++++++++---- .../CStyleArrayDeclaration.html | 13 ++- .../FieldWithWhitespace.after.java | 4 + .../FieldWithWhitespace.java | 4 + .../SimpleMethod.after.java | 6 ++ .../SimpleMethod.java | 6 ++ .../CStyleArrayDeclaration.java | 10 +- .../com/siyeh/ig/IGQuickFixesTestCase.java | 28 ++++- .../style/CStyleArrayDeclarationFixTest.java | 37 +++++++ 10 files changed, 184 insertions(+), 29 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/style/CStyleArrayDeclarationFixTest.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 84b76ba906cb..c5f881316b13 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -992,7 +992,8 @@ constant.on.lhs.of.comparison.problem.descriptor=#ref: constant on constant.on.rhs.of.comparison.problem.descriptor=#ref: constant on right side of comparison #loc control.flow.statement.without.braces.problem.descriptor=#ref without braces #loc missorted.modifiers.problem.descriptor=Missorted modifiers #ref #loc -c.style.array.declaration.problem.descriptor=C-style array declaration #ref #loc +cstyle.array.variable.declaration.problem.descriptor=C-style array declaration of {0, choice, 1#field|2#parameter|3#local variable} #ref #loc +cstyle.array.method.declaration.problem.descriptor=C-style array declaration of the return type of method #ref()#loc multiple.declaration.problem.descriptor=Multiple variables in one declaration #loc multiple.typed.declaration.problem.descriptor=Variables of different types in one declaration #loc serializable.inner.class.has.serial.version.uid.field.problem.descriptor=Inner class #ref does not define a 'serialVersionUID' field #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/CStyleArrayDeclarationInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/CStyleArrayDeclarationInspection.java index e4e91685878b..fca66288aaef 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/CStyleArrayDeclarationInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/CStyleArrayDeclarationInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2014 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. @@ -16,20 +16,24 @@ package com.siyeh.ig.style; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiType; -import com.intellij.psi.PsiTypeElement; -import com.intellij.psi.PsiVariable; -import com.intellij.util.IncorrectOperationException; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.tree.IElementType; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; public class CStyleArrayDeclarationInspection extends BaseInspection { + public boolean ignoreVariables = false; + @Override @NotNull public String getDisplayName() { @@ -40,8 +44,18 @@ public class CStyleArrayDeclarationInspection extends BaseInspection { @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "c.style.array.declaration.problem.descriptor"); + final Object info = infos[0]; + if (info instanceof PsiMethod) { + return InspectionGadgetsBundle.message("cstyle.array.method.declaration.problem.descriptor"); + } + final int choice = info instanceof PsiField ? 1 : info instanceof PsiParameter ? 2 : 3; + return InspectionGadgetsBundle.message("cstyle.array.variable.declaration.problem.descriptor", Integer.valueOf(choice)); + } + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel("Ignore C-style declarations in variables", this, "ignoreVariables"); } @Override @@ -65,12 +79,38 @@ public class CStyleArrayDeclarationInspection extends BaseInspection { } @Override - public void doFix(Project project, ProblemDescriptor descriptor) - throws IncorrectOperationException { - final PsiElement nameElement = descriptor.getPsiElement(); - final PsiVariable var = (PsiVariable)nameElement.getParent(); - assert var != null; - var.normalizeDeclaration(); + public void doFix(Project project, ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement().getParent(); + if (element instanceof PsiVariable) { + final PsiVariable variable = (PsiVariable)element; + variable.normalizeDeclaration(); + CodeStyleManager.getInstance(project).reformat(variable); + } + else if (element instanceof PsiMethod) { + final PsiMethod method = (PsiMethod)element; + final PsiTypeElement returnTypeElement = method.getReturnTypeElement(); + if (returnTypeElement == null) { + return; + } + final PsiType returnType = method.getReturnType(); + if (returnType == null) { + return; + } + PsiElement child = method.getParameterList(); + while (!(child instanceof PsiCodeBlock)) { + final PsiElement element1 = child; + child = child.getNextSibling(); + if (element1 instanceof PsiJavaToken) { + final PsiJavaToken token = (PsiJavaToken)element1; + final IElementType tokenType = token.getTokenType(); + if (JavaTokenType.LBRACKET.equals(tokenType) || JavaTokenType.RBRACKET.equals(tokenType)) { + token.delete(); + } + } + } + final PsiTypeElement typeElement = JavaPsiFacade.getElementFactory(project).createTypeElement(returnType); + returnTypeElement.replace(typeElement); + } } } @@ -79,17 +119,19 @@ public class CStyleArrayDeclarationInspection extends BaseInspection { return new CStyleArrayDeclarationVisitor(); } - private static class CStyleArrayDeclarationVisitor - extends BaseInspectionVisitor { + private class CStyleArrayDeclarationVisitor extends BaseInspectionVisitor { @Override - public void visitVariable(@NotNull PsiVariable var) { - super.visitVariable(var); - final PsiType declaredType = var.getType(); + public void visitVariable(@NotNull PsiVariable variable) { + super.visitVariable(variable); + if (ignoreVariables) { + return; + } + final PsiType declaredType = variable.getType(); if (declaredType.getArrayDimensions() == 0) { return; } - final PsiTypeElement typeElement = var.getTypeElement(); + final PsiTypeElement typeElement = variable.getTypeElement(); if (typeElement == null) { return; // Could be true for enum constants. } @@ -97,7 +139,25 @@ public class CStyleArrayDeclarationInspection extends BaseInspection { if (elementType.equals(declaredType)) { return; } - registerVariableError(var); + registerVariableError(variable, variable); + } + + @Override + public void visitMethod(PsiMethod method) { + super.visitMethod(method); + final PsiType returnType = method.getReturnType(); + if (returnType == null || returnType.getArrayDimensions() == 0) { + return; + } + final PsiTypeElement typeElement = method.getReturnTypeElement(); + if (typeElement == null) { + return; + } + final PsiType type = typeElement.getType(); + if (type.equals(returnType)) { + return; + } + registerMethodError(method, method); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/CStyleArrayDeclaration.html b/plugins/InspectionGadgets/src/inspectionDescriptions/CStyleArrayDeclaration.html index 6b8f3ff43b2a..7320abc37f19 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/CStyleArrayDeclaration.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/CStyleArrayDeclaration.html @@ -1,9 +1,18 @@ -Reports array declarations made using C-style syntax, with the array indicator attached to the variable, -rather than Java-style syntax, with the array indicator attached to the type. +Reports array declarations made using C-style syntax, +with the array indicator brackets positioned after the variable name or after the method parameter list. +For example: +
+  public String process(String value[])[] {
+    return value;
+  }
+
 +Most code styles prefer Java-style array declarations, with the array indicator brackets attached to the type name.

+Use the checkbox below to only report C-style array declaration of method return types. +

\ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.after.java new file mode 100644 index 000000000000..3a3984b1a64c --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.after.java @@ -0,0 +1,4 @@ +class FieldWithWhitespace { + + String[] s; +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.java new file mode 100644 index 000000000000..1f02831d6632 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.java @@ -0,0 +1,4 @@ +class FieldWithWhitespace { + + String s []; +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.after.java new file mode 100644 index 000000000000..0b918a955215 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.after.java @@ -0,0 +1,6 @@ +public class SimpleMethod { + + public String[] ohGod(String[] a) { + return a; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.java new file mode 100644 index 000000000000..5d0ac265b2fc --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.java @@ -0,0 +1,6 @@ +public class SimpleMethod { + + public String ohGod(String[] a)[] { + return a; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/cstyle_array_declaration/CStyleArrayDeclaration.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/cstyle_array_declaration/CStyleArrayDeclaration.java index 1b15641d6a8e..8a01835a2106 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/cstyle_array_declaration/CStyleArrayDeclaration.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/cstyle_array_declaration/CStyleArrayDeclaration.java @@ -3,7 +3,7 @@ package com.siyeh.igtest.style; public class CStyleArrayDeclaration { private int[] m_foo; - private int m_bar[]; + private int m_bar[]; public CStyleArrayDeclaration(int[] bar, int[] foo) { @@ -18,7 +18,7 @@ public class CStyleArrayDeclaration public void foo() { - final int foo[] = new int[3]; + final int foo[] = new int[3]; final int[] bar = new int[3]; for(int i = 0; i < bar.length; i++) @@ -27,8 +27,12 @@ public class CStyleArrayDeclaration } } - public void bar(int foo[], int[] bar) + public void bar(int foo[], int[] bar) { } + + String ohGod(String[] a)[] { + return a; + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/IGQuickFixesTestCase.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/IGQuickFixesTestCase.java index a7731e16196b..5a6c106984bd 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/IGQuickFixesTestCase.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/IGQuickFixesTestCase.java @@ -16,15 +16,21 @@ package com.siyeh.ig; import com.intellij.codeInsight.intention.IntentionAction; +import com.intellij.codeInspection.ex.QuickFixWrapper; import com.intellij.ide.highlighter.JavaFileType; import com.intellij.openapi.application.PluginPathManager; +import com.intellij.openapi.util.Condition; import com.intellij.pom.java.LanguageLevel; import com.intellij.testFramework.builders.JavaModuleFixtureBuilder; import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase; import com.intellij.util.ArrayUtil; +import com.intellij.util.containers.ContainerUtil; import org.intellij.lang.annotations.Language; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.junit.Assert; + +import java.util.List; /** * @author anna @@ -97,12 +103,30 @@ public abstract class IGQuickFixesTestCase extends JavaCodeInsightFixtureTestCas protected void doTest(final String testName, final String hint) { myFixture.configureByFile(getRelativePath() + "/" + testName + ".java"); - final IntentionAction action = myFixture.findSingleIntention(hint); + final IntentionAction action = findIntention(hint); assertNotNull(action); myFixture.launchAction(action); myFixture.checkResultByFile(getRelativePath() + "/" + testName + ".after.java"); } + public IntentionAction findIntention(@NotNull final String hint) { + final List list = + ContainerUtil.findAll(myFixture.getAvailableIntentions(), new Condition() { + @Override + public boolean value(final IntentionAction intentionAction) { + return intentionAction instanceof QuickFixWrapper; + } + }); + if (list.isEmpty()) { + Assert.fail("\"" + hint + "\" not in " + list); + } + else if (list.size() > 1) { + Assert.fail("Too many quickfixes found for \"" + hint + "\": " + list + "]"); + } + return list.get(0); + } + + protected void doExpressionTest( String hint, @Language(value = "JAVA", prefix = "class $X$ {{System.out.print(", suffix = ");}}") @NotNull @NonNls String before, @@ -120,7 +144,7 @@ public abstract class IGQuickFixesTestCase extends JavaCodeInsightFixtureTestCas protected void doTest(String hint, @Language("JAVA") @NotNull @NonNls String before, @Language("JAVA") @NotNull @NonNls String after) { before = before.replace("/**/", ""); myFixture.configureByText(JavaFileType.INSTANCE, before); - final IntentionAction intention = myFixture.findSingleIntention(hint); + final IntentionAction intention = findIntention(hint); assertNotNull(intention); myFixture.launchAction(intention); myFixture.checkResult(after); diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/style/CStyleArrayDeclarationFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/style/CStyleArrayDeclarationFixTest.java new file mode 100644 index 000000000000..85a07f8e08c3 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/style/CStyleArrayDeclarationFixTest.java @@ -0,0 +1,37 @@ +/* + * Copyright 2000-2014 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.style; + +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.IGQuickFixesTestCase; +import com.siyeh.ig.style.CStyleArrayDeclarationInspection; + +/** + * @author Bas Leijdekkers + */ +public class CStyleArrayDeclarationFixTest extends IGQuickFixesTestCase { + + public void testSimpleMethod() { doTest(); } + public void testFieldWithWhitespace() { doTest(); } + + @Override + public void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new CStyleArrayDeclarationInspection()); + myRelativePath = "style/cstyle_array_declaration"; + myDefaultHint = InspectionGadgetsBundle.message("c.style.array.declaration.replace.quickfix"); + } +}