Skip to content

Commit

Permalink
Issue checkstyle#3675: Replace Scope with AccessModifier in Parameter…
Browse files Browse the repository at this point in the history
…NameCheck to avoid wrong scopes comparison
  • Loading branch information
MEZk committed Jan 22, 2017
1 parent 0cee093 commit 3e5a9e7
Show file tree
Hide file tree
Showing 15 changed files with 264 additions and 140 deletions.
5 changes: 4 additions & 1 deletion config/checkstyle_sevntu_checks.xml
Expand Up @@ -121,7 +121,10 @@
<module name="ForbidCertainImports">
<property name="packageNameRegexp" value=".+\.checkstyle\.api.*|.+\.checkstyle\.utils.*"/>
<property name="forbiddenImportsRegexp" value=".+\.checks\..+"/>
<property name="forbiddenImportsExcludesRegexp" value=""/>
<!-- AccessModifier is in util package (should be moved to api package) to disallow
its usage by API clients till https://github.com/checkstyle/checkstyle/issues/3511-->
<property name="forbiddenImportsExcludesRegexp"
value="^com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier$"/>
</module>
<module name="LineLengthExtended">
<property name="max" value="100"/>
Expand Down
4 changes: 4 additions & 0 deletions config/import-control.xml
Expand Up @@ -59,6 +59,10 @@
<allow pkg="java.text"/>
<allow class="com.puppycrawl.tools.checkstyle.grammars.CommentListener"
local-only="true"/>
<!-- AccessModifier is in util package (should be moved to api package) to disallow
its usage by API clients till https://github.com/checkstyle/checkstyle/issues/3511-->
<allow class="com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier"
local-only="true"/>
<allow class="com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaTokenTypes"
local-only="true"/>
<allow class="com.puppycrawl.tools.checkstyle.Utils"
Expand Down
Expand Up @@ -34,9 +34,9 @@
public class ParameterNameTest extends BaseCheckTestSupport {

private static final String MSG_KEY = "name.invalidPattern";
private static String privFormat;
private static String genealFormat;
private static String pubFormat;
private static Configuration privConfig;
private static Configuration generalConfig;
private static Configuration pubConfig;

@Override
Expand All @@ -51,34 +51,44 @@ public static void setConfigurationBuilder() throws CheckstyleException {

Assert.assertEquals(configs.size(), 2);

privConfig = configs.get(0);
Assert.assertEquals(privConfig.getAttribute("excludeScope"), "public");
privFormat = privConfig.getAttribute("format");
generalConfig = configs.get(0);
Assert.assertEquals(generalConfig.getAttribute("accessModifiers"),
"protected, package, private");
genealFormat = generalConfig.getAttribute("format");

pubConfig = configs.get(1);
Assert.assertEquals(pubConfig.getAttribute("scope"), "public");
Assert.assertEquals(pubConfig.getAttribute("accessModifiers"), "public");
pubFormat = pubConfig.getAttribute("format");
}

@Test
public void privParameterNameTest() throws Exception {
public void generalParameterNameTest() throws Exception {

final String[] expected = {
"8:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "$arg1", privFormat),
"9:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "ar$g2", privFormat),
"10:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "arg3$", privFormat),
"11:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "a_rg4", privFormat),
"12:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "_arg5", privFormat),
"13:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "arg6_", privFormat),
"14:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "aArg7", privFormat),
"15:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "aArg8", privFormat),
"16:21: " + getCheckMessage(privConfig.getMessages(), MSG_KEY, "aar_g", privFormat),
"8:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "$arg1", genealFormat),
"9:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "ar$g2", genealFormat),
"10:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "arg3$", genealFormat),
"11:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "a_rg4", genealFormat),
"12:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "_arg5", genealFormat),
"13:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "arg6_", genealFormat),
"14:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "aArg7", genealFormat),
"15:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "aArg8", genealFormat),
"16:21: "
+ getCheckMessage(generalConfig.getMessages(), MSG_KEY, "aar_g", genealFormat),
};

final String filePath = getPath("InputParameterNameSimplePriv.java");
final String filePath = getPath("InputParameterNameSimpleGeneral.java");

final Integer[] warnList = getLinesWithWarn(filePath);
verify(privConfig, filePath, expected, warnList);
verify(generalConfig, filePath, expected, warnList);
}

@Test
Expand Down
Expand Up @@ -45,6 +45,7 @@
import org.apache.commons.beanutils.converters.LongConverter;
import org.apache.commons.beanutils.converters.ShortConverter;

import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier;
import com.puppycrawl.tools.checkstyle.utils.CommonUtils;

/**
Expand All @@ -54,6 +55,10 @@
*/
public class AutomaticBean
implements Configurable, Contextualizable {

/** Comma separator for StringTokenizer. */
private static final String COMMA_SEPARATOR = ",";

/** The configuration of this bean. */
private Configuration configuration;

Expand Down Expand Up @@ -130,6 +135,7 @@ private static void registerCustomTypes(ConvertUtilsBean cub) {
cub.register(new ServerityLevelConverter(), SeverityLevel.class);
cub.register(new ScopeConverter(), Scope.class);
cub.register(new UriConverter(), URI.class);
cub.register(new RelaxedAccessModifierArrayConverter(), AccessModifier[].class);
}

/**
Expand Down Expand Up @@ -328,7 +334,7 @@ private static class RelaxedStringArrayConverter implements Converter {
public Object convert(Class type, Object value) {
// Convert to a String and trim it for the tokenizer.
final StringTokenizer tokenizer = new StringTokenizer(
value.toString().trim(), ",");
value.toString().trim(), COMMA_SEPARATOR);
final List<String> result = new ArrayList<>();

while (tokenizer.hasMoreTokens()) {
Expand All @@ -339,4 +345,28 @@ public Object convert(Class type, Object value) {
return result.toArray(new String[result.size()]);
}
}

/**
* A converter that converts strings to {@link AccessModifier}.
* This implementation does not care whether the array elements contain characters like '_'.
* The normal {@link ArrayConverter} class has problems with this character.
*/
private static class RelaxedAccessModifierArrayConverter implements Converter {

@SuppressWarnings({"unchecked", "rawtypes"})
@Override
public Object convert(Class type, Object value) {
// Converts to a String and trims it for the tokenizer.
final StringTokenizer tokenizer = new StringTokenizer(
value.toString().trim(), COMMA_SEPARATOR);
final List<AccessModifier> result = new ArrayList<>();

while (tokenizer.hasMoreTokens()) {
final String token = tokenizer.nextToken();
result.add(AccessModifier.getInstance(token.trim()));
}

return result.toArray(new AccessModifier[result.size()]);
}
}
}
@@ -0,0 +1,63 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2016 the original author or authors.
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
////////////////////////////////////////////////////////////////////////////////

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

import java.util.Locale;

/**
* This enum represents access modifiers.
* Access modifiers names are taken from JLS:
* https://docs.oracle.com/javase/specs/jls/se8/html/jls-6.html#jls-6.6
*
* @author Andrei Selkin
*/
public enum AccessModifier {
/** Public access modifier. */
PUBLIC,
/** Protected access modifier. */
PROTECTED,
/** Package access modifier. */
PACKAGE,
/** Private access modifier. */
PRIVATE;

@Override
public String toString() {
return getName();
}

public String getName() {
return name().toLowerCase(Locale.ENGLISH);
}

/**
* Factory method which returns an AccessModifier instance that corresponds to the
* given access modifier name represented as a {@link String}.
* The access modifier name can be formatted both as lower case or upper case string.
* For example, passing PACKAGE or package as a modifier name
* will return {@link AccessModifier#PACKAGE}.
*
* @param modifierName access modifier name represented as a {@link String}.
* @return the AccessModifier associated with given access modifier name.
*/
public static AccessModifier getInstance(String modifierName) {
return valueOf(AccessModifier.class, modifierName.trim().toUpperCase(Locale.ENGLISH));
}
}

0 comments on commit 3e5a9e7

Please sign in to comment.