Allow for a few custom Checkstyle rules to be configured (#43731)

Allow for a few custom Checkstyle rules to be configured
Azure · Jan 8, 2025 · 8f7b796 · 8f7b796
1 parent 53c5b61
commit 8f7b796
Show file tree

Hide file tree

Showing 9 changed files with 506 additions and 54 deletions.
diff --git a/...quality-reports/src/main/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheck.java b/...quality-reports/src/main/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheck.java
@@ -10,9 +10,7 @@
 import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
 import com.puppycrawl.tools.checkstyle.utils.ScopeUtil;
 
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.Arrays;
 
 /**
  * Ensure that code is not using words or abbreviations that are deny listed by this Checkstyle. denyListedWords: the
@@ -21,7 +19,7 @@
  * Prints out a message stating the location and the class, method or variable as well as the list of deny listed words.
  */
 public class DenyListedWordsCheck extends AbstractCheck {
-    private final Set<String> denyListedWords = new HashSet<>();
+    private String[] denyListedWords = new String[0];
 
     static final String ERROR_MESSAGE = "%s, All Public API Classes, Fields and Methods should follow "
         + "Camelcase standards for the following words: %s.";
@@ -33,7 +31,7 @@ public class DenyListedWordsCheck extends AbstractCheck {
      */
     public final void setDenyListedWords(String... denyListedWords) {
         if (denyListedWords != null) {
-            Collections.addAll(this.denyListedWords, denyListedWords);
+            this.denyListedWords = Arrays.copyOf(denyListedWords, denyListedWords.length);
         }
     }
 

diff --git a/...ode-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java b/...ode-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java
@@ -10,12 +10,9 @@
 import com.puppycrawl.tools.checkstyle.utils.TokenUtil;
 
 import java.util.ArrayDeque;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Objects;
 import java.util.Queue;
-import java.util.Set;
 
 /**
  * Good Logging Practice:
@@ -29,9 +26,6 @@
  * </ol>
  */
 public class GoodLoggingCheck extends AbstractCheck {
-    private static final String CLIENT_LOGGER_PATH = "com.azure.core.util.logging.ClientLogger";
-    private static final String CLIENT_LOGGER = "ClientLogger";
-    private static final String LOGGER = "logger";
     private static final int[] REQUIRED_TOKENS = new int[]{
         TokenTypes.IMPORT,
         TokenTypes.INTERFACE_DEF,
@@ -52,9 +46,57 @@ public class GoodLoggingCheck extends AbstractCheck {
     // A LIFO queue stores the class names, pop top element if exist the class name AST node
     private final Queue<String> classNameDeque = Collections.asLifoQueue(new ArrayDeque<>());
     // Collection of Invalid logging packages
-    private static final Set<String> INVALID_LOGS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
+    private static final String[] INVALID_LOGS = new String[] {
         "org.slf4j", "org.apache.logging.log4j", "java.util.logging"
-    )));
+    };
+
+    private String fullyQualifiedLoggerName = "com.azure.core.util.logging.ClientLogger";
+    private String simpleClassName = "ClientLogger";
+    private String loggerName = "logger";
+
+    /**
+     * Sets the fully qualified logger name.
+     * <p>
+     * If not set this will default to {@code com.azure.core.util.logging.ClientLogger}.
+     *
+     * @param fullyQualifiedLoggerName the fully qualified logger name.
+     * @throws IllegalArgumentException if the fully qualified logger name is null or empty.
+     */
+    public final void setFullyQualifiedLoggerName(String fullyQualifiedLoggerName) {
+        if (fullyQualifiedLoggerName == null || fullyQualifiedLoggerName.isEmpty()) {
+            throw new IllegalArgumentException("fullyQualifiedLoggerName cannot be null or empty.");
+        }
+
+        this.fullyQualifiedLoggerName = fullyQualifiedLoggerName;
+    }
+
+    /**
+     * Sets the simple class name for the logger.
+     * <p>
+     * If not set this will default to {@code ClientLogger}.
+     *
+     * @param simpleClassName the simple class name for the logger.
+     * @throws IllegalArgumentException if the simple class name is null or empty.
+     */
+    public final void setSimpleClassName(String simpleClassName) {
+        if (simpleClassName == null || simpleClassName.isEmpty()) {
+            throw new IllegalArgumentException("simpleClassName cannot be null or empty.");
+        }
+
+        this.simpleClassName = simpleClassName;
+    }
+
+    /**
+     * Sets the case-insensitive name of the logger instance.
+     * <p>
+     * If not set this will default to {@code logger}.
+     *
+     * @param loggerName the case-insensitive name of the logger instance.
+     * @throws IllegalArgumentException if the logger name is null or empty.
+     */
+    public final void setLoggerName(String loggerName) {
+        this.loggerName = loggerName;
+    }
 
     @Override
     public int[] getDefaultTokens() {
@@ -90,13 +132,13 @@ public void visitToken(DetailAST ast) {
         switch (ast.getType()) {
             case TokenTypes.IMPORT:
                 final String importClassPath = FullIdent.createFullIdentBelow(ast).getText();
-                hasClientLoggerImported = hasClientLoggerImported || importClassPath.equals(CLIENT_LOGGER_PATH);
+                hasClientLoggerImported = hasClientLoggerImported || importClassPath.equals(fullyQualifiedLoggerName);
 
-                INVALID_LOGS.forEach(item -> {
-                    if (importClassPath.startsWith(item)) {
-                        log(ast, String.format(NOT_CLIENT_LOGGER_ERROR, "external logger", CLIENT_LOGGER_PATH, item));
+                for (String invalidLog : INVALID_LOGS) {
+                    if (importClassPath.startsWith(invalidLog)) {
+                        log(ast, String.format(NOT_CLIENT_LOGGER_ERROR, "external logger", fullyQualifiedLoggerName, invalidLog));
                     }
-                });
+                }
                 break;
             case TokenTypes.CLASS_DEF:
             case TokenTypes.INTERFACE_DEF:
@@ -116,7 +158,7 @@ public void visitToken(DetailAST ast) {
                 }
                 final String methodCallName = FullIdent.createFullIdentBelow(dotToken).getText();
                 if (methodCallName.startsWith("System.out") || methodCallName.startsWith("System.err")) {
-                    log(ast, String.format(NOT_CLIENT_LOGGER_ERROR, "Java System", CLIENT_LOGGER_PATH, methodCallName));
+                    log(ast, String.format(NOT_CLIENT_LOGGER_ERROR, "Java System", fullyQualifiedLoggerName, methodCallName));
                 }
                 break;
             default:
@@ -137,7 +179,7 @@ private boolean isTypeClientLogger(DetailAST varDefAST) {
             return false;
         }
         return TokenUtil.findFirstTokenByPredicate(typeAST, node ->
-            node.getType() == TokenTypes.IDENT && node.getText().equals(CLIENT_LOGGER)
+            node.getType() == TokenTypes.IDENT && node.getText().equals(simpleClassName)
         ).isPresent();
     }
 
@@ -149,7 +191,7 @@ private boolean isTypeClientLogger(DetailAST varDefAST) {
     private void checkLoggerInstantiation(DetailAST literalNewToken) {
         final DetailAST identToken = literalNewToken.findFirstToken(TokenTypes.IDENT);
         // Not ClientLogger instance
-        if (identToken == null || !identToken.getText().equals(CLIENT_LOGGER)) {
+        if (identToken == null || !identToken.getText().equals(simpleClassName)) {
             return;
         }
         // LITERAL_NEW node always has ELIST node below
@@ -180,8 +222,8 @@ private void checkLoggerNameMatch(DetailAST varToken) {
         }
         // Check if the Logger instance named as 'logger/LOGGER'.
         final DetailAST identAST = varToken.findFirstToken(TokenTypes.IDENT);
-        if (identAST != null && !identAST.getText().equalsIgnoreCase(LOGGER)) {
-            log(varToken, String.format(LOGGER_NAME_ERROR, LOGGER, identAST.getText()));
+        if (identAST != null && !identAST.getText().equalsIgnoreCase(loggerName)) {
+            log(varToken, String.format(LOGGER_NAME_ERROR, loggerName, identAST.getText()));
         }
     }
 }
diff --git a/...de-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/NoImplInPublicAPI.java b/...de-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/NoImplInPublicAPI.java
@@ -37,7 +37,7 @@ public class NoImplInPublicAPI extends AbstractCheck {
         + "for public API. " + ALTERNATIVE_MOVE_TO_PUBLIC_API;
 
     // Pattern that matches either an import statement or a fully-qualified type reference for being implementation.
-    private static final Pattern IMPLEMENTATION_CLASS = Pattern.compile("com\\.azure.*?\\.implementation.*?\\.(\\w+)");
+    private static final Pattern IMPLEMENTATION_CLASS = Pattern.compile(".*?\\.implementation.*?\\.(\\w+)");
 
     private Set<String> implementationClassSet = new HashSet<>();
 

diff --git a/...y-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrowFromClientLoggerCheck.java b/...y-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrowFromClientLoggerCheck.java
@@ -26,9 +26,9 @@
 public class ThrowFromClientLoggerCheck extends AbstractCheck {
     private static final String LOGGER_LOG_EXCEPTION_AS_ERROR = "logger.logExceptionAsError";
     private static final String LOGGER_LOG_THROWABLE_AS_ERROR = "logger.logThrowableAsError";
-    private static final String LOGGING_BUILDER_LOG_THROWABLE_AS_ERROR = "logger.atError().log";    
+    private static final String LOGGING_BUILDER_LOG_THROWABLE_AS_ERROR = "logger.atError().log";
     private static final String LOGGER_LOG_EXCEPTION_AS_WARNING = "logger.logExceptionAsWarning";
-    private static final String LOGGER_LOG_THROWABLE_AS_WARNING = "logger.logThrowableAsWarning";    
+    private static final String LOGGER_LOG_THROWABLE_AS_WARNING = "logger.logThrowableAsWarning";
     private static final String LOGGING_BUILDER_LOG_THROWABLE_AS_WARNING = "logger.atWarning().log";
 
     static final String THROW_LOGGER_EXCEPTION_MESSAGE = String.format("Directly throwing an exception is disallowed. "
@@ -98,7 +98,7 @@ public void visitToken(DetailAST token) {
             case TokenTypes.LITERAL_THROW:
                 // Skip check if the throw exception from static class, constructor or static method
                 if (classStaticDeque.isEmpty() || classStaticDeque.peek() || isInConstructor
-                    || methodStaticDeque.isEmpty() || methodStaticDeque.peek() 
+                    || methodStaticDeque.isEmpty() || methodStaticDeque.peek()
                     || findLogMethodIdentifier(token)) {
                     return;
                 }
@@ -126,7 +126,7 @@ public void visitToken(DetailAST token) {
     }
 
     /*
-     * Checks if the expression includes call to log(), which verifies logging builder call 
+     * Checks if the expression includes call to log(), which verifies logging builder call
      * e.g. logger.atError().log(ex)
      */
     private static boolean findLogMethodIdentifier(DetailAST root) {