Issue checkstyle#2999: allow regex in subpackage and importcontrol el…

…ement

config/import-control.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <!DOCTYPE import-control PUBLIC
-    "-//Puppy Crawl//DTD Import Control 1.1//EN"
-    "http://www.puppycrawl.com/dtds/import_control_1_1.dtd">
+    "-//Puppy Crawl//DTD Import Control 1.2//EN"
+    "http://checkstyle.sourceforge.net/dtds/import_control_1_2.dtd">
 
 <import-control pkg="com.puppycrawl.tools.checkstyle">
 

src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java

@@ -96,11 +96,13 @@ public void startElement(final String namespaceUri,
             throws SAXException {
         if ("import-control".equals(qName)) {
             final String pkg = safeGet(attributes, PKG_ATTRIBUTE_NAME);
-            stack.push(new PkgControl(pkg));
+            final boolean regex = containsRegexAttribute(attributes);
+            stack.push(new PkgControl(pkg, regex));
         }
         else if (SUBPACKAGE_ELEMENT_NAME.equals(qName)) {
             final String name = safeGet(attributes, "name");
-            stack.push(new PkgControl(stack.peek(), name));
+            final boolean regex = containsRegexAttribute(attributes);
+            stack.push(new PkgControl(stack.peek(), name, regex));
         }
         else if (ALLOW_ELEMENT_NAME.equals(qName) || "disallow".equals(qName)) {
             // Need to handle either "pkg" or "class" attribute.
@@ -109,7 +111,7 @@ else if (ALLOW_ELEMENT_NAME.equals(qName) || "disallow".equals(qName)) {
             final boolean isAllow = ALLOW_ELEMENT_NAME.equals(qName);
             final boolean isLocalOnly = attributes.getValue("local-only") != null;
             final String pkg = attributes.getValue(PKG_ATTRIBUTE_NAME);
-            final boolean regex = attributes.getValue("regex") != null;
+            final boolean regex = containsRegexAttribute(attributes);
             final Guard guard;
             if (pkg == null) {
                 // handle class names which can be normal class names or regular
@@ -128,6 +130,15 @@ else if (ALLOW_ELEMENT_NAME.equals(qName) || "disallow".equals(qName)) {
         }
     }
 
+    /**
+     * Check if the given attributes contain the regex attribute.
+     * @param attributes the attributes.
+     * @return if the regex attribute is contained.
+     */
+    private static boolean containsRegexAttribute(final Attributes attributes) {
+        return attributes.getValue("regex") != null;
+    }
+
     @Override
     public void endElement(final String namespaceUri, final String localName,
         final String qName) {

src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/PkgControl.java

@@ -21,6 +21,7 @@
 
 import java.util.Deque;
 import java.util.List;
+import java.util.regex.Pattern;
 
 import com.google.common.collect.Lists;
 
@@ -32,6 +33,14 @@
  * @author Oliver Burn
  */
 class PkgControl {
+    /** The package separator: "." */
+    private static final String DOT = ".";
+    /** A pattern matching the package separator: "." */
+    private static final Pattern DOT_PATTERN = Pattern.compile(DOT, Pattern.LITERAL);
+    /** The regex for the package separator: "\\.". */
+    private static final String DOT_REGEX = "\\.";
+    /** A pattern matching a string that contains a group (ignoring non-alternation chars). */
+    private static final Pattern GROUP_PATTERN = Pattern.compile("[^|]*\\(.*\\)[^|]*");
     /** List of {@link Guard} objects to check. */
     private final Deque<Guard> guards = Lists.newLinkedList();
     /** List of children {@link PkgControl} objects. */
@@ -40,27 +49,123 @@ class PkgControl {
     private final PkgControl parent;
     /** The full package name for the node. */
     private final String fullPackage;
+    /**
+     * The regex pattern for partial match (exact and for subpackages) - only not
+     * null if regex is true.
+     */
+    private final Pattern patternForPartialMatch;
+    /** The regex pattern for exact matches - only not null if regex is true. */
+    private final Pattern patternForExactMatch;
 
     /**
      * Construct a root node.
      * @param pkgName the name of the package.
+     * @param regex flags interpretation of pkgName as regex pattern.
      */
-    PkgControl(final String pkgName) {
+    PkgControl(final String pkgName, final boolean regex) {
         parent = null;
-        fullPackage = pkgName;
+        fullPackage = addGroupingIfNecessary(pkgName, regex);
+        if (regex) {
+            patternForPartialMatch = createPatternForChildPackages();
+            patternForExactMatch = createPatternForExactMatch();
+        }
+        else {
+            patternForPartialMatch = null;
+            patternForExactMatch = null;
+        }
     }
 
     /**
      * Construct a child node.
      * @param parent the parent node.
      * @param subPkg the sub package name.
+     * @param regex flags interpretation of subPkg as regex pattern.
      */
-    PkgControl(final PkgControl parent, final String subPkg) {
+    PkgControl(final PkgControl parent, final String subPkg, final boolean regex) {
         this.parent = parent;
-        fullPackage = parent.fullPackage + "." + subPkg;
+        if (regex || parent.isRegex()) {
+            // regex gets inherited
+            final String parentRegex = ensureRegex(parent.fullPackage, parent.isRegex());
+            final String thisRegex = ensureRegex(subPkg, regex);
+            fullPackage = parentRegex + DOT_REGEX + thisRegex;
+            patternForPartialMatch = createPatternForChildPackages();
+            patternForExactMatch = createPatternForExactMatch();
+        }
+        else {
+            fullPackage = parent.fullPackage + DOT + subPkg;
+            patternForPartialMatch = null;
+            patternForExactMatch = null;
+        }
         parent.children.add(this);
     }
 
+    /**
+     * Returns a regex by converting the string if necessary.
+     * @param input the input string.
+     * @param isRegex flag.
+     * @return a regex string.
+     */
+    private static String ensureRegex(final String input, final boolean isRegex) {
+        final String result;
+        if (isRegex) {
+            result = input;
+        }
+        else {
+            result = toRegex(input);
+        }
+        return result;
+    }
+
+    /**
+     * Converts a normal package name into a regex pattern by escaping all
+     * special characters that may occur in a java package name.
+     * @param input the input string.
+     * @return a regex string.
+     */
+    private static String toRegex(String input) {
+        return DOT_PATTERN.matcher(input).replaceAll(DOT_REGEX);
+    }
+
+    /**
+     * Handle alternation via pipe symbol by enclosing the pattern in a (non-capturing)
+     * group unless the pattern already is enclosed in a group.
+     * @param pkgName the name of the package.
+     * @param regex flags interpretation of pkgName as regex pattern.
+     * @return a Pattern if regex is true.
+     */
+    private static String addGroupingIfNecessary(final String pkgName, final boolean regex) {
+        final String result;
+        if (regex && pkgName.contains("|") && !GROUP_PATTERN.matcher(pkgName).matches()) {
+            result = "(?:" + pkgName + ")";
+        }
+        else {
+            result = pkgName;
+        }
+        return result;
+    }
+
+    /**
+     * Creates a Pattern from fullPackage that matches exactly or child packages.
+     * @return a Pattern.
+     */
+    private Pattern createPatternForChildPackages() {
+        return Pattern.compile(fullPackage + "(?:\\..*)?");
+    }
+
+    /**
+     * Creates a Pattern from fullPackage.
+     * @return a Pattern.
+     */
+    private Pattern createPatternForExactMatch() {
+        return Pattern.compile(fullPackage);
+    }
+
+    /** If this package represents a regular expression.
+     * @return if this PkgControl is defined as a regex. */
+    private boolean isRegex() {
+        return patternForPartialMatch != null;
+    }
+
     /**
      * Adds a guard to the node.
      * @param thug the guard to be added.
@@ -77,9 +182,7 @@ protected void addGuard(final Guard thug) {
     public PkgControl locateFinest(final String forPkg) {
         PkgControl finestMatch = null;
         // Check if we are a match.
-        // This algorithm should be improved to check for a trailing "."
-        // or nothing following.
-        if (forPkg.startsWith(fullPackage)) {
+        if (matchesAtFront(forPkg)) {
             // If there won't be match so I am the best there is.
             finestMatch = this;
             // Check if any of the children match.
@@ -94,6 +197,52 @@ public PkgControl locateFinest(final String forPkg) {
         return finestMatch;
     }
 
+    /** Matches other package name exactly or partially at front.
+     * @param pkg the package to compare with.
+     * @return if it matches. */
+    private boolean matchesAtFront(final String pkg) {
+        final boolean result;
+        if (isRegex()) {
+            result = patternForPartialMatch.matcher(pkg).matches();
+        }
+        else {
+            result = matchesAtFrontNoRegex(pkg);
+        }
+        return result;
+    }
+
+    /**
+     * Non-regex case. Ensure a trailing dot for subpackages, i.e. "com.puppy"
+     * will match "com.puppy.crawl" but not "com.puppycrawl.tools".
+     * @param pkg the package to compare with.
+     * @return if it matches.
+     */
+    private boolean matchesAtFrontNoRegex(final String pkg) {
+        final boolean result;
+        if (pkg.startsWith(fullPackage)) {
+            result = pkg.length() == fullPackage.length()
+                    || pkg.charAt(fullPackage.length()) == '.';
+        }
+        else {
+            result = false;
+        }
+        return result;
+    }
+
+    /** Check for equality of this with pkg
+     * @param pkg the package to compare with.
+     * @return if it matches. */
+    private boolean matchesExactly(final String pkg) {
+        final boolean result;
+        if (isRegex()) {
+            result = patternForExactMatch.matcher(pkg).matches();
+        }
+        else {
+            result = fullPackage.equals(pkg);
+        }
+        return result;
+    }
+
     /**
      * Returns whether a package is allowed to be used. The algorithm checks
      * with the current node for a result, and if none is found then calls
@@ -131,7 +280,7 @@ private AccessResult localCheckAccess(final String forImport,
         final String inPkg) {
         for (Guard g : guards) {
             // Check if a Guard is only meant to be applied locally.
-            if (g.isLocalOnly() && !fullPackage.equals(inPkg)) {
+            if (g.isLocalOnly() && !matchesExactly(inPkg)) {
                 continue;
             }
             final AccessResult result = g.verifyImport(forImport);

src/main/resources/com/puppycrawl/tools/checkstyle/checks/imports/import_control_1_2.dtd

@@ -0,0 +1,78 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!-- Add the following to any file that is to be validated against this DTD:
+
+<!DOCTYPE module PUBLIC
+    "-//Puppy Crawl//DTD Import Control 1.2//EN"
+    "http://checkstyle.sourceforge.net/dtds/import_control_1_2.dtd">
+-->
+
+<!--
+  The root element of the configuration file.
+-->
+<!ELEMENT import-control ((allow|disallow)*,subpackage*)>
+
+<!--
+  pkg - The root package to be checked. For example "com.puppycrawl".
+-->
+<!ATTLIST import-control
+  pkg CDATA #REQUIRED
+  regex (true) #IMPLIED>
+
+<!--
+  Represents a subpackage of the parent element.
+-->
+<!ELEMENT subpackage ((allow|disallow)*,subpackage*)>
+
+<!--
+  name - The name of the subpackage. For example if the name is "tools"
+  and the pa the parent is "com.puppycrawl", then it corresponds to the
+  package "com.puppycrawl.tools". If the regex attribute is "true" the
+  name is interpreted as a regular expression.
+-->
+<!ATTLIST subpackage
+  name CDATA #REQUIRED
+  regex (true) #IMPLIED>
+
+<!--
+  Represents attributes for a guard which can either allow or disallow
+  access.
+
+  pkg - The fully qualified name of the package to guard. Cannot be
+  specified in conjunction with "class".
+
+  class - The fully qualified name of the class to guard. Cannot be
+  specified in conjunction with "pkg".
+
+  exact-match - Only valid with "pkg". Specifies whether the package
+  name matching should be exact. For example, the pkg
+  "com.puppycrawl.tools" will match the import
+  "com.puppycrawl.tools.checkstyle.api.*" when the option is not set,
+  but will not match is the option is set.
+
+  local-only - Indicates that the guard is to apply only to the current
+  package and not to subpackages.
+
+  regex - Indicates that the class or package name has to be interpreted as
+  regular expression.
+-->
+<!ENTITY % attlist.guard "
+  pkg CDATA #IMPLIED
+  exact-match (true) #IMPLIED
+  class CDATA #IMPLIED
+  local-only (true) #IMPLIED
+  regex (true) #IMPLIED">
+
+<!--
+  Represents a guard that will allow access.
+-->
+<!ELEMENT allow EMPTY>
+<!ATTLIST allow
+  %attlist.guard;>
+
+<!--
+  Represents a guard that will disallow access.
+-->
+<!ELEMENT disallow EMPTY>
+<!ATTLIST disallow
+  %attlist.guard;>

src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java

@@ -177,6 +177,30 @@ public void testTwoRegExp() throws Exception {
         verify(checkConfig, getPath("InputImportControl.java"), expected);
     }
 
+    @Test
+    public void testPkgRegExpInParent() throws Exception {
+        testRegExpInPackage("import-control_pkg-re-in-parent.xml");
+    }
+
+    @Test
+    public void testPkgRegExpInChild() throws Exception {
+        testRegExpInPackage("import-control_pkg-re-in-child.xml");
+    }
+
+    @Test
+    public void testPkgRegExpInBoth() throws Exception {
+        testRegExpInPackage("import-control_pkg-re-in-both.xml");
+    }
+
+    // all import-control_pkg-re* files should be equivalent so use one test for all
+    private void testRegExpInPackage(String file) throws Exception {
+        final DefaultConfiguration checkConfig = createCheckConfig(ImportControlCheck.class);
+        checkConfig.addAttribute("file", getPath(file));
+        final String[] expected = {"5:1: " + getCheckMessage(MSG_DISALLOWED, "java.io.File")};
+
+        verify(checkConfig, getPath("InputImportControl.java"), expected);
+    }
+
     @Test
     public void testGetAcceptableTokens() {
         final ImportControlCheck testCheckObject =