TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Added a lot more configuration options for the ImportControlCheck that 

came out of using at work. I still need to improve the unit tests and
properly document the support options. Wanted to check in now for
backup.
This commit is contained in:
Oliver Burn 2005-07-31 12:27:44 +00:00
parent 28c5d6f062
commit a5d30bf353
8 changed files with 185 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
import-control.xml
View File

@ -15,23 +15,24 @@
<allow pkg="org.apache.commons.logging"/>
<allow pkg="org.xml.sax"/>
<!-- Want to be local -->
<allow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
<allow pkg="java.security"/>
<allow pkg="org.apache.commons.cli"/>
<allow pkg="org.apache.tools.ant"/>
<!-- The local ones -->
<allow pkg="com.puppycrawl.tools.checkstyle.grammars" local-only="selected"/>
<allow class="java.security.MessageDigest" local-only="selected"/>
<allow pkg="org.apache.commons.cli" local-only="selected"/>
<allow pkg="org.apache.tools.ant" local-only="selected"/>
<subpackage name="api">
<allow pkg="java.beans"/>
<allow pkg="java.lang.reflect"/>
<allow pkg="java.text"/>
<disallow pkg="org.apache.tools.ant"/>
<allow class="com.puppycrawl.tools.checkstyle.grammars.CommentListener"
local-only="selected"/>
<allow class="com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaTokenTypes"
local-only="selected"/>
</subpackage>
<subpackage name="checks">
<allow pkg="com.puppycrawl.tools.checkstyle.checks"/>
<disallow pkg="org.apache.tools.ant"/>
<disallow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
<subpackage name="indentation">
<allow pkg="java.lang.reflect"/>
@ -42,25 +43,14 @@
<subpackage name="doclets">
<allow pkg="com.sun.javadoc"/>
<disallow pkg="org.apache.tools.ant"/>
<disallow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
</subpackage>
<subpackage name="filters">
<allow pkg="java.lang.ref"/>
<disallow pkg="org.apache.tools.ant"/>
<disallow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
</subpackage>
<subpackage name="grammars">
<disallow pkg="org.apache.tools.ant"/>
<disallow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
</subpackage>
<subpackage name="gui">
<allow pkg="java.awt"/>
<allow pkg="javax.swing"/>
<disallow pkg="org.apache.tools.ant"/>
<disallow pkg="com.puppycrawl.tools.checkstyle.grammars"/>
</subpackage>
</import-control>

87
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/Guard.java
View File

@ -28,29 +28,96 @@ class Guard
private final boolean mAllowed;
/** Package to control access to. */
private final String mPkgName;
/** Package to control access to. */
private final String mClassName;
/**
* Indicates if must be an exact match. Only valid if guard using a
* package.
*/
private final boolean mExactMatch;
/** Indicates if the guard only applies to this package. */
private final boolean mLocalOnly;
/**
* Constructs an instance.
* @param aAllowed whether the package is allowed.
* @param aPkgName the package name.
* @param aAllow whether to allow access.
* @param aLocalOnly whether guard is to be applied locally only
* @param aPkgName the package to apply guard on.
* @param aExactMatch whether the package must match exactly.
*/
Guard(final boolean aAllowed, final String aPkgName)
Guard(final boolean aAllow, final boolean aLocalOnly,
final String aPkgName, final boolean aExactMatch)
{
mAllowed = aAllowed;
mAllowed = aAllow;
mLocalOnly = aLocalOnly;
mPkgName = aPkgName;
mClassName = null;
mExactMatch = aExactMatch;
}
/**
* Constructs an instance.
* @param aAllow whether to allow access.
* @param aLocalOnly whether guard is to be applied locally only
* @param aClassName the class to apply guard on.
*/
Guard(final boolean aAllow, final boolean aLocalOnly,
final String aClassName)
{
mAllowed = aAllow;
mLocalOnly = aLocalOnly;
mPkgName = null;
mClassName = aClassName;
mExactMatch = true; // not used.
}
/**
* Verifies whether a package name be used.
* @param aName the package to check.
* @param aForImport the package to check.
* @param aInPkg the package doing the import.
* @return a result {@link AccessResult} indicating whether it can be used.
*/
AccessResult verify(final String aName)
AccessResult verifyImport(final String aForImport, final String aInPkg)
{
assert aName != null;
if (!aName.startsWith(mPkgName + ".")) {
return AccessResult.UNKNOWN;
assert aForImport != null;
if (mClassName != null) {
final boolean classMatch = mClassName.equals(aForImport);
return calculateResult(classMatch);
}
return mAllowed ? AccessResult.ALLOWED : AccessResult.DISALLOWED;
// Must be checking a package. First check that we actually match
// the package. Then check if matched and we must be an exact match.
// In this case, the text after the first "." must not contain
// another "." as this indicates that it is not an exact match.
assert mPkgName != null;
//boolean pkgMatch = aForImport.startsWith(mPkgName + ".");
boolean pkgMatch = aForImport.startsWith(mPkgName + ".");
if (pkgMatch && mExactMatch) {
pkgMatch = (aForImport.indexOf('.', (mPkgName.length() + 1)) == -1);
}
return calculateResult(pkgMatch);
}
/**
* @return returns whether the guard is to only be applied locally.
*/
boolean isLocalOnly()
{
return mLocalOnly;
}
/**
* Returns the appropriate {@link AccessResult} based on whether there
* was a match and if the guard is to allow access.
* @param aMatched indicates whether there was a match.
* @return An appropriate {@link AccessResult}.
*/
private AccessResult calculateResult(final boolean aMatched)
{
if (aMatched) {
return mAllowed ? AccessResult.ALLOWED : AccessResult.DISALLOWED;
}
return AccessResult.UNKNOWN;
}
}

