Issue checkstyle#3497: Let PkgImportRule and ClassImportRule inherit …

…from AbstractImportRule

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

@@ -0,0 +1,94 @@
+////////////////////////////////////////////////////////////////////////////////
+// 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.imports;
+
+/**
+ * Base class for import rules.
+ * @author Jochen Van de Velde
+ */
+abstract class AbstractImportRule {
+    /** Indicates whether to allow access or not. */
+    private final boolean allowed;
+
+    /** Indicates if the guard only applies to this package. */
+    private final boolean localOnly;
+
+    /**
+     * Indicates if the name is to be interpreted
+     * as a regular expression.
+     */
+    private final boolean regExp;
+
+    /**
+     * Constructs an instance.
+     * @param allow whether to allow access.
+     * @param localOnly whether the rules is to be applied locally only.
+     * @param regExp whether the name is to be interpreted as a regular
+     *        expression.
+     */
+    protected AbstractImportRule(final boolean allow, final boolean localOnly,
+        final boolean regExp) {
+        allowed = allow;
+        this.localOnly = localOnly;
+        this.regExp = regExp;
+    }
+
+    /**
+     * Verifies whether a package name is used.
+     * @param forImport the import to check.
+     * @return a result {@link AccessResult} indicating whether it can be used.
+     */
+    public abstract AccessResult verifyImport(String forImport);
+
+    /**
+     * @return whether the guard is to only be applied locally.
+     */
+    public boolean isLocalOnly() {
+        return localOnly;
+    }
+
+    /**
+     * @return whether the name is to be interpreted as a regular expression.
+     */
+    protected boolean isRegExp() {
+        return regExp;
+    }
+
+    /**
+     * Returns the appropriate {@link AccessResult} based on whether there
+     * was a match and if the rule is to allow access.
+     * @param matched indicates whether there was a match.
+     * @return An appropriate {@link AccessResult}.
+     */
+    protected AccessResult calculateResult(final boolean matched) {
+        AccessResult result = AccessResult.UNKNOWN;
+
+        if (matched) {
+            if (allowed) {
+                result = AccessResult.ALLOWED;
+            }
+            else {
+                result = AccessResult.DISALLOWED;
+            }
+        }
+
+        return result;
+    }
+}

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

@@ -23,20 +23,10 @@
  * Represents whether a class is allowed to be imported or not.
  * @author Oliver Burn
  */
