Issue checkstyle#9280: New property 'accessModifiers' as substitution of 'scope' and 'excludeScope' in JavadocVariableCheck

Author: kkoutsilis

.ci/validation.sh

@@ -216,12 +216,17 @@ no-error-pmd)
   echo "CS_version: ${CS_POM_VERSION}"
   mvn -e --no-transfer-progress clean install -Pno-validations
   echo "Checkout target sources ..."
-  checkout_from "https://github.com/pmd/build-tools.git"
+  #  checkout_from "https://github.com/pmd/build-tools.git"
+  checkout_from "https://github.com/kkoutsilis/build-tools.git"
   cd .ci-temp/build-tools/
+  git ls-remote
+  git checkout "66ded33c74662cb3da612f3d34a5aefaa629b443"
   mvn -e --no-transfer-progress install
   cd ..
-  git clone https://github.com/pmd/pmd.git
+  git clone https://github.com/kkoutsilis/pmd.git
   cd pmd
+  git ls-remote
+  git checkout "fa6a862ac8278906d7bcf21852f6552d27a46a73"
   ./mvnw verify --show-version --errors --batch-mode --no-transfer-progress \
                 -DskipTests \
                 -Dmaven.javadoc.skip=true \
@@ -279,8 +284,10 @@ no-error-xwiki)
   echo "version:${CS_POM_VERSION} antlr4:${ANTLR4_VERSION}"
   mvn -e --no-transfer-progress clean install -Pno-validations
   echo "Checkout target sources ..."
-  checkout_from "https://github.com/xwiki/xwiki-commons.git"
+  checkout_from "https://github.com/kkoutsilis/xwiki-commons"
   cd .ci-temp/xwiki-commons
+  git ls-remote
+  git checkout "00b68b71a34d184a1c3b42520bffafc61151ca32"
   # Build custom Checkstyle rules
   mvn -e --no-transfer-progress -f \
     xwiki-commons-tools/xwiki-commons-tool-verification-resources/pom.xml \
@@ -311,8 +318,10 @@ no-error-xwiki)
   cd ..
   removeFolderWithProtectedFiles xwiki-rendering
   cd ..
-  checkout_from https://github.com/xwiki/xwiki-platform.git
+  checkout_from https://github.com/kkoutsilis/xwiki-platform.git
   cd .ci-temp/xwiki-platform
+  git ls-remote
+  git checkout "6c99a70983acb371015f231aecee6bd20270fb5b"
   # Validate xwiki-platform
   mvn -e --no-transfer-progress checkstyle:check@default -Dcheckstyle.version="${CS_POM_VERSION}"
   cd ..

config/checker-framework-suppressions/checker-nullness-optional-interning-suppressions.xml

@@ -4286,13 +4286,6 @@
     <lineContent>private Pattern versionFormat;</lineContent>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
-    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java</fileName>
-    <specifier>initialization.field.uninitialized</specifier>
-    <message>the default constructor does not initialize field excludeScope</message>
-    <lineContent>private Scope excludeScope;</lineContent>
-  </checkerFrameworkError>
-
   <checkerFrameworkError unstable="false">
     <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java</fileName>
     <specifier>initialization.field.uninitialized</specifier>

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java

@@ -19,39 +19,37 @@
 
 package com.puppycrawl.tools.checkstyle.checks.javadoc;
 
+import java.util.Arrays;
 import java.util.regex.Pattern;
 
 import com.puppycrawl.tools.checkstyle.StatelessCheck;
 import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.FileContents;
-import com.puppycrawl.tools.checkstyle.api.Scope;
 import com.puppycrawl.tools.checkstyle.api.TextBlock;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption;
+import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
 import com.puppycrawl.tools.checkstyle.utils.ScopeUtil;
+import com.puppycrawl.tools.checkstyle.utils.UnmodifiableCollectionUtil;
 
 /**
  * <div>
  * Checks that a variable has a Javadoc comment. Ignores {@code serialVersionUID} fields.
  * </div>
  * <ul>
  * <li>
- * Property {@code excludeScope} - Specify the visibility scope where Javadoc
- * comments are not checked.
- * Type is {@code com.puppycrawl.tools.checkstyle.api.Scope}.
- * Default value is {@code null}.
+ * Property {@code accessModifiers} - Access modifiers of methods where parameters are
+ * checked.
+ * Type is {@code com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption[]}.
+ * Default value is {@code public, protected, package, private}.
  * </li>
  * <li>
  * Property {@code ignoreNamePattern} - Specify the regexp to define variable names to ignore.
  * Type is {@code java.util.regex.Pattern}.
  * Default value is {@code null}.
  * </li>
  * <li>
- * Property {@code scope} - Specify the visibility scope where Javadoc comments are checked.
- * Type is {@code com.puppycrawl.tools.checkstyle.api.Scope}.
- * Default value is {@code private}.
- * </li>
- * <li>
  * Property {@code tokens} - tokens to check
  * Type is {@code java.lang.String[]}.
  * Validation type is {@code tokenSet}.
@@ -84,35 +82,28 @@ public class JavadocVariableCheck
      * A key is pointing to the warning message text in "messages.properties"
      * file.
      */