9
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheck.java
View File

@ -52,6 +52,9 @@ public class ImportControlCheck extends Check
{
/** The root package controller. */
private PkgControl mRoot;
/** The package doing the import. */
private String mInPkg;
/**
* The package controller for the current file. Used for performance
* optimisation.
@ -75,7 +78,8 @@ public class ImportControlCheck extends Check
log(nameAST, "import.control.missing.file");
}
else {
mCurrentLeaf = mRoot.locateFinest(full.getText());
mInPkg = full.getText();
mCurrentLeaf = mRoot.locateFinest(mInPkg);
}
}
else if (mCurrentLeaf != null) {
@ -87,7 +91,8 @@ public class ImportControlCheck extends Check
imp = FullIdent.createFullIdent((DetailAST) aAST
.getFirstChild().getNextSibling());
}
final AccessResult access = mCurrentLeaf.checkAccess(imp.getText());
final AccessResult access = mCurrentLeaf.checkAccess(imp.getText(),
mInPkg);
if (!AccessResult.ALLOWED.equals(access)) {
log(aAST, "import.control.disallowed", imp.getText());
}

24
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java
View File

@ -59,7 +59,9 @@ final class ImportControlLoader extends AbstractLoader
/** {@inheritDoc} */
public void startElement(final String aNamespaceURI,
final String aLocalName, final String aQName, final Attributes aAtts)
final String aLocalName,
final String aQName,
final Attributes aAtts)
throws SAXException
{
if (aQName.equals("import-control")) {
@ -73,9 +75,25 @@ final class ImportControlLoader extends AbstractLoader
}
else if (aQName.equals("allow") || aQName.equals("disallow")) {
assert mStack.size() > 0;
final String pkg = safeGet(aAtts, "pkg");
// Need to handle either "pkg" or "class" attribute.
// May have "exact-match" for "pkg"
// May have "local-only"
final boolean isAllow = aQName.equals("allow");
final boolean isLocalOnly = (aAtts.getValue("local-only") != null);
final String pkg = aAtts.getValue("pkg");
final Guard g;
if (pkg != null) {
final boolean exactMatch =
(aAtts.getValue("exact-match") != null);
g = new Guard(isAllow, isLocalOnly, pkg, exactMatch);
}
else {
final String clazz = safeGet(aAtts, "class");
g = new Guard(isAllow, isLocalOnly, clazz);
}
final PkgControl pc = (PkgControl) mStack.peek();
pc.addGuard(new Guard(aQName.equals("allow"), pkg));
pc.addGuard(g);
}
}

21
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/PkgControl.java
View File

