Fix for 1084512 (Parameters of abstract methods hide fields)

Added ignoreAbstractMethods property to allow configure HiddenField check to ignore params of abstract methods. Now test inputs (almost all) will be compiled as part of run.test.suite and run.tests targets (this compilation requires jdk1.5 or higher)
2005-06-12 19:43:06 +00:00 · 2005-06-12 19:43:06 +00:00 · 9f39a76cd5
parent 82b9499604
commit 9f39a76cd5
12 changed files with 140 additions and 26 deletions
--- a/build.xml
+++ b/build.xml
@ -30,6 +30,7 @@
  <property name="xdocs.src" value="src/xdocs"/>
  <property name="xdocs.dest" value="target/docs"/>
  <property name="checkstyle.minimum.javaversion" value="1.4" />
+  <property name="checkstyle.testinputs.minimum.javaversion" value="1.5" />
  <property name="version" value="4.0-beta4" />

  <path id="build.classpath">
@ -179,13 +180,24 @@
    <depend srcdir="src/testinputs" destdir="target/testinputs" closure="yes"/>
    <!-- start of a change to turn on compilation of all input files -->
    <!-- under JDK1.5. -->
-    <property name="testExcludes"
-              value="**/grammars/*,**/j2ee/*,**/InputAssertIdentifier.java,**/InputJUnitTest.java,**/InputValidMethodIndent.java,**/InputSetterGetter.java,**/InputImport.java,**/InputCovariant.java,**/InputClone.java,**/InputSimple.java"/>
    <javac srcdir="src/testinputs"
           destdir="target/testinputs"
           deprecation="on" debug="on"
-           excludes="${testExcludes}"
-           includeAntRuntime="false"/>
+           source="${checkstyle.testinputs.minimum.javaversion}"
+           target="${checkstyle.testinputs.minimum.javaversion}"
+           includeAntRuntime="false">
+        <exclude name="**/j2ee/*" />
+        <exclude name="**/InputAssertIdentifier.java" />
+        <exclude name="**/InputClone.java" />
+        <exclude name="**/InputCovariant.java" />
+        <exclude name="**/InputGrammar.java" />
+        <exclude name="**/InputImport.java" />
+        <exclude name="**/InputJUnitTest.java" />
+        <exclude name="**/InputSetterGetter.java" />
+        <exclude name="**/InputSimple.java" />
+        <exclude name="**/InputValidMethodIndent.java" />
+        <exclude name="**/Post13KeywordsAsIdentifiersOK.java" />
+    </javac>
  </target>

  <!-- Compiles only the test code. Input files are excluded from
@ -373,7 +385,7 @@ For users of JDK 1.5 at least version 1.6.2 of Ant is required.

  <!-- To run the tests need Xalan in the classpath -->
  <target name="run.tests"
-          depends="compile.tests,require.ant15,require.junit,require.xalan"
+          depends="compile.tests,require.ant15,require.junit,require.xalan,compile.testinputs"
          description="Runs the tests for checkstyle">

    <mkdir dir="${testreport.dir}"/>
@ -413,7 +425,7 @@ For users of JDK 1.5 at least version 1.6.2 of Ant is required.

  <!-- To run the tests need Xalan in the classpath -->
  <target name="run.test.suite"
-          depends="compile.tests,require.ant15,require.junit,require.xalan"
+          depends="compile.tests,require.ant15,require.junit,require.xalan,compile.testinputs"
          description="Runs the tests for checkstyle as a test suite">

    <mkdir dir="${testreport.dir}"/>
--- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java
+++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheck.java
@ -88,6 +88,9 @@ public class HiddenFieldCheck
    /** controls whether to check the parameter of a constructor */
    private boolean mIgnoreConstructorParameter;

+    /** controls whether to check the parameter of abstract methods. */
+    private boolean mIgnoreAbstractMethods;
+
    /** @see com.puppycrawl.tools.checkstyle.api.Check */
    public int[] getDefaultTokens()
    {
@ -207,7 +210,8 @@ public class HiddenFieldCheck
             || (!inStatic(aAST) && mCurrentFrame.containsInstanceField(name)))
            && ((mRegexp == null) || (!getRegexp().matcher(name).find()))
            && !isIgnoredSetterParam(aAST, name)
-            && !isIgnoredConstructorParam(aAST))
+            && !isIgnoredConstructorParam(aAST)
+            && !isIgnoredParamOfAbstractMethod(aAST))
        {
            log(nameAST, "hidden.field", name);
        }
@ -296,6 +300,29 @@ public class HiddenFieldCheck
        return (constructorAST.getType() == TokenTypes.CTOR_DEF);
    }

