Skip to content

Commit

Permalink
Issue checkstyle#9280: replace Scope with AccessModifierOption in…
Browse files Browse the repository at this point in the history 
… JavadocVariableCheck
  • Loading branch information
@martin-mfg
martin-mfg committed May 19, 2021
1 parent df5cda9 commit b5fbec6
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 127 deletions.
137 changes: 74 additions & 63 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package com.puppycrawl.tools.checkstyle.checks.javadoc;

import java.util.Arrays;
import java.util.regex.Pattern;

import com.puppycrawl.tools.checkstyle.StatelessCheck;
Expand All @@ -28,6 +29,7 @@
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.ScopeUtil;

/**
Expand All @@ -36,15 +38,10 @@
* </p>
* <ul>
* <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 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} - Specify the access modifiers where Javadoc comments 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.
Expand All @@ -67,8 +64,8 @@
* &lt;module name="JavadocVariable"/&gt;
* </pre>
* <p>
* By default, this setting will report a violation if
* there is no javadoc for any scope member.
* By default, this setting will report a violation
* if there is no javadoc for a member with any access modifier.
* </p>
* <pre>
* public class Test {
Expand All @@ -84,40 +81,16 @@
* }
* </pre>
* <p>
* To configure the check for {@code public} scope:
* </p>
* <pre>
* &lt;module name="JavadocVariable"&gt;
* &lt;property name="scope" value="public"/&gt;
* &lt;/module&gt;
* </pre>
* <p>This setting will report a violation if there is no javadoc for {@code public} member.</p>
* <pre>
* public class Test {
* private int a; // OK
*
* &#47;**
* * Some description here
* *&#47;
* private int b; // OK
* protected int c; // OK
* public int d; // violation, missing javadoc for public member
* &#47;*package*&#47; int e; // OK
* }
* </pre>
* <p>
* To configure the check for members which are in {@code private},
* but not in {@code protected} scope:
* To configure the check for {@code package} and {@code private} variables:
* </p>
* <pre>
* &lt;module name="JavadocVariable"&gt;
* &lt;property name="scope" value="private"/&gt;
* &lt;property name="excludeScope" value="protected"/&gt;
* &lt;property name="accessModifiers" value="package,private"/&gt;
* &lt;/module&gt;
* </pre>
* <p>
* This setting will report a violation if there is no javadoc for {@code private}
* member and ignores {@code protected} member.
* This setting will report a violation if there is no javadoc for {@code package}
* or {@code private} members.
* </p>
* <pre>
* public class Test {
Expand All @@ -141,8 +114,8 @@
* &lt;/module&gt;
* </pre>
* <p>
* This setting will report a violation if there is no javadoc for any scope
* member and ignores members with name {@code log} or {@code logger}.
* This setting will report a violation if there is no javadoc for a
* member with any scope and ignores members with name {@code log} or {@code logger}.
* </p>
* <pre>
* public class Test {
Expand Down Expand Up @@ -183,31 +156,25 @@ public class JavadocVariableCheck
*/
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;
/** Specify the access modifiers where Javadoc comments 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.
*
* @param scope a scope.
*/
public void setScope(Scope scope) {
this.scope = scope;
}

/**
* Setter to specify the visibility scope where Javadoc comments are not checked.
* Setter to specify the access modifiers where Javadoc comments are checked.
*
* @param excludeScope a scope.
* @param accessModifiers access modifiers of variables which should be checked.
*/
public void setExcludeScope(Scope excludeScope) {
this.excludeScope = excludeScope;
public void setAccessModifiers(AccessModifierOption... accessModifiers) {
this.accessModifiers =
Arrays.copyOf(accessModifiers, accessModifiers.length);
}

/**
Expand Down Expand Up @@ -285,12 +252,56 @@ private boolean shouldCheck(final DetailAST ast) {
}

final Scope surroundingScope = ScopeUtil.getSurroundingScope(ast);
result = customScope.isIn(scope) && surroundingScope.isIn(scope)
&& (excludeScope == null
|| !customScope.isIn(excludeScope)
|| !surroundingScope.isIn(excludeScope));

final Scope effectiveScope;
if (surroundingScope.isIn(customScope)) {
effectiveScope = customScope;
}
else {
effectiveScope = surroundingScope;
}
result = matchAccessModifiers(toAccessModifier(effectiveScope));
}
return result;
}

/**
* Checks whether a variable has the correct access modifier to be checked.
*
* @param accessModifier the access modifier of the variable.
* @return whether the variable matches the expected access modifier.
*/
private boolean matchAccessModifiers(final AccessModifierOption accessModifier) {
return Arrays.stream(accessModifiers)
.anyMatch(modifier -> modifier == accessModifier);
}

/**
* Converts a {@link Scope} to {@link AccessModifierOption}. {@code Scope.NOTHING} and {@code
* Scope.ANONINNER} are converted to {@code AccessModifierOption.PUBLIC}.
*
* @param scope Scope to be converted.
* @return the corresponding AccessModifierOption.
*/
private static AccessModifierOption toAccessModifier(Scope scope) {
final AccessModifierOption accessModifier;
switch (scope) {
case PROTECTED:
accessModifier = AccessModifierOption.PROTECTED;
break;
case PACKAGE:
accessModifier = AccessModifierOption.PACKAGE;
break;
case PRIVATE:
accessModifier = AccessModifierOption.PRIVATE;
break;
case NOTHING:
case ANONINNER:
case PUBLIC:
default:
accessModifier = AccessModifierOption.PUBLIC;
}
return accessModifier;
}

}
13 changes: 5 additions & 8 deletions ...in/resources/com/puppycrawl/tools/checkstyle/meta/checks/javadoc/JavadocVariableCheck.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@
Checks that a variable has a Javadoc comment. Ignores {@code serialVersionUID} fields.
&lt;/p&gt;</description>
<properties>
<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 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>Specify the access modifiers where Javadoc comments are
checked.</description>
</property>
<property name="ignoreNamePattern" type="java.util.regex.Pattern">
<description>Specify the regexp to define variable names to ignore.</description>
Expand Down
23 changes: 14 additions & 9 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
import com.puppycrawl.tools.checkstyle.api.Scope;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption;
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;

