Skip to content

Commit

Permalink
Issue #669: removed branchContains for complicated cases (part 1)
Browse files Browse the repository at this point in the history
  • Loading branch information
rnveach committed May 28, 2018
1 parent 21ccb92 commit 3ca760e
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private boolean isRefactoringRequired(DetailAST logicNode) {
final int constantType = firstOperand.getType();

return isTargetConstantType(constantType)
&& !secondOperand.branchContains(constantType);
&& secondOperand.getType() != firstOperand.getType();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ private boolean isSetterCorrect(DetailAST methodDefAst, String methodPrefix) {

final DetailAST methodTypeAst = methodDefAst.findFirstToken(TokenTypes.TYPE);

if (methodTypeAst.branchContains(TokenTypes.LITERAL_VOID)) {
if (methodTypeAst.findFirstToken(TokenTypes.LITERAL_VOID) != null) {
final DetailAST statementsAst = methodDefAst.findFirstToken(TokenTypes.SLIST);
final String methodName = getIdentifier(methodDefAst);
final String setterFieldName = fieldPrefix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ private static LinkedList<DetailAST> getAllSubblocks(DetailAST blockDef) {
subblocks.addAll(getChildren(blockBody, TokenTypes.LITERAL_TRY));
final List<DetailAST> nestedSubblocks = new LinkedList<>();
for (DetailAST currentSubblock : subblocks) {
if (currentSubblock.branchContains(TokenTypes.SLIST)) {
if (currentSubblock.findFirstToken(TokenTypes.SLIST) != null) {
nestedSubblocks.addAll(getAllSubblocks(currentSubblock));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ else if (methodName.startsWith(GETTER_PREFIX)) {
private boolean isSetterCorrect(DetailAST methodDef, String methodName) {
final DetailAST methodType = methodDef.findFirstToken(TokenTypes.TYPE);
boolean result = true;
if (methodType.branchContains(TokenTypes.LITERAL_VOID)) {
if (methodType.findFirstToken(TokenTypes.LITERAL_VOID) != null) {
DetailAST currentVerifiedTop = methodDef.findFirstToken(TokenTypes.SLIST);

if (containsOnlyExpression(currentVerifiedTop)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public void visitToken(DetailAST ast) {
// and this check does not apply.
if (regexp.matcher(className).find()
&& !ignoredRegexp.matcher(className).find()
&& !ast.branchContains(TokenTypes.ARRAY_DECLARATOR)) {
&& ast.findFirstToken(TokenTypes.ARRAY_DECLARATOR) == null) {
final DetailAST parameterListAST = ast.findFirstToken(TokenTypes.ELIST);
final int numberOfParameters = parameterListAST.getChildCount();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,9 @@ private void addExternallyAccessibleFieldTypes(DetailAST fieldDefAst) {
*/
private boolean isExtendsOrImplementsSmth(DetailAST classOrInterfaceDefAst) {
return (classOrInterfaceDefAst
.branchContains(TokenTypes.EXTENDS_CLAUSE)
.findFirstToken(TokenTypes.EXTENDS_CLAUSE) != null
|| classOrInterfaceDefAst
.branchContains(TokenTypes.IMPLEMENTS_CLAUSE))
.findFirstToken(TokenTypes.IMPLEMENTS_CLAUSE) != null)
&& !isExtendsOrImplementsPrivate(classOrInterfaceDefAst);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ private Frame createMethodFrame(Frame parentFrame, DetailAST ast) {
private static boolean isAnonymousClass(DetailAST ast) {
final int astType = ast.getType();
return astType == TokenTypes.LITERAL_NEW
&& ast.branchContains(TokenTypes.LCURLY);
&& ast.findFirstToken(TokenTypes.OBJBLOCK) != null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ public void testAll() throws Exception {
"101:23: " + getCheckMessage(MSG_KEY, "=="),
"111:19: " + getCheckMessage(MSG_KEY, "=="),
"112:40: " + getCheckMessage(MSG_KEY, "=="),
"118:15: " + getCheckMessage(MSG_KEY, "=="),
"119:15: " + getCheckMessage(MSG_KEY, "!="),
"120:15: " + getCheckMessage(MSG_KEY, "!="),
"121:18: " + getCheckMessage(MSG_KEY, "!="),
};
verify(checkConfig, getPath("InputAvoidConstantAsFirstOperandInConditionCheck.java"),
expected);
Expand All @@ -81,6 +85,9 @@ public void testAttributes() throws Exception {
"25:15: " + getCheckMessage(MSG_KEY, "=="),
"28:18: " + getCheckMessage(MSG_KEY, "=="),
"31:19: " + getCheckMessage(MSG_KEY, "=="),
"118:15: " + getCheckMessage(MSG_KEY, "=="),
"119:15: " + getCheckMessage(MSG_KEY, "!="),
"120:15: " + getCheckMessage(MSG_KEY, "!="),
};
verify(checkConfig, getPath("InputAvoidConstantAsFirstOperandInConditionCheck.java"),
expected);
Expand Down Expand Up @@ -123,6 +130,10 @@ public void testNullProperties() throws Exception {
"101:23: " + getCheckMessage(MSG_KEY, "=="),
"111:19: " + getCheckMessage(MSG_KEY, "=="),
"112:40: " + getCheckMessage(MSG_KEY, "=="),
"118:15: " + getCheckMessage(MSG_KEY, "=="),
"119:15: " + getCheckMessage(MSG_KEY, "!="),
"120:15: " + getCheckMessage(MSG_KEY, "!="),
"121:18: " + getCheckMessage(MSG_KEY, "!="),
};

verify(checkConfig, getPath("InputAvoidConstantAsFirstOperandInConditionCheck.java"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,19 @@ public void usingPrivateTypeAsMethodParameterTest()
expected);
}

@Test
public void testNestedInnerClass()
throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(PublicReferenceToPrivateTypeCheck.class);
final String[] expected = {
"7:18: " + getCheckMessage(MSG_KEY, "ConcurrentWeakInternSet"),
};
verify(checkConfig,
getPath("InputPublicReferenceToPrivateTypeCheck20.java"),
expected);
}

@Test
public void testUnsupportedNode() {
final DetailAST sync = new DetailAST();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,14 @@ public void forCyclesCheck() {
for (someVariable = null; null == someVariable; someVariable = null) {} //!!
}

public void expressionsWithInnerConstant() {
int a = 0, b = 0;

if (1 == (1 & (a >> b))) {}
if (1 != someMethod(1)) {}
if (1 != a * 2) {}
if (null != System.getProperty("test.src", null)) {}
}

public int someMethod(int i) { return i; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.github.sevntu.checkstyle.checks.design;

import java.lang.invoke.MethodType;
import java.lang.ref.WeakReference;

public class InputPublicReferenceToPrivateTypeCheck20 {
static final ConcurrentWeakInternSet<MethodType> internTable = new ConcurrentWeakInternSet<>();

private static class ConcurrentWeakInternSet<T> {
private static class WeakEntry<T> extends WeakReference<T> {
public WeakEntry(T referent) {
super(referent);
}
}
}
}

0 comments on commit 3ca760e

Please sign in to comment.