+    /**
+     * Decides whether to ignore an AST node that is the parameter of an
+     * abstract method.
+     * @param aAST the AST to check.
+     * @return true if aAST should be ignored because check property
+     * ignoreAbstactMethods is true and aAST is a parameter of abstract
+     * methods.
+     */
+    private boolean isIgnoredParamOfAbstractMethod(DetailAST aAST)
+    {
+        if (aAST.getType() != TokenTypes.PARAMETER_DEF
+            || !mIgnoreAbstractMethods)
+        {
+            return false;
+        }
+        final DetailAST method = aAST.getParent().getParent();
+        if (method.getType() != TokenTypes.METHOD_DEF) {
+            return false;
+        }
+        final DetailAST mods = method.findFirstToken(TokenTypes.MODIFIERS);
+        return (mods != null && mods.branchContains(TokenTypes.ABSTRACT));
+    }
+
    /**
     * Set the ignore format to the specified regular expression.
     * @param aFormat a <code>String</code> value
@ -333,6 +360,17 @@ public class HiddenFieldCheck
        mIgnoreConstructorParameter = aIgnoreConstructorParameter;
    }

+    /**
+     * Set whether to ignore parameters of abstract methods.
+     * @param aIgnoreAbstractMethods decide whether to ignore
+     * parameters of abstract methods.
+     */
+    public void setIgnoreAbstractMethods(
+        boolean aIgnoreAbstractMethods)
+    {
+        mIgnoreAbstractMethods = aIgnoreAbstractMethods;
+    }
+
    /** @return the regexp to match against */
    public Pattern getRegexp()
    {
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputHiddenField.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputHiddenField.java
@ -224,8 +224,8 @@ enum HiddenEnum
    }
 }

-// perhaps we should ignore this
-// abstract class InputHiddenFieldBug1084512 {
-//     String x;
-//     public abstract void methodA(String x);
-// }
+// we should ignore this if user wants (ignoreAbstractMethods is true)
+abstract class InputHiddenFieldBug1084512 {
+    String x;
+    public abstract void methodA(String x);
+}
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputJavadoc.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputJavadoc.java
@ -109,6 +109,6 @@ enum InputJavadocEnum2
 * @author ABC
 * @version 1.1
 */
-@interface InputJavadocInterfaceType
+@interface InputJavadocInterfaceType1
 {
-}
+}
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputJavadocStyleCheck.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputJavadocStyleCheck.java
@ -251,7 +251,7 @@ public class InputJavadocStyleCheck
    * @param <T> this is NOT an unclosed T tag
    * @author <a href="mailto:foo@nomail.com">Foo Bar</a>
    */
-    public class Test<T>
+    public class TestClass<T>
    {
        /**
        * Retrieves X.
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputTypeParamsTags.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputTypeParamsTags.java
@ -8,7 +8,7 @@ package com.puppycrawl.tools.checkstyle;
 * @author Nobody
 * @version 1.0
 */
-public class InputTypeArgTags<A,B,C extends Comparable>
+public class InputTypeParamsTags<A,B,C extends Comparable>
 {
    /**
     * Some explanation.
@ -37,4 +37,4 @@ public class InputTypeArgTags<A,B,C extends Comparable>
    public <L> void doSomethingElse2(L aAnEl)
    {
    }
-}
+}
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputEmbeddedNullChar.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputEmbeddedNullChar.java
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputHexFloat.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputHexFloat.java
@ -5,12 +5,12 @@ package com.puppycrawl.tools.checkstyle.grammars;
 */
 public class InputHexFloat
 {
-    float f1 = 0x.0P10;
-    float f2 = 0x1.P-1;
-    float f3 = 0Xab1P0;
-    float f4 = 0Xab1ap+20;
-    float f5 = 0Xab1ap+20D;
-    float f6 = 0Xab1ap+20d;
-    float f5 = 0Xab1ap+20f;
-    float f6 = 0Xab1ap+20F;
+    double f1 = 0x.0P10;
+    double f2 = 0x1.P-1;
+    double f3 = 0Xab1P0;
+    double f4 = 0Xab1ap+20;
+    double f5 = 0Xab1ap+20D;
+    double f6 = 0Xab1ap+20d;
+    double f7 = 0Xab1ap+20f;
+    double f8 = 0Xab1ap+20F;
 }
--- a/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputVararg.java
+++ b/src/testinputs/com/puppycrawl/tools/checkstyle/grammars/InputVararg.java
@ -3,7 +3,7 @@ package com.puppycrawl.tools.checkstyle.grammars;
 /**
 * Input for vararg test.
 */
-public class InputGrammar
+public class InputVararg
 {
    public static void main(String... args)
    {
--- a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheckTest.java
+++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/HiddenFieldCheckTest.java
@ -70,6 +70,7 @@ public class HiddenFieldCheckTest
            "210:20: 'hidden' hides a field.",
            "217:13: 'hidden' hides a field.",
            "223:13: 'hiddenStatic' hides a field.",
+            "230:41: 'x' hides a field.",
        };
        verify(checkConfig, getPath("InputHiddenField.java"), expected);
    }
@ -105,6 +106,7 @@ public class HiddenFieldCheckTest
            "210:20: 'hidden' hides a field.",
            "217:13: 'hidden' hides a field.",
            "223:13: 'hiddenStatic' hides a field.",
+            "230:41: 'x' hides a field.",
        };
        verify(checkConfig, getPath("InputHiddenField.java"), expected);
    }
