Skip to content

Commit

Permalink
Issue checkstyle#10859: FinalClass now exempts private-only ctor clas…
Browse files Browse the repository at this point in the history 
…ses that are extended by another class in the same CU
  • Loading branch information
@pbludov
pbludov committed Jan 1, 2022
1 parent 3dad361 commit 3d0cc7c
Show file tree
Hide file tree
Showing 12 changed files with 530 additions and 110 deletions.
227 changes: 160 additions & 67 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/design/FinalClassCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,25 @@
package com.puppycrawl.tools.checkstyle.checks.design;

import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import com.puppycrawl.tools.checkstyle.FileStatefulCheck;
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.ScopeUtil;
import com.puppycrawl.tools.checkstyle.utils.TokenUtil;

/**
* <p>
* Checks that a class which has only private constructors
* is declared as final. Doesn't check for classes nested in interfaces
* or annotations, as they are always {@code final} there.
* Checks that a class that has only private constructors and has no descendant
* classes is declared as final.
* </p>
* <p>
* To configure the check:
Expand Down Expand Up @@ -66,21 +71,6 @@
* }
* }
*
* interface CheckInterface
* {
* class MyClass { // OK, nested class in interface is always final
* private MyClass() {}
* }
* }
*
* public @interface Test {
* public boolean enabled()
* default true;
* class MyClass { // OK, class nested in an annotation is always final
* private MyClass() { }
* }
* }
*
* class TestAnonymousInnerClasses { // OK, class has an anonymous inner class.
* public static final TestAnonymousInnerClasses ONE = new TestAnonymousInnerClasses() {
*
Expand Down Expand Up @@ -119,6 +109,9 @@ public class FinalClassCheck
*/
private static final String PACKAGE_SEPARATOR = ".";

/** Keeps ClassDesc objects for all inner classes. */
private Map<String, ClassDesc> innerClasses;

/** Keeps ClassDesc objects for stack of declared classes. */
private Deque<ClassDesc> classes;

