From 4cac1b96189f5ff808a1330e837f7860b1f8f86b Mon Sep 17 00:00:00 2001 From: shafang Date: Thu, 11 Jul 2019 11:40:45 -0700 Subject: [PATCH 1/3] remove java code isImple check but move to suppression and add only check for public class for external Dependency check --- .../ExternalDependencyExposedCheck.java | 20 +++++++++---------- .../checkstyle/checkstyle-suppressions.xml | 6 +++++- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ExternalDependencyExposedCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ExternalDependencyExposedCheck.java index 6c4a730d5a99c..af388c4d6778f 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ExternalDependencyExposedCheck.java +++ b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ExternalDependencyExposedCheck.java @@ -29,12 +29,12 @@ public class ExternalDependencyExposedCheck extends AbstractCheck { ))); private final Map simpleClassNameToQualifiedNameMap = new HashMap<>(); - private static boolean isImplPackage; + + private boolean isPublicClass; @Override public void beginTree(DetailAST rootAST) { simpleClassNameToQualifiedNameMap.clear(); - isImplPackage = false; } @Override @@ -50,7 +50,6 @@ public int[] getAcceptableTokens() { @Override public int[] getRequiredTokens() { return new int[] { - TokenTypes.PACKAGE_DEF, TokenTypes.IMPORT, TokenTypes.METHOD_DEF }; @@ -58,22 +57,21 @@ public int[] getRequiredTokens() { @Override public void visitToken(DetailAST token) { - if (isImplPackage) { - return; - } - switch (token.getType()) { - case TokenTypes.PACKAGE_DEF: - String packageName = FullIdent.createFullIdent(token.findFirstToken(TokenTypes.DOT)).getText(); - isImplPackage = packageName.contains(".implementation"); - break; case TokenTypes.IMPORT: // Add all imported classes into a map, key is the name of class and value is the full package path of class. final String importClassPath = FullIdent.createFullIdentBelow(token).getText(); final String className = importClassPath.substring(importClassPath.lastIndexOf(".") + 1); simpleClassNameToQualifiedNameMap.put(className, importClassPath); break; + case TokenTypes.CLASS_DEF: + final DetailAST modifiersToken = token.findFirstToken(TokenTypes.MODIFIERS); + isPublicClass = modifiersToken.branchContains(TokenTypes.LITERAL_PUBLIC); + break; case TokenTypes.METHOD_DEF: + if (!isPublicClass) { + return; + } checkNoExternalDependencyExposed(token); break; default: diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml index 1f9752c52de54..ac7af8e887727 100755 --- a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -86,7 +86,11 @@ - + + + + + From 5e198cb66c1b0537f5b01352d76d2da8b633f29e Mon Sep 17 00:00:00 2001 From: shafang Date: Thu, 15 Aug 2019 01:00:49 -0700 Subject: [PATCH 2/3] fixes --- .../checks/OnlyFinalFieldsForImmutableClassCheck.java | 6 +++--- .../tools/checkstyle/checks/ServiceClientBuilderCheck.java | 2 +- .../main/java/com/azure/tools/checkstyle/checks/Utils.java | 5 ++++- .../main/resources/checkstyle/checkstyle-suppressions.xml | 3 +++ 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/OnlyFinalFieldsForImmutableClassCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/OnlyFinalFieldsForImmutableClassCheck.java index 85252a28a2149..916c7533859f3 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/OnlyFinalFieldsForImmutableClassCheck.java +++ b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/OnlyFinalFieldsForImmutableClassCheck.java @@ -1,5 +1,5 @@ -// Licensed under the MIT License. // Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. package com.azure.tools.checkstyle.checks; @@ -19,8 +19,8 @@ */ public class OnlyFinalFieldsForImmutableClassCheck extends AbstractCheck { private static final String IMMUTABLE_NOTATION = "Immutable"; - private static final String ERROR_MSG = "The variable field ''%s'' should be final." + - "Classes annotated with @Immutable are supposed to be immutable."; + private static final String ERROR_MSG = "The variable field ''%s'' should be final." + + "Classes annotated with @Immutable are supposed to be immutable."; private boolean hasImmutableAnnotation; diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java index 8623828a9c63e..417643ccc4f8f 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java +++ b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java @@ -90,7 +90,7 @@ public void visitToken(DetailAST token) { // method name has prefix 'build' but not 'build*Client' or 'build*AsyncClient' if (!methodName.endsWith("Client")) { log(token, String.format( - "@ServiceClientBuilder class should not have a method name, ''%s'' starting with ''build'' but not ending with ''Client''." , methodName)); + "@ServiceClientBuilder class should not have a method name, ''%s'' starting with ''build'' but not ending with ''Client''.", methodName)); } break; default: diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/Utils.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/Utils.java index 5cc32d5249c2f..11ff09b1b760a 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/Utils.java +++ b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/Utils.java @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + package com.azure.tools.checkstyle.checks; import com.puppycrawl.tools.checkstyle.api.DetailAST; @@ -39,7 +42,7 @@ protected static boolean hasIllegalCombination(DetailAST modifiers) { Optional illegalCombination = TokenUtil.findFirstTokenByPredicate(modifiers, (node) -> { final int type = node.getType(); return INVALID_FINAL_COMBINATION.contains(node.getType()) || (TokenTypes.ANNOTATION == type - && INVALID_FINAL_ANNOTATIONS.contains(node.findFirstToken(TokenTypes.IDENT).getText())); + && INVALID_FINAL_ANNOTATIONS.contains(node.findFirstToken(TokenTypes.IDENT).getText())); }); return illegalCombination.isPresent(); diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml index 553c34ca080fe..1ab6cec4d0b9a 100755 --- a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -112,6 +112,9 @@ + + + From 397c5a760c187766b6a907db4e42fa8a3fa4644a Mon Sep 17 00:00:00 2001 From: Shawn Fang <45607042+mssfang@users.noreply.github.com> Date: Thu, 15 Aug 2019 08:24:25 -0700 Subject: [PATCH 3/3] Update eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml suggest changes on comments Co-Authored-By: Connie Yau --- .../src/main/resources/checkstyle/checkstyle-suppressions.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml index 1ab6cec4d0b9a..916cd16a77969 100755 --- a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -112,7 +112,7 @@ - +