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 committed Mar 18, 2017
1 parent 9b72b0f commit 5067a1c
Show file tree
Hide file tree
Showing 18 changed files with 684 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
Expand All @@ -32,6 +36,7 @@
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.CheckUtils;
import com.puppycrawl.tools.checkstyle.utils.CommonUtils;

/**
* Base class for coupling calculation.
Expand All @@ -40,6 +45,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 +72,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 +127,28 @@ 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.
* @param excludedPackages the list of packages to ignore.
*/
public final void setExcludedPackages(String... excludedPackages) {
final List<String> invalidIdentifiers = Arrays.stream(excludedPackages)
.filter(x -> !CommonUtils.isName(x))
.collect(Collectors.toList());
if (!invalidIdentifiers.isEmpty()) {
throw new IllegalArgumentException(
"the following values are not valid identifiers: "
+ invalidIdentifiers.stream().collect(Collectors.joining(", ", "[", "]")));
}

this.excludedPackages = Collections.unmodifiableSet(Arrays.stream(excludedPackages)
.map(x -> x + DOT).collect(Collectors.toSet()));
}

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

@Override
Expand All @@ -130,20 +157,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 +199,120 @@ 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(DOT);
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.
*/
public void createNewClassContext(String className, int lineNo, int columnNo) {
classesContexts.push(classContext);
classContext = new ClassContext(this, className, lineNo, columnNo);
}

/** Restores previous context. */
public 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 +321,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 +339,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 +371,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 @@ -278,7 +404,7 @@ private void addReferencedClassName(String referencedClassName) {
/** 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 +420,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);
}
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -467,4 +467,31 @@ public static String getFileExtension(String fileNameWithExtension) {
}
return extension;
}

/**
* Checks whether the given string is a valid identifier.
* @param str A string to check.
* @return true when the given string contains valid identifier.
*/
public static boolean isIdentifier(String str) {
boolean isIdentifier = !str.isEmpty() && Character.isJavaIdentifierStart(str.charAt(0));
for (int i = 1; isIdentifier && i < str.length(); ++i) {
isIdentifier = Character.isJavaIdentifierPart(str.charAt(i));
}
return isIdentifier;
}

/**
* Checks whether the given string is a valid name.
* @param str A string to check.
* @return true when the given string contains valid name.
*/
public static boolean isName(String str) {
boolean isName = !str.isEmpty();
final String[] identifiers = str.split("\\.", -1);
for (int i = 0; isName && i < identifiers.length; i++) {
isName = isIdentifier(identifiers[i]);
}
return isName;
}
}
Loading

0 comments on commit 5067a1c

Please sign in to comment.