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

Patch #1939775 for star imports (first draft)

This commit is contained in:
Oliver Burn 2010-10-09 16:26:15 +11:00
parent 2e50ca968c
commit d9bd21f3f7
4 changed files with 153 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

156
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/AvoidStarImportCheck.java
View File

@ -27,73 +27,171 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
* <p>
* Check that finds import statements that use the * notation.
* </p>
* <p> Rationale: Importing all classes from a package leads to tight coupling
* between packages and might lead to problems when a new version of a library
* introduces name clashes.
* <p>
* Rationale: Importing all classes from a package or static
* members from a class leads to tight coupling between packages
* or classes and might lead to problems when a new version of a
* library introduces name clashes.
* </p>
* <p>
* An example of how to configure the check is:
* </p>
* <pre>
* &lt;module name="AvoidStarImport"&gt;
* &lt;property name="excludes" value="java.io,java.net"/&gt;
* &lt;property name="excludes" value="java.io,java.net,java.lang.Math"/&gt;
* &lt;property name="allowClassImports" value="false"/&gt;
* &lt;property name="allowStaticMemberImports" value="false"/&gt;
* &lt;/module&gt;
* </pre>
*
* The optional "excludes" property allows for certain packages like
* java.io or java.net to be exempted from the rule. Note that the excludes
* property is not recursive, subpackages of excluded packages are not
* automatically excluded.
* java.io or java.net to be exempted from the rule. It also is used to
* allow certain classes like java.lang.Math or java.io.File to be
* excluded in order to support static member imports.
*
* Compatible with Java 1.5 source.
* The optional "allowClassImports" when set to true, will allow starred
* class imports but will not affect static member imports.
*
* The optional "allowStaticMemberImports" when set to true will allow
* starred static member imports but will not affect class imports.
*
* @author Oliver Burn
* @author <a href="bschneider@vecna.com">Bill Schneider</a>
* @version 1.0
* @author Travis Schneeberger
* @version 2.0
*/
public class AvoidStarImportCheck
extends Check
{
/** the packages to exempt from this check. */
/** the packages/classes to exempt from this check. */
private String[] mExcludes = new String[0];
/** whether to allow all class imports */
private boolean mAllowClassImports;
/** whether to allow all static member imports */
private boolean mAllowStaticMemberImports;
@Override
public int[] getDefaultTokens()
{
return new int[] {TokenTypes.IMPORT};
return new int[] {TokenTypes.IMPORT, TokenTypes.STATIC_IMPORT};
}
/**
* Sets the list of packages to exempt from the check.
* @param aExcludes a list of package names where star imports are ok
* Sets the list of packages or classes to be exempt from the check.
* The excludes can contain a .* or not.
* @param aExcludes a list of package names/fully-qualifies class names
* where star imports are ok
*/
public void setExcludes(String[] aExcludes)
{
mExcludes = new String[aExcludes.length];
mExcludes = appendDotStar(aExcludes);
}
/**
* Sets whether or not to allow all non-static class imports.
* @param aAllow true to allow false to disallow
*/
public void setAllowClassImports(boolean aAllow)
{
mAllowClassImports = aAllow;
}
/**
* Sets whether or not to allow all static member imports.
* @param aAllow true to allow false to disallow
*/
public void setAllowStaticMemberImports(boolean aAllow)
{
mAllowStaticMemberImports = aAllow;
}
@Override
public void visitToken(final DetailAST aAST)
{
if (!mAllowClassImports
&& aAST.getType() == TokenTypes.IMPORT)
{
final DetailAST startingDot =
aAST.getFirstChild();
logsStarredImportViolation(startingDot, mExcludes);
}
else if (!mAllowStaticMemberImports
&& aAST.getType() == TokenTypes.STATIC_IMPORT)
{
//must navigate past the static keyword
final DetailAST startingDot =
aAST.getFirstChild().getNextSibling();
logsStarredImportViolation(startingDot, mExcludes);
}
}
/**
* Appends a .* to the end of the string in a given
* array of excludes.
* @param aExcludes array of excludes (either package or class)
* @return array of excludes with .*
*/
private String[] appendDotStar(String[] aExcludes)
{
final String[] excludes = new String[aExcludes.length];
for (int i = 0; i < aExcludes.length; i++) {
mExcludes[i] = aExcludes[i];
if (!mExcludes[i].endsWith(".*")) {
excludes[i] = aExcludes[i];
if (!excludes[i].endsWith(".*")) {
// force all package names to end with ".*" to disambiguate
// "java.io"
mExcludes[i] = mExcludes[i] + ".*";
excludes[i] += ".*";
}
}
return excludes;
}
/**
* Gets the full import identifier. If the import is a starred import and
* it's not excluded then a violation is logged.
* @param aStartingDot the starting dot for the import statement
* @param aExcludes an array of excludes
*/
private void logsStarredImportViolation(DetailAST aStartingDot,
String[] aExcludes)
{
final FullIdent name = FullIdent.createFullIdent(aStartingDot);
if (isStaredImport(name)) {
if (!isExempt(name.getText(), aExcludes)) {
log(aStartingDot.getLineNo(),
"import.avoidStar", name.getText());
}
}
}
@Override
public void visitToken(DetailAST aAST)
/**
* Checks is an import is a stared import.
* @param aImportIdent the full import identifier
* @return true if a start import false if not
*/
private boolean isStaredImport(FullIdent aImportIdent)
{
final FullIdent name = FullIdent.createFullIdentBelow(aAST);
if ((name != null) && name.getText().endsWith(".*")) {
boolean exempt = false;
for (int i = 0; (i < mExcludes.length) && !exempt; i++) {
if (name.getText().equals(mExcludes[i])) {
exempt = true;
}
}
if (!exempt) {
log(aAST.getLineNo(), "import.avoidStar", name.getText());
if ((aImportIdent != null) && aImportIdent.getText().endsWith(".*")) {
return true;
}
return false;
}
/**
* Checks if a class of package is exempt from a give array of excludes.
* @param aClassOrPackage the class or package
* @param aExcludes an array of excludes
* @return true if except false if not
*/
private boolean isExempt(String aClassOrPackage, String[] aExcludes)
{
for (final String exclude : aExcludes) {
if (aClassOrPackage.equals(exclude)) {
return true;
}
}
return false;
}
}

5
src/tests/com/puppycrawl/tools/checkstyle/api/ScopeTest.java
View File

@ -18,8 +18,9 @@
////////////////////////////////////////////////////////////////////////////////
package com.puppycrawl.tools.checkstyle.api;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
public class ScopeTest

4
src/tests/com/puppycrawl/tools/checkstyle/api/SeverityLevelTest.java
View File

@ -18,8 +18,8 @@
////////////////////////////////////////////////////////////////////////////////
package com.puppycrawl.tools.checkstyle.api;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import org.junit.Test;
public class SeverityLevelTest

23
src/tests/com/puppycrawl/tools/checkstyle/checks/imports/AvoidStarImportTest.java
View File

@ -36,6 +36,9 @@ public class AvoidStarImportTest
"7: Using the '.*' form of import should be avoided - com.puppycrawl.tools.checkstyle.imports.*.",
"9: Using the '.*' form of import should be avoided - java.io.*.",
"10: Using the '.*' form of import should be avoided - java.lang.*.",
"25: Using the '.*' form of import should be avoided - javax.swing.WindowConstants.*.",
"26: Using the '.*' form of import should be avoided - javax.swing.WindowConstants.*.",
"28: Using the '.*' form of import should be avoided - sun.net.ftpclient.FtpClient.*.",
};
verify(checkConfig, getPath("imports" + File.separator + "InputImport.java"), expected);
@ -47,11 +50,27 @@ public class AvoidStarImportTest
{
final DefaultConfiguration checkConfig =
createCheckConfig(AvoidStarImportCheck.class);
checkConfig.addAttribute("excludes", "java.io,java.lang");
// allow the java.io/java.lang star imports
checkConfig.addAttribute("excludes",
"java.io,java.lang,javax.swing.WindowConstants.*, javax.swing.WindowConstants");
// allow the java.io/java.lang,javax.swing.WindowConstants star imports
final String[] expected2 = new String[] {
"7: Using the '.*' form of import should be avoided - com.puppycrawl.tools.checkstyle.imports.*.",
"28: Using the '.*' form of import should be avoided - sun.net.ftpclient.FtpClient.*.",
};
verify(checkConfig, getPath("imports" + File.separator + "InputImport.java"), expected2);
}
@Test
public void testAllowClassImports() throws Exception
{
final DefaultConfiguration checkConfig = createCheckConfig(AvoidStarImportCheck.class);
checkConfig.addAttribute("allowClassImports", "true");
// allow all class star imports
final String[] expected2 = new String[] {
"25: Using the '.*' form of import should be avoided - javax.swing.WindowConstants.*.",
"26: Using the '.*' form of import should be avoided - javax.swing.WindowConstants.*.",
"28: Using the '.*' form of import should be avoided - sun.net.ftpclient.FtpClient.*.", };
verify(checkConfig, getPath("imports" + File.separator
+ "InputImport.java"), expected2);
}
}