IDEA-129589 (C-style array declaration in method return type not detected)

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties

@@ -992,7 +992,8 @@ constant.on.lhs.of.comparison.problem.descriptor=<code>#ref</code>: constant on
 constant.on.rhs.of.comparison.problem.descriptor=<code>#ref</code>: constant on right side of comparison #loc
 control.flow.statement.without.braces.problem.descriptor=<code>#ref</code> without braces #loc
 missorted.modifiers.problem.descriptor=Missorted modifiers <code>#ref</code> #loc
-c.style.array.declaration.problem.descriptor=C-style array declaration <code>#ref</code> #loc
+cstyle.array.variable.declaration.problem.descriptor=C-style array declaration of {0, choice, 1#field|2#parameter|3#local variable} <code>#ref</code> #loc
+cstyle.array.method.declaration.problem.descriptor=C-style array declaration of the return type of method <code>#ref()</code>#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 <code>#ref</code> does not define a 'serialVersionUID' field #loc

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);
     }
   }
 }

plugins/InspectionGadgets/src/inspectionDescriptions/CStyleArrayDeclaration.html

@@ -1,9 +1,18 @@
 <html>
 <body>
-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:
+<code><pre>
+  <b>public</b> String process(String value[])[] {
+    return value;
+  }
+</pre></code>
+Most code styles prefer Java-style array declarations, with the array indicator brackets attached to the type name.
 <!-- tooltip end -->
 <p>
+Use the checkbox below to only report C-style array declaration of method return types.
+<p>
 
 </body>
 </html>

plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.after.java

@@ -0,0 +1,4 @@
+class FieldWithWhitespace {
+
+    String[] s;
+}

plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/FieldWithWhitespace.java

@@ -0,0 +1,4 @@
+class FieldWithWhitespace {
+
+  String s<caret> [];
+}

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;
+  }
+}

plugins/InspectionGadgets/test/com/siyeh/igfixes/style/cstyle_array_declaration/SimpleMethod.java

@@ -0,0 +1,6 @@
+public class SimpleMethod {
+
+  public String ohGod<caret>(String[] a)[] {
+    return a;
+  }
+}

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 <warning descr="C-style array declaration 'm_bar'">m_bar</warning>[];
+    private int <warning descr="C-style array declaration of field 'm_bar'">m_bar</warning>[];
 
     public CStyleArrayDeclaration(int[] bar, int[] foo)
     {
@@ -18,7 +18,7 @@ public class CStyleArrayDeclaration
 
     public void foo()
     {
-        final int <warning descr="C-style array declaration 'foo'">foo</warning>[] = new int[3];
+        final int <warning descr="C-style array declaration of local variable 'foo'">foo</warning>[] = 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 <warning descr="C-style array declaration 'foo'">foo</warning>[], int[] bar)
+    public void bar(int <warning descr="C-style array declaration of parameter 'foo'">foo</warning>[], int[] bar)
     {
 
     }
+
+    String <warning descr="C-style array declaration of the return type of method 'ohGod()'">ohGod</warning>(String[] a)[] {
+        return a;
+    }
 }

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<IntentionAction> list =
+      ContainerUtil.findAll(myFixture.getAvailableIntentions(), new Condition<IntentionAction>() {
+        @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("/**/", "<caret>");
     myFixture.configureByText(JavaFileType.INSTANCE, before);
-    final IntentionAction intention = myFixture.findSingleIntention(hint);
+    final IntentionAction intention = findIntention(hint);
     assertNotNull(intention);
     myFixture.launchAction(intention);
     myFixture.checkResult(after);

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");
+  }
+}