@ -117,12 +117,13 @@ class PkgControl
* its parent looking for a match. This will recurse looking for match.
* If there is no clear result then {@link AccessResult#UNKNOWN} is
* returned.
* @param aForPkg the package to check on.
* @param aForImport the package to check on.
* @param aInPkg the package doing the import.
* @return an {@link AccessResult}.
*/
AccessResult checkAccess(final String aForPkg)
AccessResult checkAccess(final String aForImport, final String aInPkg)
{
AccessResult retVal = localCheckAccess(aForPkg);
AccessResult retVal = localCheckAccess(aForImport, aInPkg);
if (retVal != AccessResult.UNKNOWN) {
return retVal;
}
@ -131,21 +132,27 @@ class PkgControl
return AccessResult.DISALLOWED;
}
return mParent.checkAccess(aForPkg);
return mParent.checkAccess(aForImport, aInPkg);
}
/**
* Checks whether any of the guards for this node control access to
* a specified package.
* @param aForPkg the package to check.
* @param aForImport the package to check.
* @param aInPkg the package doing the import.
* @return an {@link AccessResult}.
*/
private AccessResult localCheckAccess(final String aForPkg)
private AccessResult localCheckAccess(final String aForImport,
final String aInPkg)
{
final Iterator it = mGuards.iterator();
while (it.hasNext()) {
final Guard g = (Guard) it.next();
final AccessResult result = g.verify(aForPkg);
// Check if a Guard is only meant to be applied locally.
if (g.isLocalOnly() && !mFullPackage.equals(aInPkg)) {
continue;
}
final AccessResult result = g.verifyImport(aForImport, aInPkg);
if (result != AccessResult.UNKNOWN) {
return result;
}

14
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/import_control_1_0.dtd
View File

@ -1,4 +1,4 @@
<?xml version="1.0" encoding="UTF-8"?>
<?xml encoding="UTF-8"?>
<!ELEMENT import-control ((allow|disallow)*,subpackage*)>
@ -10,12 +10,16 @@
<!ATTLIST subpackage
name NMTOKEN #REQUIRED>
<!ELEMENT allow EMPTY>
<!ENTITY % attlist.guard "
pkg NMTOKEN #IMPLIED
exact-match (selected) #IMPLIED
class NMTOKEN #IMPLIED
local-only (selected) #IMPLIED">
<!ELEMENT allow EMPTY>
<!ATTLIST allow
pkg NMTOKEN #REQUIRED>
%attlist.guard;>
<!ELEMENT disallow EMPTY>
<!ATTLIST disallow
pkg NMTOKEN #REQUIRED>
%attlist.guard;>

37
src/tests/com/puppycrawl/tools/checkstyle/checks/imports/GuardTest.java
View File

@ -6,13 +6,38 @@ import junit.framework.TestCase;
public class GuardTest extends TestCase
{
public void testGuard()
public void testPkgGuard1()
{
final Guard g = new Guard(true, "pkg");
final Guard g = new Guard(true, false, "pkg", false);
assertNotNull(g);
assertEquals(AccessResult.UNKNOWN, g.verify("asda"));
assertEquals(AccessResult.UNKNOWN, g.verify("p"));
assertEquals(AccessResult.ALLOWED, g.verify("pkg.a"));
assertEquals(AccessResult.UNKNOWN, g.verify("pkg"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("asda", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("p", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkga", "ignored"));
assertEquals(AccessResult.ALLOWED, g.verifyImport("pkg.a", "ignored"));
assertEquals(AccessResult.ALLOWED, g.verifyImport("pkg.a.b", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkg", "ignored"));
}
public void testPkgGuard2()
{
final Guard g = new Guard(true, false, "pkg", true);
assertNotNull(g);
assertEquals(AccessResult.UNKNOWN, g.verifyImport("asda", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("p", "ignored"));
assertEquals(AccessResult.ALLOWED, g.verifyImport("pkg.a", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkg.a.b", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkg", "ignored"));
}
public void testClassGuard()
{
final Guard g = new Guard(true, false, "pkg.a");
assertNotNull(g);
assertEquals(AccessResult.UNKNOWN, g.verifyImport("asda", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("p", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkga", "ignored"));
assertEquals(AccessResult.ALLOWED, g.verifyImport("pkg.a", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkg.a.b", "ignored"));
assertEquals(AccessResult.UNKNOWN, g.verifyImport("pkg", "ignored"));
}
}

32
src/tests/com/puppycrawl/tools/checkstyle/checks/imports/PkgControlTest.java
View File

@ -13,11 +13,11 @@ public class PkgControlTest extends TestCase
protected void setUp() throws Exception
{
super.setUp();
pcRoot.addGuard(new Guard(false, "org.springframework"));
pcRoot.addGuard(new Guard(false, "org.hibernate"));
pcRoot.addGuard(new Guard(true, "org.apache.commons"));
pcRoot.addGuard(new Guard(false, false, "org.springframework", false));
pcRoot.addGuard(new Guard(false, false, "org.hibernate", false));
pcRoot.addGuard(new Guard(true, false, "org.apache.commons", false));
pcCommon.addGuard(new Guard(true, "org.hibernate"));
pcCommon.addGuard(new Guard(true, false, "org.hibernate", false));
}
public void testFullPkg()
@ -37,17 +37,19 @@ public class PkgControlTest extends TestCase
public void testCheckAccess()
{
assertEquals(AccessResult.DISALLOWED, pcCommon
.checkAccess("org.springframework.something"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
"org.springframework.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.ALLOWED, pcCommon
.checkAccess("org.apache.commons.something"));
assertEquals(AccessResult.DISALLOWED, pcCommon
.checkAccess("org.apache.commons"));
assertEquals(AccessResult.ALLOWED, pcCommon
.checkAccess("org.hibernate.something"));
assertEquals(AccessResult.DISALLOWED, pcCommon
.checkAccess("com.badpackage.something"));
assertEquals(AccessResult.DISALLOWED, pcRoot
.checkAccess("org.hibernate.something"));
.checkAccess("org.apache.commons.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
"org.apache.commons", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.ALLOWED, pcCommon.checkAccess(
"org.hibernate.something", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
"com.badpackage.something", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcRoot.checkAccess(
"org.hibernate.something", "com.kazgroup.courtlink"));
}
}