@ -145,6 +147,7 @@ public class HiddenFieldCheckTest
            "210:20: 'hidden' hides a field.",
            "217:13: 'hidden' hides a field.",
            "223:13: 'hiddenStatic' hides a field.",
+            "230:41: 'x' hides a field.",
        };
        verify(checkConfig, getPath("InputHiddenField.java"), expected);
    }
@ -183,6 +186,7 @@ public class HiddenFieldCheckTest
            "200:17: 'hidden' hides a field.",
            "217:13: 'hidden' hides a field.",
            "223:13: 'hiddenStatic' hides a field.",
+            "230:41: 'x' hides a field.",
        };
        verify(checkConfig, getPath("InputHiddenField.java"), expected);
    }
@ -217,4 +221,44 @@ public class HiddenFieldCheckTest
        };
        verify(checkConfig, getPath("InputHiddenFieldReorder.java"), expected);
    }
+
+    public void testIgnoreAbstractMethods() throws Exception
+    {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(HiddenFieldCheck.class);
+        checkConfig.addAttribute("ignoreAbstractMethods", "true");
+
+        final String[] expected = {
+            "18:13: 'hidden' hides a field.",
+            "21:33: 'hidden' hides a field.",
+            "27:13: 'hidden' hides a field.",
+            "32:18: 'hidden' hides a field.",
+            "36:33: 'hidden' hides a field.",
+            "46:17: 'innerHidden' hides a field.",
+            "49:26: 'innerHidden' hides a field.",
+            "55:17: 'innerHidden' hides a field.",
+            "56:17: 'hidden' hides a field.",
+            "61:22: 'innerHidden' hides a field.",
+            "64:22: 'hidden' hides a field.",
+            "69:17: 'innerHidden' hides a field.",
+            "70:17: 'hidden' hides a field.",
+            "76:17: 'innerHidden' hides a field.",
+            "77:17: 'hidden' hides a field.",
+            "82:13: 'hidden' hides a field.",
+            "100:29: 'prop' hides a field.",
+            "106:29: 'prop' hides a field.",
+            "112:29: 'prop' hides a field.",
+            "124:28: 'prop' hides a field.",
+            "138:13: 'hidden' hides a field.",
+            "143:13: 'hidden' hides a field.",
+            "148:13: 'hidden' hides a field.",
+            "152:13: 'hidden' hides a field.",
+            "179:23: 'y' hides a field.",
+            "200:17: 'hidden' hides a field.",
+            "210:20: 'hidden' hides a field.",
+            "217:13: 'hidden' hides a field.",
+            "223:13: 'hiddenStatic' hides a field.",
+        };
+        verify(checkConfig, getPath("InputHiddenField.java"), expected);
+    }
 }
--- a/src/xdocs/config_coding.xml
+++ b/src/xdocs/config_coding.xml
@ -389,6 +389,7 @@ public class MySingleton
            <td><a href="property_types.html#boolean">Boolean</a></td>
            <td><span class="default">false</span></td>
          </tr>
+
          <tr>
            <td>ignoreSetter</td>
            <td>
@ -400,6 +401,14 @@ public class MySingleton
            <td><a href="property_types.html#boolean">Boolean</a></td>
            <td><span class="default">false</span></td>
          </tr>
+
+          <tr>
+            <td>ignoreAbstractMethods</td>
+            <td>Controls whether to ignore parameters of abstract methods.</td>
+            <td><a href="property_types.html#boolean">Boolean</a></td>
+            <td><span class="default">false</span></td>
+          </tr>
+
        </table>
      </subsection>

--- a/src/xdocs/releasenotes.xml
+++ b/src/xdocs/releasenotes.xml
@ -10,6 +10,17 @@

  <body>

+    <section name="Release 4.0 Beta 5">
+      <p>Fixed Bugs:</p>
+
+      <ul>
+        <li>HiddenField can be configured to ignore parameters of
+        abstract methods (property ignoreAbstractMethods, bug
+        1084512)</li>
+
+      </ul>
+    </section>
+
    <section name="Release 4.0 Beta 4">
      <p>Fixed Bugs:</p>