Skip to content

Commit

Permalink
Issue checkstyle#3309: Added excludedPackages to class coupling checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@soon
soon committed Mar 5, 2017
1 parent 6ae9253 commit 6ae704e
Show file tree
Hide file tree
Showing 11 changed files with 332 additions and 29 deletions.
208 changes: 179 additions & 29 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/AbstractClassCouplingCheck.java
View file
Expand Up @@ -23,6 +23,9 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
Expand All @@ -40,6 +43,9 @@
* @author o_sukhodolsky
*/
public abstract class AbstractClassCouplingCheck extends AbstractCheck {
/** A package separator - "." */
private static final String DOT = ".";

/** Class names to ignore. */
private static final Set<String> DEFAULT_EXCLUDED_CLASSES = Collections.unmodifiableSet(
Arrays.stream(new String[] {
Expand All @@ -64,18 +70,18 @@ public abstract class AbstractClassCouplingCheck extends AbstractCheck {
"Map", "HashMap", "SortedMap", "TreeMap",
}).collect(Collectors.toSet()));

/** Stack of contexts. */
private final Deque<Context> contextStack = new ArrayDeque<>();
/** Package names to ignore. All packages should end with a period. */
private static final Set<String> DEFAULT_EXCLUDED_PACKAGES = Collections.unmodifiableSet(
Arrays.stream(new String[] {"java.lang."}).collect(Collectors.toSet()));

/** User-configured class names to ignore. */
private Set<String> excludedClasses = DEFAULT_EXCLUDED_CLASSES;
/** User-configured package names to ignore. */
private Set<String> excludedPackages = DEFAULT_EXCLUDED_PACKAGES;
/** Allowed complexity. */
private int max;
/** Package of the file we check. */
private String packageName;

/** Current context. */
private Context context = new Context("", 0, 0);
/** Current file context. */
private FileContext fileContext;

/**
* Creates new instance of the check.
Expand Down Expand Up @@ -119,9 +125,33 @@ public final void setExcludedClasses(String... excludedClasses) {
Collections.unmodifiableSet(Arrays.stream(excludedClasses).collect(Collectors.toSet()));
}

/**
* Sets user-excluded pakcages to ignore. All exlcuded packages should end with a period,
* so it also appends a dot to a package name if it does not end with a period.
* @param excludedPackages the list of packages to ignore.
*/
public final void setExcludedPackages(String... excludedPackages) {
this.excludedPackages = Collections.unmodifiableSet(Arrays.stream(excludedPackages)
.map(this::makeCorrectPackageName)
.filter(x -> !x.isEmpty()).collect(Collectors.toSet()));
}

/**
* Creates correct package name. A correct package name should end with a period.
* @param packageName Package name to be corrected.
* @return Correct package name or empty string if the given package name is invalid.
*/
private String makeCorrectPackageName(String packageName) {
String result = packageName;
if (!result.isEmpty() && !result.endsWith(DOT)) {
result += DOT;
}
return result;
}

@Override
public final void beginTree(DetailAST ast) {
packageName = "";
fileContext = new FileContext();
}

@Override
Expand All @@ -130,20 +160,23 @@ public void visitToken(DetailAST ast) {
case TokenTypes.PACKAGE_DEF:
visitPackageDef(ast);
break;
case TokenTypes.IMPORT:
fileContext.registerImport(ast);
break;
case TokenTypes.CLASS_DEF:
case TokenTypes.INTERFACE_DEF:
case TokenTypes.ANNOTATION_DEF:
case TokenTypes.ENUM_DEF:
visitClassDef(ast);
break;
case TokenTypes.TYPE:
context.visitType(ast);
fileContext.visitType(ast);
break;
case TokenTypes.LITERAL_NEW:
context.visitLiteralNew(ast);
fileContext.visitLiteralNew(ast);
break;
case TokenTypes.LITERAL_THROWS:
context.visitLiteralThrows(ast);
fileContext.visitLiteralThrows(ast);
break;
default:
throw new IllegalArgumentException("Unknown type: " + ast);
Expand All @@ -169,28 +202,122 @@ public void leaveToken(DetailAST ast) {
* @param pkg package definition.
*/
private void visitPackageDef(DetailAST pkg) {
final FullIdent ident = FullIdent.createFullIdent(pkg.getLastChild()
.getPreviousSibling());
packageName = ident.getText();
final FullIdent ident = FullIdent.createFullIdent(pkg.getLastChild().getPreviousSibling());
fileContext.setPackageName(ident.getText());
}

/**
* Creates new context for a given class.
* @param classDef class definition node.
*/
private void visitClassDef(DetailAST classDef) {
contextStack.push(context);
final String className =
classDef.findFirstToken(TokenTypes.IDENT).getText();
context = new Context(className,
classDef.getLineNo(),
classDef.getColumnNo());
final String className = classDef.findFirstToken(TokenTypes.IDENT).getText();
fileContext.createNewClassContext(className, classDef.getLineNo(), classDef.getColumnNo());
}

/** Restores previous context. */
private void leaveClassDef() {
context.checkCoupling();
context = contextStack.pop();
fileContext.checkCurrentClassAndRestorePrevious();
}

/**
* Encapsulates information about classes coupling inside single file.
*/
private class FileContext {
/** A map of (imported class name -> class name with package) pairs. */
private final Map<String, String> importedClassPackage = new HashMap<>();

/** Stack of class contexts. */
private final Deque<ClassContext> classesContexts = new ArrayDeque<>();

/** Current file package. */
private String packageName = "";

/** Current context. */
private ClassContext classContext = new ClassContext(this, "", 0, 0);

/**
* Retrieves current file package name.
* @return Package name.
*/
public String getPackageName() {
return packageName;
}

/**
* Sets current context package name.
* @param packageName Package name to be set.
*/
public void setPackageName(String packageName) {
this.packageName = packageName;
}

/**
* Registers given import. This allows us to track imported classes.
* @param imp import definition.
*/
public void registerImport(DetailAST imp) {
final FullIdent ident = FullIdent.createFullIdent(
imp.getLastChild().getPreviousSibling());
final String fullName = ident.getText();
if (fullName.charAt(fullName.length() - 1) != '*') {
final int lastDot = fullName.lastIndexOf('.');
if (lastDot != -1) {
importedClassPackage.put(fullName.substring(lastDot + 1), fullName);
}
}
}

/**
* Retrieves class name with packages. Uses previously registered imports to
* get the full class name.
* @param className Class name to be retrieved.
* @return Class name with package name, if found, {@link Optional#empty()} otherwise.
*/
public Optional<String> getClassNameWithPackage(String className) {
return Optional.ofNullable(importedClassPackage.get(className));
}

/**
* Creates new inner class context with given name and location.
* @param className The class name.
* @param lineNo The class line number.
* @param columnNo The class column number.
*/
private void createNewClassContext(String className, int lineNo, int columnNo) {
classesContexts.push(classContext);
classContext = new ClassContext(this, className, lineNo, columnNo);
}

/** Restores previous context. */
private void checkCurrentClassAndRestorePrevious() {
classContext.checkCoupling();
classContext = classesContexts.pop();
}

/**
* Visits type token for the current class context.
* @param ast TYPE token.
*/
public void visitType(DetailAST ast) {
classContext.visitType(ast);
}

/**
* Visits NEW token for the current class context.
* @param ast NEW token.
*/
public void visitLiteralNew(DetailAST ast) {
classContext.visitLiteralNew(ast);
}

/**
* Visits THROWS token for the current class context.
* @param ast THROWS token.
*/
public void visitLiteralThrows(DetailAST ast) {
classContext.visitLiteralThrows(ast);
}
}

/**
Expand All @@ -199,7 +326,9 @@ private void leaveClassDef() {
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
* @author o_sukhodolsky
*/
private class Context {
private class ClassContext {
/** Parent file context. */
private final FileContext parentContext;
/**
* Set of referenced classes.
* Sorted by name for predictable error messages in unit tests.
Expand All @@ -215,11 +344,13 @@ private class Context {

/**
* Create new context associated with given class.
* @param parentContext Parent file context.
* @param className name of the given class.
* @param lineNo line of class definition.
* @param columnNo column of class definition.
*/
Context(String className, int lineNo, int columnNo) {
ClassContext(FileContext parentContext, String className, int lineNo, int columnNo) {
this.parentContext = parentContext;
this.className = className;
this.lineNo = lineNo;
this.columnNo = columnNo;
Expand All @@ -245,15 +376,15 @@ public void visitLiteralThrows(DetailAST literalThrows) {
*/
public void visitType(DetailAST ast) {
final String fullTypeName = CheckUtils.createFullType(ast).getText();
context.addReferencedClassName(fullTypeName);
addReferencedClassName(fullTypeName);
}

/**
* Visits NEW.
* @param ast NEW to process.
*/
public void visitLiteralNew(DetailAST ast) {
context.addReferencedClassName(ast.getFirstChild());
addReferencedClassName(ast.getFirstChild());
}

/**
Expand All @@ -275,10 +406,12 @@ private void addReferencedClassName(String referencedClassName) {
}
}

/** Checks if coupling less than allowed or not. */
/**
* Checks if coupling less than allowed or not.
*/
public void checkCoupling() {
referencedClassNames.remove(className);
referencedClassNames.remove(packageName + "." + className);
referencedClassNames.remove(parentContext.getPackageName() + DOT + className);

if (referencedClassNames.size() > max) {
log(lineNo, columnNo, getLogMessageId(),
Expand All @@ -294,7 +427,24 @@ public void checkCoupling() {
*/
private boolean isSignificant(String candidateClassName) {
return !excludedClasses.contains(candidateClassName)
&& !candidateClassName.startsWith("java.lang.");
&& !isFromExcludedPackage(candidateClassName);
}

/**
* Checks if given class should be ignored as it belongs to excluded package.
* @param candidateClassName class to check
* @return true if we should not count this class.
*/
private boolean isFromExcludedPackage(String candidateClassName) {
String classNameWithPackage = candidateClassName;
if (!candidateClassName.contains(DOT)) {
classNameWithPackage = parentContext
.getClassNameWithPackage(candidateClassName)
.orElse(null);
}

return classNameWithPackage != null && excludedPackages.stream()
.anyMatch(classNameWithPackage::startsWith);
}
}
}
2 changes: 2 additions & 0 deletions ...ava/com/puppycrawl/tools/checkstyle/checks/metrics/ClassDataAbstractionCouplingCheck.java
View file
Expand Up @@ -50,6 +50,7 @@ public ClassDataAbstractionCouplingCheck() {
public int[] getRequiredTokens() {
return new int[] {
TokenTypes.PACKAGE_DEF,
TokenTypes.IMPORT,
TokenTypes.CLASS_DEF,
TokenTypes.INTERFACE_DEF,
TokenTypes.ENUM_DEF,
Expand All @@ -61,6 +62,7 @@ public int[] getRequiredTokens() {
public int[] getAcceptableTokens() {
return new int[] {
TokenTypes.PACKAGE_DEF,
TokenTypes.IMPORT,
TokenTypes.CLASS_DEF,
TokenTypes.INTERFACE_DEF,
TokenTypes.ENUM_DEF,
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/ClassFanOutComplexityCheck.java
View file
Expand Up @@ -49,6 +49,7 @@ public ClassFanOutComplexityCheck() {
public int[] getRequiredTokens() {
return new int[] {
TokenTypes.PACKAGE_DEF,
TokenTypes.IMPORT,
TokenTypes.CLASS_DEF,
TokenTypes.INTERFACE_DEF,
TokenTypes.ENUM_DEF,
Expand All @@ -63,6 +64,7 @@ public int[] getRequiredTokens() {
public int[] getAcceptableTokens() {
return new int[] {
TokenTypes.PACKAGE_DEF,
TokenTypes.IMPORT,
TokenTypes.CLASS_DEF,
TokenTypes.INTERFACE_DEF,
TokenTypes.ENUM_DEF,
Expand Down

0 comments on commit 6ae704e

Please sign in to comment.