Issue #3675: Replace Scope with AccessModifier in ParameterNameCheck …

…to avoid wrong scopes comparison

config/checkstyle_sevntu_checks.xml

@@ -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"/>

config/import-control.xml

@@ -61,6 +61,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"

src/it/java/com/google/checkstyle/test/chapter5naming/rule526parameternames/ParameterNameTest.java

@@ -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
@@ -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

src/main/java/com/puppycrawl/tools/checkstyle/api/AutomaticBean.java

@@ -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;
 
 /**
@@ -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;
 
@@ -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);
     }
 
     /**
@@ -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()) {
@@ -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()]);
+        }
+    }
 }

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AccessModifier.java

@@ -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));
+    }
+}