-    public static final String MSG_JAVADOC_MISSING = "javadoc.missing";
-
-    /** Specify the visibility scope where Javadoc comments are checked. */
-    private Scope scope = Scope.PRIVATE;
 
-    /** Specify the visibility scope where Javadoc comments are not checked. */
-    private Scope excludeScope;
+    public static final String MSG_JAVADOC_MISSING = "javadoc.missing";
+    /** Access modifiers of methods where parameters are checked. */
+    private AccessModifierOption[] accessModifiers = {
+        AccessModifierOption.PUBLIC,
+        AccessModifierOption.PROTECTED,
+        AccessModifierOption.PACKAGE,
+        AccessModifierOption.PRIVATE,
+    };
 
     /** Specify the regexp to define variable names to ignore. */
     private Pattern ignoreNamePattern;
 
     /**
-     * Setter to specify the visibility scope where Javadoc comments are checked.
+     * Setter to access modifiers of methods where parameters are checked.
      *
-     * @param scope a scope.
-     * @since 3.0
+     * @param accessModifiers access modifiers of methods which should be checked.
+     * @since 10.22.0
      */
-    public void setScope(Scope scope) {
-        this.scope = scope;
-    }
-
-    /**
-     * Setter to specify the visibility scope where Javadoc comments are not checked.
-     *
-     * @param excludeScope a scope.
-     * @since 3.4
-     */
-    public void setExcludeScope(Scope excludeScope) {
-        this.excludeScope = excludeScope;
+    public void setAccessModifiers(AccessModifierOption... accessModifiers) {
+        this.accessModifiers =
+            UnmodifiableCollectionUtil.copyOfArray(accessModifiers, accessModifiers.length);
     }
 
     /**
@@ -176,6 +167,17 @@ private boolean isIgnored(DetailAST ast) {
             || "serialVersionUID".equals(name);
     }
 
+    /**
+     * Checks whether a method has the correct access modifier to be checked.
+     *
+     * @param accessModifier the access modifier of the method.
+     * @return whether the method matches the expected access modifier.
+     */
+    private boolean matchAccessModifiers(final AccessModifierOption accessModifier) {
+        return Arrays.stream(accessModifiers)
+            .anyMatch(modifier -> modifier == accessModifier);
+    }
+
     /**
      * Whether we should check this node.
      *
@@ -185,14 +187,15 @@ private boolean isIgnored(DetailAST ast) {
     private boolean shouldCheck(final DetailAST ast) {
         boolean result = false;
         if (!ScopeUtil.isInCodeBlock(ast) && !isIgnored(ast)) {
-            final Scope customScope = ScopeUtil.getScope(ast);
-            final Scope surroundingScope = ScopeUtil.getSurroundingScope(ast);
-            result = customScope.isIn(scope) && surroundingScope.isIn(scope)
-                && (excludeScope == null
-                    || !customScope.isIn(excludeScope)
-                    || !surroundingScope.isIn(excludeScope));
+            try {
+                final AccessModifierOption accessModifier =
+                    CheckUtil.getAccessModifierFromModifiersToken(ast);
+                result = matchAccessModifiers(accessModifier);
+            }
+            catch (IllegalArgumentException ex) {
+                result = false;
+            }
         }
         return result;
     }
-
 }

src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/JavadocVariableCheck.xml

@@ -8,18 +8,15 @@
  Checks that a variable has a Javadoc comment. Ignores {@code serialVersionUID} fields.
  &lt;/div&gt;</description>
          <properties>
-            <property name="excludeScope" type="com.puppycrawl.tools.checkstyle.api.Scope">
-               <description>Specify the visibility scope where Javadoc
- comments are not checked.</description>
+            <property default-value="public, protected, package, private"
+                      name="accessModifiers"
+                      type="com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption[]">
+               <description>Access modifiers of methods where parameters are
+ checked.</description>
             </property>
             <property name="ignoreNamePattern" type="java.util.regex.Pattern">
                <description>Specify the regexp to define variable names to ignore.</description>
             </property>
-            <property default-value="private"
-                      name="scope"
-                      type="com.puppycrawl.tools.checkstyle.api.Scope">
-               <description>Specify the visibility scope where Javadoc comments are checked.</description>
-            </property>
             <property default-value="ENUM_CONSTANT_DEF"
                       name="tokens"
                       type="java.lang.String[]"

src/site/xdoc/checks/javadoc/javadocvariable.xml

@@ -26,11 +26,11 @@
               <th>since</th>
             </tr>
             <tr>
-              <td>excludeScope</td>
-              <td>Specify the visibility scope where Javadoc comments are not checked.</td>
-              <td><a href="../../property_types.html#Scope">Scope</a></td>
-              <td><code>null</code></td>
-              <td>3.4</td>
+              <td>accessModifiers</td>
+              <td>Access modifiers of methods where parameters are checked.</td>
+              <td><a href="../../property_types.html#AccessModifierOption.5B.5D">AccessModifierOption[]</a></td>
+              <td><code>public, protected, package, private</code></td>
+              <td>10.22.0</td>
             </tr>
             <tr>
               <td>ignoreNamePattern</td>
@@ -39,13 +39,6 @@
               <td><code>null</code></td>
               <td>5.8</td>
             </tr>
-            <tr>
-              <td>scope</td>
-              <td>Specify the visibility scope where Javadoc comments are checked.</td>
-              <td><a href="../../property_types.html#Scope">Scope</a></td>
-              <td><code>private</code></td>
-              <td>3.0</td>
-            </tr>
             <tr>
               <td>tokens</td>
               <td>tokens to check</td>
@@ -78,33 +71,33 @@
 </code></pre></div>
         <p id="Example1-code">
           By default, this setting will report a violation if
-          there is no javadoc for any scope member.
+          there is no javadoc for a member with any access modifier.
         </p>
         <div class="wrapper"><pre class="prettyprint"><code class="language-java">
 public class Example1 {
-  private int a; // violation
+  private int a; // violation, 'Missing a Javadoc comment'
 
   /**
    * Some description here
    */
   private int b;
-  protected int c; // violation
-  public int d; // violation
-  /*package*/ int e; // violation
+  protected int c; // violation, 'Missing a Javadoc comment'
+  public int d; // violation, 'Missing a Javadoc comment'
+  /*package*/ int e; // violation, 'Missing a Javadoc comment'
 
 }
 </code></pre></div>
 
         <p id="Example2-config">
