Issue #3473: ParameterNameCheck: new scope and excludeScope properties
This commit is contained in:
parent
81ad4595fe
commit
b99c7d976c
|
|
@ -63,6 +63,11 @@ public class ParameterNameTest extends BaseCheckTestSupport {
|
|||
"26:21: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "bB", format),
|
||||
"49:22: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "llll_llll", format),
|
||||
"50:21: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "bB", format),
|
||||
"60:23: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "p", format),
|
||||
"63:24: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "p", format),
|
||||
"69:31: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "p", format),
|
||||
"74:41: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "p", format),
|
||||
"77:44: " + getCheckMessage(checkConfig.getMessages(), MSG_KEY, "p", format),
|
||||
};
|
||||
|
||||
final String filePath = getPath("InputParameterNameSimple.java");
|
||||
|
|
|
|||
|
|
@ -49,3 +49,30 @@ enum MyEnum1
|
|||
long llll_llll, //warn
|
||||
boolean bB) {} //warn
|
||||
}
|
||||
|
||||
/** Test public vs private method parameter naming check. */
|
||||
class InputParameterNameSimple
|
||||
{
|
||||
/** Valid: public and more than one char Long */
|
||||
public void a(int par, int parA) {}
|
||||
|
||||
/** Invalid: public and one char long */
|
||||
public void b(int p) {} //warn
|
||||
|
||||
/** Invalid: private and one char long. */
|
||||
private void c(int p) {} //warn
|
||||
|
||||
/** Holder for inner anonymous classes */
|
||||
private void d(int param) {
|
||||
new Object() {
|
||||
/** Invalid: public and one char long. */
|
||||
public void e(int p) { } //warn
|
||||
};
|
||||
}
|
||||
|
||||
/** Invalid: public constructor and one char long */
|
||||
public InputParameterNameSimple(int p) { } // warn
|
||||
|
||||
/** Invalid: private constructor and one char long */
|
||||
private InputParameterNameSimple(float p) { } // warn
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,8 +22,10 @@ package com.puppycrawl.tools.checkstyle.checks.naming;
|
|||
import java.util.Optional;
|
||||
|
||||
import com.puppycrawl.tools.checkstyle.api.DetailAST;
|
||||
import com.puppycrawl.tools.checkstyle.api.Scope;
|
||||
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
|
||||
import com.puppycrawl.tools.checkstyle.utils.CheckUtils;
|
||||
import com.puppycrawl.tools.checkstyle.utils.ScopeUtils;
|
||||
|
||||
/**
|
||||
* <p>
|
||||
|
|
@ -33,9 +35,13 @@ import com.puppycrawl.tools.checkstyle.utils.CheckUtils;
|
|||
* and defaults to
|
||||
* <strong>^[a-z][a-zA-Z0-9]*$</strong>.
|
||||
* </p>
|
||||
* <p>The check has the following option:</p>
|
||||
* <p>The check has the following options:</p>
|
||||
* <p><b>ignoreOverridden</b> - allows to skip methods with Override annotation from
|
||||
* validation. Default values is <b>false</b> .</p>
|
||||
* <p><b>scope</b> - visibility scope of methods to be checked.
|
||||
* Default value is <b>anoninner</b> .</p>
|
||||
* <p><b>excludeScope</b> - visibility scope of methods not to be checked.
|
||||
* Default value is <b>null</b> .</p>
|
||||
* <p>
|
||||
* An example of how to configure the check is:
|
||||
* </p>
|
||||
|
|
@ -72,6 +78,12 @@ public class ParameterNameCheck
|
|||
*/
|
||||
private boolean ignoreOverridden;
|
||||
|
||||
/** The visibility scope where methods are checked. */
|
||||
private Scope scope = Scope.ANONINNER;
|
||||
|
||||
/** The visibility scope where methods shouldn't be checked. */
|
||||
private Scope excludeScope;
|
||||
|
||||
/**
|
||||
* Creates a new {@code ParameterNameCheck} instance.
|
||||
*/
|
||||
|
|
@ -88,6 +100,22 @@ public class ParameterNameCheck
|
|||
this.ignoreOverridden = ignoreOverridden;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the scope.
|
||||
* @param from a {@code String} value
|
||||
*/
|
||||
public void setScope(String from) {
|
||||
scope = Scope.getInstance(from);
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the excludeScope.
|
||||
* @param excludeScope a {@code String} value
|
||||
*/
|
||||
public void setExcludeScope(String excludeScope) {
|
||||
this.excludeScope = Scope.getInstance(excludeScope);
|
||||
}
|
||||
|
||||
@Override
|
||||
public int[] getDefaultTokens() {
|
||||
return getAcceptableTokens();
|
||||
|
|
@ -106,14 +134,55 @@ public class ParameterNameCheck
|
|||
@Override
|
||||
protected boolean mustCheckName(DetailAST ast) {
|
||||
boolean checkName = true;
|
||||
final boolean isDefault = scope == Scope.ANONINNER && excludeScope == null;
|
||||
|
||||
if (ignoreOverridden && isOverriddenMethod(ast)
|
||||
|| ast.getParent().getType() == TokenTypes.LITERAL_CATCH
|
||||
|| CheckUtils.isReceiverParameter(ast)) {
|
||||
|| CheckUtils.isReceiverParameter(ast)
|
||||
|| !isDefault && !matchScope(calculateScope(ast))) {
|
||||
checkName = false;
|
||||
}
|
||||
return checkName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the scope for the method/constructor at the specified AST. If
|
||||
* the method is in an interface or annotation block, the scope is assumed
|
||||
* to be public.
|
||||
*
|
||||
* @param ast the token of the method/constructor
|
||||
* @return the scope of the method/constructor
|
||||
*/
|
||||
private static Scope calculateScope(final DetailAST ast) {
|
||||
final DetailAST params = ast.getParent();
|
||||
final DetailAST meth = params.getParent();
|
||||
Scope scope = Scope.PRIVATE;
|
||||
|
||||
if (meth.getType() == TokenTypes.METHOD_DEF
|
||||
|| meth.getType() == TokenTypes.CTOR_DEF) {
|
||||
if (ScopeUtils.isInInterfaceOrAnnotationBlock(ast)) {
|
||||
scope = Scope.PUBLIC;
|
||||
}
|
||||
else {
|
||||
final DetailAST mods = meth.findFirstToken(TokenTypes.MODIFIERS);
|
||||
scope = ScopeUtils.getScopeFromMods(mods);
|
||||
}
|
||||
}
|
||||
|
||||
return scope;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a method has the correct scope to be checked.
|
||||
* @param nodeScope the scope of the method
|
||||
* @return whether the method matches the expected scope
|
||||
*/
|
||||
private boolean matchScope(final Scope nodeScope) {
|
||||
return nodeScope.isIn(scope)
|
||||
&& (excludeScope == null
|
||||
|| !nodeScope.isIn(excludeScope));
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a method is annotated with Override annotation.
|
||||
* @param ast method parameter definition token.
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import org.junit.Test;
|
|||
|
||||
import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
|
||||
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.utils.CommonUtils;
|
||||
|
||||
|
|
@ -138,6 +139,68 @@ public class ParameterNameCheckTest
|
|||
verify(checkConfig, getPath("InputOverrideAnnotation.java"), expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testScope()
|
||||
throws Exception {
|
||||
final DefaultConfiguration checkConfig =
|
||||
createCheckConfig(ParameterNameCheck.class);
|
||||
checkConfig.addAttribute("format", "^h$");
|
||||
checkConfig.addAttribute("scope", Scope.PUBLIC.getName());
|
||||
|
||||
final String pattern = "^h$";
|
||||
|
||||
final String[] expected = {
|
||||
"5:27: " + getCheckMessage(MSG_INVALID_PATTERN, "pubconstr", pattern),
|
||||
"9:31: " + getCheckMessage(MSG_INVALID_PATTERN, "inner", pattern),
|
||||
"19:24: " + getCheckMessage(MSG_INVALID_PATTERN, "pubpub", pattern),
|
||||
"30:21: " + getCheckMessage(MSG_INVALID_PATTERN, "pubifc", pattern),
|
||||
"44:24: " + getCheckMessage(MSG_INVALID_PATTERN, "packpub", pattern),
|
||||
"60:21: " + getCheckMessage(MSG_INVALID_PATTERN, "packifc", pattern),
|
||||
};
|
||||
verify(checkConfig, getPath("InputScope.java"), expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testExcludeScope()
|
||||
throws Exception {
|
||||
final DefaultConfiguration checkConfig =
|
||||
createCheckConfig(ParameterNameCheck.class);
|
||||
checkConfig.addAttribute("format", "^h$");
|
||||
checkConfig.addAttribute("excludeScope", Scope.PROTECTED.getName());
|
||||
|
||||
final String pattern = "^h$";
|
||||
|
||||
final String[] expected = {
|
||||
"23:17: " + getCheckMessage(MSG_INVALID_PATTERN, "pubpack", pattern),
|
||||
"25:25: " + getCheckMessage(MSG_INVALID_PATTERN, "pubpriv", pattern),
|
||||
"48:17: " + getCheckMessage(MSG_INVALID_PATTERN, "packpack", pattern),
|
||||
"50:25: " + getCheckMessage(MSG_INVALID_PATTERN, "packpriv", pattern),
|
||||
"68:27: " + getCheckMessage(MSG_INVALID_PATTERN, "lexp", pattern),
|
||||
"70:23: " + getCheckMessage(MSG_INVALID_PATTERN, "limp", pattern),
|
||||
};
|
||||
verify(checkConfig, getPath("InputScope.java"), expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testScopeExcludeScope()
|
||||
throws Exception {
|
||||
final DefaultConfiguration checkConfig =
|
||||
createCheckConfig(ParameterNameCheck.class);
|
||||
checkConfig.addAttribute("format", "^h$");
|
||||
checkConfig.addAttribute("scope", Scope.PACKAGE.getName());
|
||||
checkConfig.addAttribute("excludeScope", Scope.PUBLIC.getName());
|
||||
|
||||
final String pattern = "^h$";
|
||||
|
||||
final String[] expected = {
|
||||
"21:27: " + getCheckMessage(MSG_INVALID_PATTERN, "pubprot", pattern),
|
||||
"23:17: " + getCheckMessage(MSG_INVALID_PATTERN, "pubpack", pattern),
|
||||
"46:27: " + getCheckMessage(MSG_INVALID_PATTERN, "packprot", pattern),
|
||||
"48:17: " + getCheckMessage(MSG_INVALID_PATTERN, "packpack", pattern),
|
||||
};
|
||||
verify(checkConfig, getPath("InputScope.java"), expected);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIsOverriddenNoNullPointerException()
|
||||
throws Exception {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,74 @@
|
|||
package com.puppycrawl.tools.checkstyle.checks.naming;
|
||||
|
||||
public class InputScope {
|
||||
|
||||
public InputScope(int pubconstr) {}
|
||||
|
||||
public void v1(int h) {
|
||||
new Object () {
|
||||
public void i(int inner) {}
|
||||
};
|
||||
}
|
||||
|
||||
protected void v4(int h) {}
|
||||
|
||||
void v2(int h) {}
|
||||
|
||||
private void v3(int h) {}
|
||||
|
||||
public void i1(int pubpub) {}
|
||||
|
||||
protected void i4(int pubprot) {}
|
||||
|
||||
void i2(int pubpack) {}
|
||||
|
||||
private void i3(int pubpriv) {}
|
||||
|
||||
public interface InterfaceScope {
|
||||
void v1(int h);
|
||||
|
||||
void i1(int pubifc);
|
||||
}
|
||||
}
|
||||
|
||||
class PrivateScope {
|
||||
|
||||
public void v1(int h) {}
|
||||
|
||||
protected void v4(int h) {}
|
||||
|
||||
void v2(int h) {}
|
||||
|
||||
private void v3(int h) {}
|
||||
|
||||
public void i1(int packpub) {}
|
||||
|
||||
protected void i4(int packprot) {}
|
||||
|
||||
void i2(int packpack) {}
|
||||
|
||||
private void i3(int packpriv) {
|
||||
try {
|
||||
/* Make sure catch var is ignored */
|
||||
} catch (Exception exc) {
|
||||
}
|
||||
}
|
||||
|
||||
interface InterfaceScope {
|
||||
void v1(int h);
|
||||
|
||||
void i1(int packifc);
|
||||
}
|
||||
|
||||
interface FuncIfc {
|
||||
void a(int h);
|
||||
}
|
||||
|
||||
public void l() {
|
||||
FuncIfc l1 = (int lexp)->{};
|
||||
|
||||
FuncIfc l2 = (limp)->{};
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -1145,7 +1145,8 @@ class MyClass {
|
|||
<subsection name="Description">
|
||||
<p>
|
||||
Checks that method and <code>catch</code> parameter names conform to a format specified
|
||||
by the format property.
|
||||
by the format property. By using <code>scope</code> and <code>excludeScope</code> properties
|
||||
it is possible to specify different formats for methods at different visibility levels.
|
||||
</p>
|
||||
</subsection>
|
||||
|
||||
|
|
@ -1179,6 +1180,18 @@ public boolean equals(Object o) {
|
|||
<td><a href="property_types.html#boolean">Boolean</a></td>
|
||||
<td><code>false</code></td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>scope</td>
|
||||
<td>Visibility scope of methods where parameters are checked.</td>
|
||||
<td><a href="property_types.html#scope">scope</a></td>
|
||||
<td><code>anoninner</code></td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>excludeScope</td>
|
||||
<td>Visibility scope of methods where parameters are not checked.</td>
|
||||
<td><a href="property_types.html#scope">scope</a></td>
|
||||
<td><code>null</code></td>
|
||||
</tr>
|
||||
</table>
|
||||
</subsection>
|
||||
|
||||
|
|
@ -1205,6 +1218,25 @@ public boolean equals(Object o) {
|
|||
<source>
|
||||
<module name="ParameterName">
|
||||
<property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
|
||||
</module>
|
||||
</source>
|
||||
<p>
|
||||
The following configuration checks that the parameters always start with two
|
||||
lowercase characters and, in addition, that public method parameters cannot be one character
|
||||
long:
|
||||
</p>
|
||||
<source>
|
||||
<module name="ParameterName">
|
||||
<property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
|
||||
<property name="excludeScope" value="public"/>
|
||||
<message key="name.invalidPattern"
|
||||
value="Parameter name ''{0}'' must match pattern ''{1}''."/>
|
||||
</module>
|
||||
<module name="ParameterName">
|
||||
<property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9]*$"/>
|
||||
<property name="scope" value="public"/>
|
||||
<message key="name.invalidPattern"
|
||||
value="Parameter name ''{0}'' must match pattern ''{1}''"/>
|
||||
</module>
|
||||
</source>
|
||||
</subsection>
|
||||
|
|
|
|||
Loading…
Reference in New Issue