public class JavadocVariableCheckTest
Expand Down Expand Up @@ -93,7 +93,7 @@ public void testAnother2()
throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocVariableCheck.class);
checkConfig.addAttribute("scope", Scope.PUBLIC.getName());
checkConfig.addAttribute("accessModifiers", AccessModifierOption.PUBLIC.toString());
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verify(checkConfig, getPath("InputJavadocVariableInner2.java"), expected);
}
Expand All @@ -120,15 +120,15 @@ public void testAnother4()
throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocVariableCheck.class);
checkConfig.addAttribute("scope", Scope.PUBLIC.getName());
checkConfig.addAttribute("accessModifiers", AccessModifierOption.PUBLIC.toString());
final String[] expected = {
"46:5: " + getCheckMessage(MSG_JAVADOC_MISSING),
};
verify(checkConfig, getPath("InputJavadocVariablePublicOnly2.java"), expected);
}

@Test
public void testScopes() throws Exception {
public void testAccessModifiersAll() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocVariableCheck.class);
final String[] expected = {
Expand Down Expand Up @@ -176,10 +176,13 @@ public void testScopes() throws Exception {
}

@Test
public void testScopes2() throws Exception {
public void testAccessModifiersPublicProtected() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocVariableCheck.class);
checkConfig.addAttribute("scope", Scope.PROTECTED.getName());
checkConfig.addAttribute(
"accessModifiers",
AccessModifierOption.PROTECTED.name() + "," + AccessModifierOption.PUBLIC.name()
);
final String[] expected = {
"10:5: " + getCheckMessage(MSG_JAVADOC_MISSING),
"11:5: " + getCheckMessage(MSG_JAVADOC_MISSING),
Expand All @@ -192,11 +195,13 @@ public void testScopes2() throws Exception {
}

@Test
public void testExcludeScope() throws Exception {
public void testAccessModifiersPrivatePackage() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocVariableCheck.class);
checkConfig.addAttribute("scope", Scope.PRIVATE.getName());
checkConfig.addAttribute("excludeScope", Scope.PROTECTED.getName());
checkConfig.addAttribute(
"accessModifiers",
AccessModifierOption.PRIVATE.name() + "," + AccessModifierOption.PACKAGE.name()
);
final String[] expected = {
"11:5: " + getCheckMessage(MSG_JAVADOC_MISSING),
"12:5: " + getCheckMessage(MSG_JAVADOC_MISSING),
Expand Down
59 changes: 12 additions & 47 deletions src/xdocs/config_javadoc.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2280,19 +2280,14 @@ class DatabaseConfiguration {}
<th>since</th>
</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>accessModifiers</td>
<td>Specify the access modifiers where Javadoc comments are checked.</td>
<td><a href="property_types.html#AccessModifierOption.5B.5D">AccessModifierOption[]
</a>
</td>
<td><code>public, protected, package, private</code></td>
<td>3.0</td>
</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>
</tr>
<tr>
<td>ignoreNamePattern</td>
<td>Specify the regexp to define variable names to ignore.</td>
Expand Down Expand Up @@ -2326,7 +2321,7 @@ class DatabaseConfiguration {}
</source>
<p>
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>
<source>
public class Test {
Expand All @@ -2343,48 +2338,18 @@ public int d; // violation, missing javadoc for public member
</source>

<p>
To configure the check for <code>public</code>
scope:
To configure the check for <code>package</code> and <code>private</code>
variables:
</p>

<source>
&lt;module name="JavadocVariable"&gt;
&lt;property name="scope" value="public"/&gt;
&lt;/module&gt;
</source>
<p>
This setting will report a violation if there
is no javadoc for <code>public</code> member.
</p>
<source>
public class Test {
private int a; // OK

/**
* Some description here
*/
private int b; // OK
protected int c; // OK
public int d; // violation, missing javadoc for public member
/*package*/ int e; // OK
}
</source>

<p>
To configure the check for members which are in <code>private</code>, but not in
<code>protected</code> scope:
</p>

<source>
&lt;module name="JavadocVariable"&gt;
&lt;property name="scope" value="private"/&gt;
&lt;property name="excludeScope" value="protected"/&gt;
&lt;property name="accessModifiers" value="package,private"/&gt;
&lt;/module&gt;
</source>
<p>
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>package</code> or <code>private</code> members.
</p>
<source>
public class Test {
Expand All @@ -2411,7 +2376,7 @@ public int d; // OK
</source>
<p>
This setting will report a violation if there is no
javadoc for any scope member and ignores members with
javadoc for a member with any scope and ignores members with
name <code>log</code> or <code>logger</code>.
</p>
<source>
Expand Down

0 comments on commit b5fbec6

Please sign in to comment.