Expand All @@ -138,7 +131,11 @@ public int[] getAcceptableTokens() {
@Override
public int[] getRequiredTokens() {
return new int[] {
TokenTypes.ANNOTATION_DEF,
TokenTypes.CLASS_DEF,
TokenTypes.ENUM_DEF,
TokenTypes.INTERFACE_DEF,
TokenTypes.RECORD_DEF,
TokenTypes.CTOR_DEF,
TokenTypes.PACKAGE_DEF,
TokenTypes.LITERAL_NEW,
Expand All @@ -148,6 +145,7 @@ public int[] getRequiredTokens() {
@Override
public void beginTree(DetailAST rootAST) {
classes = new ArrayDeque<>();
innerClasses = new HashMap<>();
packageName = "";
}

Expand All @@ -158,6 +156,12 @@ public void visitToken(DetailAST ast) {
packageName = extractQualifiedName(ast.getFirstChild().getNextSibling());
break;

case TokenTypes.ANNOTATION_DEF:
case TokenTypes.ENUM_DEF:
case TokenTypes.INTERFACE_DEF:
case TokenTypes.RECORD_DEF:
break;

case TokenTypes.CLASS_DEF:
visitClass(ast);
break;
Expand Down Expand Up @@ -188,14 +192,10 @@ public void visitToken(DetailAST ast) {
* @param ast the token to process
*/
private void visitClass(DetailAST ast) {
final DetailAST modifiers = ast.findFirstToken(TokenTypes.MODIFIERS);
registerNestedSubclassToOuterSuperClasses(ast);

final boolean isFinal = modifiers.findFirstToken(TokenTypes.FINAL) != null;
final boolean isAbstract = modifiers.findFirstToken(TokenTypes.ABSTRACT) != null;

final String qualifiedClassName = getQualifiedClassName(ast);
classes.push(new ClassDesc(qualifiedClassName, isFinal, isAbstract));
final ClassDesc currClass = new ClassDesc(qualifiedClassName, classes.size(), ast);
classes.push(currClass);
innerClasses.put(qualifiedClassName, currClass);
}

/**
Expand All @@ -219,19 +219,35 @@ private void visitCtor(DetailAST ast) {
@Override
public void leaveToken(DetailAST ast) {
if (ast.getType() == TokenTypes.CLASS_DEF) {
final ClassDesc desc = classes.pop();
if (desc.isWithPrivateCtor()
classes.pop();
}
if (TokenUtil.isRootNode(ast.getParent())) {
innerClasses.forEach((qualifiedClassName, classDesc) -> {
final DetailAST classAst = classDesc.getClassAst();
registerNestedSubclassToOuterSuperClasses(classAst, qualifiedClassName);
});
innerClasses.forEach((key, value) -> {
if (shouldBeDeclaredAsFinal(value)) {
final String className = getClassNameFromQualifiedName(key);
log(value.getClassAst(), MSG_KEY, className);
}
});
}
}

/**
* Checks whether a class should be declared as final or not.
*
* @param desc description of the class
* @return true if given class should be declared as final otherwise false
*/
private static boolean shouldBeDeclaredAsFinal(ClassDesc desc) {
return desc.isWithPrivateCtor()
&& !(desc.isDeclaredAsAbstract()
|| desc.isWithAnonymousInnerClass())
&& !desc.isDeclaredAsFinal()
&& !desc.isWithNonPrivateCtor()
&& !desc.isWithNestedSubclass()
&& !ScopeUtil.isInInterfaceOrAnnotationBlock(ast)) {
final String qualifiedName = desc.getQualifiedName();
final String className = getClassNameFromQualifiedName(qualifiedName);
log(ast, MSG_KEY, className);
}
}
&& !desc.isWithNestedSubclass();
}

/**
Expand All @@ -245,25 +261,96 @@ private static String extractQualifiedName(DetailAST ast) {
}

/**
* Register to outer super classes of given classAst that
* Register to outer super class of given classAst that
* given classAst is extending them.
*
* @param classAst class which outer super classes will be
* @param classAst class which outer super class will be
* informed about nesting subclass
* @param qualifiedClassName qualifies class name(with package) of the current class
*/
private void registerNestedSubclassToOuterSuperClasses(DetailAST classAst) {
private void registerNestedSubclassToOuterSuperClasses(DetailAST classAst,
String qualifiedClassName) {
final String currentAstSuperClassName = getSuperClassName(classAst);
if (currentAstSuperClassName != null) {
for (ClassDesc classDesc : classes) {
final String classDescQualifiedName = classDesc.getQualifiedName();
if (doesNameInExtendMatchSuperClassName(classDescQualifiedName,
currentAstSuperClassName)) {
final List<ClassDesc> sameNamedClassList =
classesWithSameName(currentAstSuperClassName);
if (sameNamedClassList.isEmpty()) {
final ClassDesc classDesc = innerClasses.get(currentAstSuperClassName);
if (classDesc != null) {
classDesc.registerNestedSubclass();
}
}
else {
registerNearestAsSuperClass(qualifiedClassName, sameNamedClassList);
}
}
}

/**
* Checks if there is a class with same name.
*
* @param className name of the class
* @return true if there is another class with same name.
* @noinspection CallToStringConcatCanBeReplacedByOperator
*/
private List<ClassDesc> classesWithSameName(String className) {
return innerClasses.entrySet().stream()
.filter(entry -> {
return entry.getKey().endsWith(
PACKAGE_SEPARATOR.concat(className));
})
.map(Map.Entry::getValue)
.collect(Collectors.toList());
}

/**
* For all classes with the same name as the superclass, finds the nearest one
* and registers a subclass for it.
*
* @param superClassName current class name
* @param classesWithSameName classes which same name as super class
*/
private static void registerNearestAsSuperClass(String superClassName,
List<ClassDesc> classesWithSameName) {
final ClassDesc nearest = Collections.min(classesWithSameName, (first, second) -> {
int diff = Integer.compare(
classNameMatchingCount(superClassName, second.getQualifiedName()),
classNameMatchingCount(superClassName, first.getQualifiedName()));
if (diff == 0) {
diff = Integer.compare(first.getDepth(), second.getDepth());
}
return diff;
});
nearest.registerNestedSubclass();
}

/**
* Calculates and returns the class name matching count.
*
* <p>
* Suppose our pattern class is {@code foo.a.b} and class to be matched is
* {@code foo.a.ball} then classNameMatchingCount would be calculated by comparing every
* character, and updating main counter when we hit "."
* to prevent matching "a.b" with "a.ball". In this case classNameMatchingCount would
* be equal to 6 and not 7 (b of ball is not counted).
* </p>
*
* @param patternClass class against which the given class has to be matched
* @param classToBeMatched class to be matched
* @return class name matching count
*/
private static int classNameMatchingCount(String patternClass, String classToBeMatched) {
final char packageSeparator = PACKAGE_SEPARATOR.charAt(0);
final int length = Math.min(classToBeMatched.length(), patternClass.length());
int result = 0;
for (int i = 0; i < length && patternClass.charAt(i) == classToBeMatched.charAt(i); ++i) {
if (patternClass.charAt(i) == packageSeparator) {
result = i;
}
}
return result;
}

/**
* Check if class name matches with anonymous inner class name.
*
Expand Down Expand Up @@ -336,23 +423,6 @@ private static String getSuperClassName(DetailAST classAst) {
return superClassName;
}

/**
* Checks if given super class name in extend clause match super class qualified name.
*
* @param superClassQualifiedName super class qualified name (with package)
* @param superClassInExtendClause name in extend clause
* @return true if given super class name in extend clause match super class qualified name,
* false otherwise
*/
private static boolean doesNameInExtendMatchSuperClassName(String superClassQualifiedName,
String superClassInExtendClause) {
String superClassNormalizedName = superClassQualifiedName;
if (!superClassInExtendClause.contains(PACKAGE_SEPARATOR)) {
superClassNormalizedName = getClassNameFromQualifiedName(superClassQualifiedName);
}
return superClassNormalizedName.equals(superClassInExtendClause);
}

/**
* Get class name from qualified name.
*
Expand All @@ -366,6 +436,9 @@ private static String getClassNameFromQualifiedName(String qualifiedName) {
/** Maintains information about class' ctors. */
private static final class ClassDesc {

/** Corresponding node. */
private final DetailAST classAst;

/** Qualified class name(with package). */
private final String qualifiedName;

Expand All @@ -375,6 +448,9 @@ private static final class ClassDesc {
/** Is class declared as abstract. */
private final boolean declaredAsAbstract;

/** Class nesting level. */
private final int depth;

/** Does class have non-private ctors. */
private boolean withNonPrivateCtor;

Expand All @@ -391,16 +467,16 @@ private static final class ClassDesc {
* Create a new ClassDesc instance.
*
* @param qualifiedName qualified class name(with package)
* @param declaredAsFinal indicates if the
* class declared as final
* @param declaredAsAbstract indicates if the
* class declared as abstract
* @param depth class nesting level
* @param classAst classAst node
*/
/* package */ ClassDesc(String qualifiedName, boolean declaredAsFinal,
boolean declaredAsAbstract) {
/* package */ ClassDesc(String qualifiedName, int depth, DetailAST classAst) {
this.qualifiedName = qualifiedName;
this.declaredAsFinal = declaredAsFinal;
this.declaredAsAbstract = declaredAsAbstract;
this.depth = depth;
this.classAst = classAst;
final DetailAST modifiers = classAst.findFirstToken(TokenTypes.MODIFIERS);
declaredAsFinal = modifiers.findFirstToken(TokenTypes.FINAL) != null;
declaredAsAbstract = modifiers.findFirstToken(TokenTypes.ABSTRACT) != null;
}

/**
Expand All @@ -412,6 +488,15 @@ private String getQualifiedName() {
return qualifiedName;
}

/**
* Get the classAst node.
*
* @return classAst node
*/
public DetailAST getClassAst() {
return classAst;
}

/** Adds private ctor. */
private void registerPrivateCtor() {
withPrivateCtor = true;
Expand All @@ -432,6 +517,15 @@ private void registerAnonymousInnerClass() {
withAnonymousInnerClass = true;
}

/**
* Returns class nesting level.
*
* @return class nesting level
*/
private int getDepth() {
return depth;
}

/**
* Does class have private ctors.
*
Expand Down Expand Up @@ -487,5 +581,4 @@ private boolean isWithAnonymousInnerClass() {
}

}

}
5 changes: 2 additions & 3 deletions src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/design/FinalClassCheck.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
name="FinalClass"
parent="com.puppycrawl.tools.checkstyle.TreeWalker">
<description>&lt;p&gt;
Checks that a class which has only private constructors
is declared as final. Doesn't check for classes nested in interfaces
or annotations, as they are always {@code final} there.
Checks that a class that has only private constructors and has no descendant
classes is declared as final.
&lt;/p&gt;</description>
<message-keys>
<message-key key="final.class"/>
Expand Down

0 comments on commit 3d0cc7c

Please sign in to comment.