Skip to content

Commit

Permalink
Issue checkstyle#6320: Kill surviving mutations for REMOVE_CONDITIONA…
Browse files Browse the repository at this point in the history
…LS in UnusedLocalVariableCheck
  • Loading branch information
Vyom-Yadav committed Jan 20, 2022
1 parent 275eb12 commit e168353
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import com.puppycrawl.tools.checkstyle.FileStatefulCheck;
Expand Down Expand Up @@ -218,10 +220,10 @@ public class UnusedLocalVariableCheck extends AbstractCheck {
private final Map<DetailAST, TypeDeclDesc> anonInnerAstToTypeDeclDesc;

/**
* Queue of tokens of type {@link UnusedLocalVariableCheck#CONTAINERS_FOR_ANON_INNERS}
* Set of tokens of type {@link UnusedLocalVariableCheck#CONTAINERS_FOR_ANON_INNERS}
* and {@link TokenTypes#LAMBDA} in some cases.
*/
private final Queue<DetailAST> anonInnerClassHolders;
private final Set<DetailAST> anonInnerClassHolders;

/**
* Name of the package.
Expand All @@ -233,13 +235,6 @@ public class UnusedLocalVariableCheck extends AbstractCheck {
*/
private int depth;

/**
* Block containing local anon inner class, keeps updating throughout the check.
* Acts as a tracker and checks whether the current node being visited is present
* inside the block containing a local anonymous inner class.
*/
private DetailAST blockContainingLocalAnonInnerClass;

/**
* Creates a new {@code UnusedLocalVariableCheck} instance.
*/
Expand All @@ -248,9 +243,8 @@ public UnusedLocalVariableCheck() {
typeDeclarations = new ArrayDeque<>();
typeDeclAstToTypeDeclDesc = new HashMap<>();
anonInnerAstToTypeDeclDesc = new HashMap<>();
anonInnerClassHolders = new ArrayDeque<>();
blockContainingLocalAnonInnerClass = null;
packageName = "";
anonInnerClassHolders = new HashSet<>();
packageName = null;
depth = 0;
}

Expand Down Expand Up @@ -300,7 +294,7 @@ public void beginTree(DetailAST root) {
typeDeclAstToTypeDeclDesc.clear();
anonInnerAstToTypeDeclDesc.clear();
anonInnerClassHolders.clear();
packageName = "";
packageName = null;
depth = 0;
}

Expand All @@ -316,16 +310,16 @@ else if (type == TokenTypes.VARIABLE_DEF) {
else if (type == TokenTypes.IDENT) {
visitIdentToken(ast, variables);
}
else if (type == TokenTypes.LITERAL_NEW
&& isInsideLocalAnonInnerClass(ast)) {
visitLocalAnonInnerClass(ast);
}
else if (TokenUtil.isTypeDeclaration(type)) {
visitTypeDeclarationToken(ast);
}
else if (type == TokenTypes.PACKAGE_DEF) {
packageName = extractQualifiedName(ast.getFirstChild().getNextSibling());
}
else if (type == TokenTypes.LITERAL_NEW
&& isInsideLocalAnonInnerClass(ast)) {
visitLocalAnonInnerClass(ast);
}
}

@Override
Expand All @@ -336,9 +330,6 @@ public void leaveToken(DetailAST ast) {
else if (ast.getType() == TokenTypes.COMPILATION_UNIT) {
leaveCompilationUnit();
}
else if (ast.equals(blockContainingLocalAnonInnerClass)) {
blockContainingLocalAnonInnerClass = null;
}
else if (isNonLocalTypeDeclaration(ast)) {
depth--;
typeDeclarations.pop();
Expand All @@ -352,8 +343,7 @@ else if (isNonLocalTypeDeclaration(ast)) {
* @param variablesStack stack of all the relevant variables in the scope
*/
private void visitDotToken(DetailAST dotAst, Deque<VariableDesc> variablesStack) {
if (blockContainingLocalAnonInnerClass == null
&& dotAst.getParent().getType() != TokenTypes.LITERAL_NEW
if (dotAst.getParent().getType() != TokenTypes.LITERAL_NEW
&& shouldCheckIdentTokenNestedUnderDot(dotAst)) {
checkIdentifierAst(dotAst.findFirstToken(TokenTypes.IDENT), variablesStack);
}
Expand All @@ -365,10 +355,8 @@ && shouldCheckIdentTokenNestedUnderDot(dotAst)) {
* @param varDefAst varDefAst
*/
private void visitVariableDefToken(DetailAST varDefAst) {
if (blockContainingLocalAnonInnerClass == null) {
addLocalVariables(varDefAst, variables);
addInstanceOrClassVar(varDefAst);
}
addLocalVariables(varDefAst, variables);
addInstanceOrClassVar(varDefAst);
}

/**
Expand All @@ -378,12 +366,10 @@ private void visitVariableDefToken(DetailAST varDefAst) {
* @param variablesStack stack of all the relevant variables in the scope
*/
private void visitIdentToken(DetailAST identAst, Deque<VariableDesc> variablesStack) {
if (blockContainingLocalAnonInnerClass == null) {
final DetailAST parentAst = identAst.getParent();
if (!TokenUtil.isOfType(parentAst, UNACCEPTABLE_PARENT_OF_IDENT)
&& shouldCheckIdentWithMethodRefParent(identAst)) {
checkIdentifierAst(identAst, variablesStack);
}
final DetailAST parentAst = identAst.getParent();
if (!TokenUtil.isOfType(parentAst, UNACCEPTABLE_PARENT_OF_IDENT)
&& shouldCheckIdentWithMethodRefParent(identAst)) {
checkIdentifierAst(identAst, variablesStack);
}
}

Expand All @@ -409,11 +395,7 @@ private void visitTypeDeclarationToken(DetailAST typeDeclAst) {
*/
private void visitLocalAnonInnerClass(DetailAST literalNewAst) {
anonInnerAstToTypeDeclDesc.put(literalNewAst, typeDeclarations.peek());
if (blockContainingLocalAnonInnerClass == null) {
blockContainingLocalAnonInnerClass = getBlockContainingLocalAnonInnerClass(
literalNewAst);
anonInnerClassHolders.add(blockContainingLocalAnonInnerClass);
}
anonInnerClassHolders.add(getBlockContainingLocalAnonInnerClass(literalNewAst));
}

/**
Expand Down Expand Up @@ -441,9 +423,7 @@ private static boolean isInsideLocalAnonInnerClass(DetailAST literalNewAst) {
if (lastChild != null && lastChild.getType() == TokenTypes.OBJBLOCK) {
DetailAST parentAst = literalNewAst.getParent();
while (parentAst.getType() != TokenTypes.SLIST) {
if (parentAst.getType() == TokenTypes.ENUM_CONSTANT_DEF
|| parentAst.getType() == TokenTypes.OBJBLOCK
&& TokenUtil.isTypeDeclaration(parentAst.getParent().getType())) {
if (TokenUtil.isTypeDeclaration(parentAst.getParent().getType())) {
break;
}
parentAst = parentAst.getParent();
Expand All @@ -462,8 +442,7 @@ private static boolean isInsideLocalAnonInnerClass(DetailAST literalNewAst) {
private void logViolations(DetailAST scopeAst, Deque<VariableDesc> variablesStack) {
while (!variablesStack.isEmpty() && variablesStack.peek().getScope() == scopeAst) {
final VariableDesc variableDesc = variablesStack.pop();
if (blockContainingLocalAnonInnerClass == null
&& !variableDesc.isUsed()
if (!variableDesc.isUsed()
&& !variableDesc.isInstVarOrClassVar()) {
final DetailAST typeAst = variableDesc.getTypeAst();
log(typeAst, MSG_UNUSED_LOCAL_VARIABLE, variableDesc.getName());
Expand All @@ -478,10 +457,9 @@ private void logViolations(DetailAST scopeAst, Deque<VariableDesc> variablesStac
* cast a shadow on local variables.
*/
private void leaveCompilationUnit() {
while (!anonInnerClassHolders.isEmpty()) {
final DetailAST holder = anonInnerClassHolders.remove();
anonInnerClassHolders.forEach(holder -> {
iterateOverBlockContainingLocalAnonInnerClass(holder, new ArrayDeque<>());
}
});
}

/**
Expand All @@ -491,8 +469,8 @@ private void leaveCompilationUnit() {
* @return true if type declaration is non-local
*/
private static boolean isNonLocalTypeDeclaration(DetailAST typeDeclAst) {
return typeDeclAst.getParent().getType() != TokenTypes.SLIST
&& TokenUtil.isTypeDeclaration(typeDeclAst.getType());
return TokenUtil.isTypeDeclaration(typeDeclAst.getType())
&& typeDeclAst.getParent().getType() != TokenTypes.SLIST;
}

/**
Expand Down Expand Up @@ -527,15 +505,14 @@ private static DetailAST getBlockContainingLocalAnonInnerClass(DetailAST literal
private static void addLocalVariables(DetailAST varDefAst, Deque<VariableDesc> variablesStack) {
final DetailAST parentAst = varDefAst.getParent();
final DetailAST grandParent = parentAst.getParent();
final boolean isInstanceVarInLocalAnonymousInnerClass =
grandParent.getType() == TokenTypes.LITERAL_NEW
&& isInsideLocalAnonInnerClass(grandParent);
if (isInstanceVarInLocalAnonymousInnerClass
final boolean isInstanceVarInAnonymousInnerClass =
grandParent.getType() == TokenTypes.LITERAL_NEW;
if (isInstanceVarInAnonymousInnerClass
|| parentAst.getType() != TokenTypes.OBJBLOCK) {
final DetailAST ident = varDefAst.findFirstToken(TokenTypes.IDENT);
final VariableDesc desc = new VariableDesc(ident.getText(),
varDefAst.findFirstToken(TokenTypes.TYPE), findScopeOfVariable(varDefAst));
if (isInstanceVarInLocalAnonymousInnerClass) {
if (isInstanceVarInAnonymousInnerClass) {
desc.registerAsInstOrClassVar();
}
variablesStack.push(desc);
Expand All @@ -550,8 +527,7 @@ private static void addLocalVariables(DetailAST varDefAst, Deque<VariableDesc> v
*/
private void addInstanceOrClassVar(DetailAST varDefAst) {
final DetailAST parentAst = varDefAst.getParent();
if (parentAst.getType() == TokenTypes.OBJBLOCK
&& isNonLocalTypeDeclaration(parentAst.getParent())
if (isNonLocalTypeDeclaration(parentAst.getParent())
&& !isPrivateInstanceVariable(varDefAst)) {
final DetailAST ident = varDefAst.findFirstToken(TokenTypes.IDENT);
final VariableDesc desc = new VariableDesc(ident.getText(),
Expand Down Expand Up @@ -581,12 +557,25 @@ private static boolean isPrivateInstanceVariable(DetailAST varDefAst) {
private TypeDeclDesc getSuperClassOfAnonInnerClass(DetailAST literalNewAst) {
TypeDeclDesc obtainedClass = null;
final String shortNameOfClass = getShortNameOfAnonInnerClass(literalNewAst);

final List<TypeDeclDesc> typeDeclWithSameName = typeDeclWithSameName(shortNameOfClass);
if (!typeDeclWithSameName.isEmpty()) {
obtainedClass = getTheNearestClass(
anonInnerAstToTypeDeclDesc.get(literalNewAst).getQualifiedName(),
typeDeclWithSameName);
if (packageName != null && shortNameOfClass.startsWith(packageName)) {
Optional<TypeDeclDesc> classWithCompletePackageName
= typeDeclAstToTypeDeclDesc.values()
.stream()
.filter(typeDeclDesc -> {
return typeDeclDesc.getQualifiedName().equals(shortNameOfClass);
})
.findFirst();
if (classWithCompletePackageName.isPresent()) {
obtainedClass = classWithCompletePackageName.get();
}
}
else {
final List<TypeDeclDesc> typeDeclWithSameName = typeDeclWithSameName(shortNameOfClass);
if (!typeDeclWithSameName.isEmpty()) {
obtainedClass = getTheNearestClass(
anonInnerAstToTypeDeclDesc.get(literalNewAst).getQualifiedName(),
typeDeclWithSameName);
}
}
return obtainedClass;
}
Expand Down Expand Up @@ -641,8 +630,16 @@ private void modifyVariablesStack(TypeDeclDesc obtainedClass,
private List<TypeDeclDesc> typeDeclWithSameName(String superClassName) {
return typeDeclAstToTypeDeclDesc.values().stream()
.filter(typeDeclDesc -> {
return typeDeclDesc.getQualifiedName()
.endsWith(PACKAGE_SEPARATOR.concat(superClassName));
final boolean result;
if (packageName == null && typeDeclDesc.getDepth() == 0) {
result = typeDeclDesc.getQualifiedName()
.equals(superClassName);
}
else {
result = typeDeclDesc.getQualifiedName()
.endsWith(PACKAGE_SEPARATOR.concat(superClassName));
}
return result;
})
.collect(Collectors.toList());
}
Expand Down Expand Up @@ -729,15 +726,15 @@ private static String getQualifiedTypeDeclarationName(String packageName,
final String qualifiedClassName;

if (outerClassQualifiedName == null) {
if (packageName.isEmpty()) {
if (packageName == null) {
qualifiedClassName = className;
}
else {
qualifiedClassName = packageName + PACKAGE_SEPARATOR + className;
qualifiedClassName = packageName.concat(PACKAGE_SEPARATOR + className);
}
}
else {
qualifiedClassName = outerClassQualifiedName + PACKAGE_SEPARATOR + className;
qualifiedClassName = outerClassQualifiedName.concat(PACKAGE_SEPARATOR + className);
}
return qualifiedClassName;
}
Expand Down Expand Up @@ -781,24 +778,21 @@ else if (type == TokenTypes.VARIABLE_DEF) {
else if (type == TokenTypes.IDENT) {
visitIdentToken(ast, variablesStack);
}
else if (type == TokenTypes.LITERAL_NEW
&& isInsideLocalAnonInnerClass(ast)) {
else if (isInsideLocalAnonInnerClass(ast)) {
final TypeDeclDesc obtainedClass = getSuperClassOfAnonInnerClass(ast);
modifyVariablesStack(obtainedClass, variablesStack, ast);
}
}

/**
* Visit all ast nodes under {@link UnusedLocalVariableCheck#anonInnerClassHolders} once
* Leave all ast nodes under {@link UnusedLocalVariableCheck#anonInnerClassHolders} once
* again.
*
* @param ast ast
* @param variablesStack stack of all the relevant variables in the scope
*/
private void customLeaveToken(DetailAST ast, Deque<VariableDesc> variablesStack) {
if (TokenUtil.isOfType(ast, SCOPES)) {
logViolations(ast, variablesStack);
}
logViolations(ast, variablesStack);
}

/**
Expand Down Expand Up @@ -845,9 +839,7 @@ private static void checkIdentifierAst(DetailAST identAst, Deque<VariableDesc> v
for (VariableDesc variableDesc : variablesStack) {
if (identAst.getText().equals(variableDesc.getName())
&& !isLeftHandSideValue(identAst)) {
if (!variableDesc.isInstVarOrClassVar()) {
variableDesc.registerAsUsed();
}
variableDesc.registerAsUsed();
break;
}
}
Expand All @@ -866,14 +858,7 @@ private static DetailAST findScopeOfVariable(DetailAST variableDef) {
result = parentAst;
}
else {
final DetailAST grandParent = parentAst.getParent();
final DetailAST scopeAst = grandParent.findFirstToken(TokenTypes.SLIST);
if (scopeAst == null) {
result = grandParent;
}
else {
result = scopeAst;
}
result = parentAst.getParent();
}
return result;
}
Expand Down

0 comments on commit e168353

Please sign in to comment.