-class ClassImportRule {
-    /** Indicates whether to allow access or not. */
-    private final boolean allowed;
+class ClassImportRule extends AbstractImportRule {
     /** Package to control access to. */
     private final String className;
 
-    /** Indicates if the rule only applies to this package. */
-    private final boolean localOnly;
-    /**
-     * Indicates if the class name is to be interpreted
-     * as a regular expression.
-     */
-    private final boolean regExp;
-
     /**
      * Constructs an instance.
      * @param allow whether to allow access.
@@ -47,9 +37,7 @@ class ClassImportRule {
      */
     ClassImportRule(final boolean allow, final boolean localOnly,
         final String className, final boolean regExp) {
-        allowed = allow;
-        this.localOnly = localOnly;
-        this.regExp = regExp;
+        super(allow, localOnly, regExp);
         this.className = className;
     }
 
@@ -58,10 +46,11 @@ class ClassImportRule {
      * @param forImport the import to check.
      * @return a result {@link AccessResult} indicating whether it can be used.
      */
+    @Override
     public AccessResult verifyImport(final String forImport) {
         final boolean classMatch;
 
-        if (regExp) {
+        if (isRegExp()) {
             classMatch = forImport.matches(className);
         }
         else {
@@ -70,32 +59,4 @@ public AccessResult verifyImport(final String forImport) {
 
         return calculateResult(classMatch);
     }
-
-    /**
-     * @return returns whether the guard is to only be applied locally.
-     */
-    public boolean isLocalOnly() {
-        return localOnly;
-    }
-
-    /**
-     * Returns the appropriate {@link AccessResult} based on whether there
-     * was a match and if the rule is to allow access.
-     * @param matched indicates whether there was a match.
-     * @return An appropriate {@link AccessResult}.
-     */
-    private AccessResult calculateResult(final boolean matched) {
-        AccessResult result = AccessResult.UNKNOWN;
-
-        if (matched) {
-            if (allowed) {
-                result = AccessResult.ALLOWED;
-            }
-            else {
-                result = AccessResult.DISALLOWED;
-            }
-        }
-
-        return result;
-    }
 }

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

@@ -112,21 +112,19 @@ else if (ALLOW_ELEMENT_NAME.equals(qName) || "disallow".equals(qName)) {
             final boolean isLocalOnly = attributes.getValue("local-only") != null;
             final String pkg = attributes.getValue(PKG_ATTRIBUTE_NAME);
             final boolean regex = containsRegexAttribute(attributes);
+            final AbstractImportRule rule;
             if (pkg == null) {
                 // handle class names which can be normal class names or regular
                 // expressions
                 final String clazz = safeGet(attributes, "class");
-                final ClassImportRule classRule =
-                    new ClassImportRule(isAllow, isLocalOnly, clazz, regex);
-                stack.peek().addClassImportRule(classRule);
+                rule = new ClassImportRule(isAllow, isLocalOnly, clazz, regex);
             }
             else {
                 final boolean exactMatch =
                         attributes.getValue("exact-match") != null;
-                final PkgImportRule pkgRule =
-                    new PkgImportRule(isAllow, isLocalOnly, pkg, exactMatch, regex);
-                stack.peek().addPkgImportRule(pkgRule);
+                rule = new PkgImportRule(isAllow, isLocalOnly, pkg, exactMatch, regex);
             }
+            stack.peek().addImportRule(rule);
         }
     }
 

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

@@ -39,10 +39,8 @@ class PkgControl {
     private static final Pattern DOT_PATTERN = Pattern.compile(DOT, Pattern.LITERAL);
     /** The regex for the package separator: "\\.". */
     private static final String DOT_REGEX = "\\.";
-    /** List of {@link PkgImportRule} objects to check. */
-    private final Deque<PkgImportRule> pkgRules = new LinkedList<>();
-    /** List of {@link ClassImportRule} objects to check. */
-    private final Deque<ClassImportRule> classRules = new LinkedList<>();
+    /** List of {@link AbstractImportRule} objects to check. */
+    private final Deque<AbstractImportRule> rules = new LinkedList<>();
     /** List of children {@link PkgControl} objects. */
     private final List<PkgControl> children = new ArrayList<>();
     /** The parent. Null indicates we are the root node. */
@@ -182,19 +180,11 @@ private static Pattern createPatternForExactMatch(String expression) {
     }
 
     /**
-     * Adds a {@link PkgImportRule} to the node.
+     * Adds a {@link AbstractImportRule} to the node.
      * @param rule the rule to be added.
      */
-    protected void addPkgImportRule(final PkgImportRule rule) {
-        pkgRules.addFirst(rule);
-    }
-
-    /**
-     * Adds a {@link ClassImportRule} to the node.
-     * @param rule the rule to be added.
-     */
-    protected void addClassImportRule(final ClassImportRule rule) {
-        classRules.addFirst(rule);
+    protected void addImportRule(final AbstractImportRule rule) {
+        rules.addFirst(rule);
     }
 
     /**
@@ -283,8 +273,7 @@ else if (parent == null) {
      */
     private AccessResult localCheckAccess(final String forImport,
         final String inPkg) {
-
-        for (PkgImportRule r : pkgRules) {
+        for (AbstractImportRule r : rules) {
             // Check if a PkgImportRule is only meant to be applied locally.
             if (r.isLocalOnly() && !matchesExactly(inPkg)) {
                 continue;
@@ -294,17 +283,6 @@ private AccessResult localCheckAccess(final String forImport,
                 return result;
             }
         }
-
-        for (ClassImportRule r : classRules) {
-            // Check if a ClassImportRule is only meant to be applied locally.
-            if (r.isLocalOnly() && !matchesExactly(inPkg)) {
-                continue;
-            }
-            final AccessResult result = r.verifyImport(forImport);
-            if (result != AccessResult.UNKNOWN) {
-                return result;
-            }
-        }
         return AccessResult.UNKNOWN;
     }
 

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

@@ -23,21 +23,12 @@
  * Represents whether a package is allowed to be imported or not.
  * @author Oliver Burn
  */
-class PkgImportRule {
-    /** Indicates whether to allow access or not. */
-    private final boolean allowed;
+class PkgImportRule extends AbstractImportRule {
     /** Package to control access to. */
     private final String pkgName;
 
     /** Indicates if the package name must be an exact match. */
     private final boolean exactMatch;
-    /** Indicates if the guard only applies to this package. */
-    private final boolean localOnly;
-    /**
-     * Indicates if the package name is to be interpreted
-     * as a regular expression.
-     */
-    private final boolean regExp;
 
     /**
      * Constructs an instance.
@@ -50,10 +41,8 @@ class PkgImportRule {
      */
     PkgImportRule(final boolean allow, final boolean localOnly,
         final String pkgName, final boolean exactMatch, final boolean regExp) {
-        allowed = allow;
-        this.localOnly = localOnly;
+        super(allow, localOnly, regExp);
         this.pkgName = pkgName;
-        this.regExp = regExp;
         this.exactMatch = exactMatch;
     }
 
@@ -62,6 +51,7 @@ class PkgImportRule {
      * @param forImport the import to check.
      * @return a result {@link AccessResult} indicating whether it can be used.
      */
+    @Override
     public AccessResult verifyImport(final String forImport) {
         // First check that we actually match the package.
         // Then check if matched and f we must be an exact match.
@@ -70,7 +60,7 @@ public AccessResult verifyImport(final String forImport) {
 
         boolean pkgMatch;
 
-        if (regExp) {
+        if (isRegExp()) {
             pkgMatch = forImport.matches(pkgName + "\\..*");
 
             if (pkgMatch && exactMatch) {
@@ -88,32 +78,4 @@ public AccessResult verifyImport(final String forImport) {
 
         return calculateResult(pkgMatch);
     }
-
-    /**
-     * @return returns whether the guard is to only be applied locally.
-     */
-    public boolean isLocalOnly() {
-        return localOnly;
-    }
-
-    /**
-     * Returns the appropriate {@link AccessResult} based on whether there
-     * was a match and if the rule is to allow access.
-     * @param matched indicates whether there was a match.
-     * @return An appropriate {@link AccessResult}.
-     */
-    private AccessResult calculateResult(final boolean matched) {
-        AccessResult result = AccessResult.UNKNOWN;
-
-        if (matched) {
-            if (allowed) {
-                result = AccessResult.ALLOWED;
-            }
-            else {
-                result = AccessResult.DISALLOWED;
-            }
-        }
-
-        return result;
-    }
 }

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

@@ -31,14 +31,14 @@ public class PkgControlRegExpTest {
 
     @Before
     public void setUp() {
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(false, false, ".*\\.(spring|lui)framework", false, true));
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(false, false, "org\\.hibernate", false, true));
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(true, false, "org\\.(apache|lui)\\.commons", false, true));
 
-        pcCommon.addPkgImportRule(
+        pcCommon.addImportRule(
             new PkgImportRule(true, false, "org\\.h.*", false, true));
     }
 

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

@@ -31,14 +31,14 @@ public class PkgControlTest {
 
     @Before
     public void setUp() {
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(false, false, "org.springframework", false, false));
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(false, false, "org.hibernate", false, false));
-        pcRoot.addPkgImportRule(
+        pcRoot.addImportRule(
             new PkgImportRule(true, false, "org.apache.commons", false, false));
 
-        pcCommon.addPkgImportRule(
+        pcCommon.addImportRule(
             new PkgImportRule(true, false, "org.hibernate", false, false));
     }