From 3f0befefc20431aa6f658f85c12555e93cd34cfa Mon Sep 17 00:00:00 2001
From: Shawn Fang <45607042+mssfang@users.noreply.github.com>
Date: Thu, 15 Aug 2019 08:59:06 -0700
Subject: [PATCH] Fixes on Check on checkstyle (#5003)

* remove java code isImple check but move to suppression and add only check for public class for external Dependency check

* fixes

* Update eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml

suggest changes on comments

Co-Authored-By: Connie Yau <conniey@microsoft.com>
---
 .../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<DetailAST> 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..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,6 +112,9 @@
   <suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]samples[/\\].*\.java"/>
   <suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicyCheck" files=".*[/\\]samples[/\\].*\.java"/>
 
+  <!-- Don't apply custom Checkstyle rules to files under checkstyle package. -->
+  <suppress checks="com\.azure\.tools\.checkstyle\.checks\..+" files=".*[/\\]tools[/\\]checkstyle[/\\].*"/>
+
   <!-- JavaPackage suppression -->
   <suppress checks="JavadocPackage" files=".*[/\\]samples[/\\].*\.java"/>