From ddb7bc5b34e194a4f890774809c766bfa79fc5e9 Mon Sep 17 00:00:00 2001 From: rnveach Date: Sun, 27 May 2018 00:22:46 -0400 Subject: [PATCH] Issue #669: removed branchContains for complicated cases (part 1) --- ...idConstantAsFirstOperandInConditionCheck.java | 2 +- .../coding/CustomDeclarationOrderCheck.java | 2 +- .../coding/NoNullForCollectionReturnCheck.java | 2 +- .../coding/SimpleAccessorNameNotationCheck.java | 2 +- .../design/ConstructorWithoutParamsCheck.java | 2 +- .../PublicReferenceToPrivateTypeCheck.java | 4 ++-- .../design/StaticMethodCandidateCheck.java | 2 +- ...nstantAsFirstOperandInConditionCheckTest.java | 11 +++++++++++ .../PublicReferenceToPrivateTypeCheckTest.java | 13 +++++++++++++ ...idConstantAsFirstOperandInConditionCheck.java | 10 ++++++++++ ...InputPublicReferenceToPrivateTypeCheck20.java | 16 ++++++++++++++++ 11 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/design/InputPublicReferenceToPrivateTypeCheck20.java diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheck.java index 6df86449ff..62e0d98dee 100755 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheck.java @@ -128,7 +128,7 @@ private boolean isRefactoringRequired(DetailAST logicNode) { final int constantType = firstOperand.getType(); return isTargetConstantType(constantType) - && !secondOperand.branchContains(constantType); + && secondOperand.getType() != firstOperand.getType(); } /** diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java index a14cc0eb4d..605ad50745 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java @@ -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 diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NoNullForCollectionReturnCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NoNullForCollectionReturnCheck.java index 472c65a6de..3c118ee7f3 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NoNullForCollectionReturnCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/NoNullForCollectionReturnCheck.java @@ -280,7 +280,7 @@ private static LinkedList getAllSubblocks(DetailAST blockDef) { subblocks.addAll(getChildren(blockBody, TokenTypes.LITERAL_TRY)); final List nestedSubblocks = new LinkedList<>(); for (DetailAST currentSubblock : subblocks) { - if (currentSubblock.branchContains(TokenTypes.SLIST)) { + if (currentSubblock.findFirstToken(TokenTypes.SLIST) != null) { nestedSubblocks.addAll(getAllSubblocks(currentSubblock)); } } diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.java index 1f2b868068..d0837a39ee 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.java @@ -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)) { diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/ConstructorWithoutParamsCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/ConstructorWithoutParamsCheck.java index 63d78d7f37..ba98962729 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/ConstructorWithoutParamsCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/ConstructorWithoutParamsCheck.java @@ -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(); diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheck.java index 4ff4bbb20f..7e29e6323b 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheck.java @@ -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); } diff --git a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/StaticMethodCandidateCheck.java b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/StaticMethodCandidateCheck.java index 14a335cc64..cfbcac2dc2 100644 --- a/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/StaticMethodCandidateCheck.java +++ b/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/StaticMethodCandidateCheck.java @@ -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; } /** diff --git a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheckTest.java b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheckTest.java index 17805277e1..30f7f89476 100755 --- a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheckTest.java +++ b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/coding/AvoidConstantAsFirstOperandInConditionCheckTest.java @@ -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); @@ -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); @@ -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"), diff --git a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheckTest.java b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheckTest.java index 4c9d7aa478..e864b57fc4 100644 --- a/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheckTest.java +++ b/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheckTest.java @@ -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(); diff --git a/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputAvoidConstantAsFirstOperandInConditionCheck.java b/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputAvoidConstantAsFirstOperandInConditionCheck.java index 73356e1a1d..7fc6518812 100755 --- a/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputAvoidConstantAsFirstOperandInConditionCheck.java +++ b/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/coding/InputAvoidConstantAsFirstOperandInConditionCheck.java @@ -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; } } diff --git a/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/design/InputPublicReferenceToPrivateTypeCheck20.java b/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/design/InputPublicReferenceToPrivateTypeCheck20.java new file mode 100644 index 0000000000..90e9da7208 --- /dev/null +++ b/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/design/InputPublicReferenceToPrivateTypeCheck20.java @@ -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 internTable = new ConcurrentWeakInternSet<>(); + + private static class ConcurrentWeakInternSet { + private static class WeakEntry extends WeakReference { + public WeakEntry(T referent) { + super(referent); + } + } + } +} \ No newline at end of file