-          To configure the check for <code>public</code>
-          scope:
+          To configure the check <code>public</code>
+          access modifier:
         </p>
 
         <div class="wrapper"><pre class="prettyprint"><code class="language-xml">
 &lt;module name="Checker"&gt;
   &lt;module name="TreeWalker"&gt;
     &lt;module name="JavadocVariable"&gt;
-      &lt;property name="scope" value="public"/&gt;
+      &lt;property name="accessModifiers" value="public"/&gt;
     &lt;/module&gt;
   &lt;/module&gt;
 &lt;/module&gt;
@@ -122,42 +115,40 @@ public class Example2 {
    */
   private int b;
   protected int c;
-  public int d; // violation
+  public int d; // violation, 'Missing a Javadoc comment'
   /*package*/ int e;
 }
 </code></pre></div>
 
         <p id="Example3-config">
-          To configure the check for members which are in <code>private</code>, but not in
-          <code>protected</code> scope:
+          To configure the check for members which are in <code>private</code> or
+          <code>package</code> access modifier:
         </p>
 
         <div class="wrapper"><pre class="prettyprint"><code class="language-xml">
 &lt;module name="Checker"&gt;
   &lt;module name="TreeWalker"&gt;
     &lt;module name="JavadocVariable"&gt;
-      &lt;property name="scope" value="private"/&gt;
-      &lt;property name="excludeScope" value="protected"/&gt;
+      &lt;property name="accessModifiers" value="private,package"/&gt;
     &lt;/module&gt;
   &lt;/module&gt;
 &lt;/module&gt;
 </code></pre></div>
         <p id="Example3-code">
           This setting will report a violation if there is no
-          javadoc for <code>private</code> member and
-          ignores <code>protected</code> member.
+          javadoc for <code>private</code>  or <code>package</code> member.
         </p>
         <div class="wrapper"><pre class="prettyprint"><code class="language-java">
 public class Example3 {
-  private int a; // violation
+  private int a; // violation, 'Missing a Javadoc comment'
 
   /**
    * Some description here
    */
   private int b;
   protected int c;
   public int d;
-  /*package*/ int e; // violation
+  /*package*/ int e; // violation, 'Missing a Javadoc comment'
 }
 </code></pre></div>
         <p id="Example4-config">
@@ -181,15 +172,15 @@ public class Example3 {
         </p>
         <div class="wrapper"><pre class="prettyprint"><code class="language-java">
 public class Example4 {
-  private int a; // violation
+  private int a; // violation, 'Missing a Javadoc comment'
 
   /**
    * Some description here
   */
   private int b;
-  protected int c; // violation
-  public int d; // violation
-  /*package*/ int e; // violation
+  protected int c; // violation, 'Missing a Javadoc comment'
+  public int d; // violation, 'Missing a Javadoc comment'
+  /*package*/ int e; // violation, 'Missing a Javadoc comment'
 }
 </code></pre></div>
       </subsection>