diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheck.java index 33be27d49de97..6051d95b63927 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheck.java +++ b/eng/code-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 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/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java index 18f48a3ff62b5..96b85b1c98ec9 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java +++ b/eng/code-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 @@ * */ 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 classNameDeque = Collections.asLifoQueue(new ArrayDeque<>()); // Collection of Invalid logging packages - private static final Set 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. + *

+ * 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. + *

+ * 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. + *

+ * 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/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/NoImplInPublicAPI.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/NoImplInPublicAPI.java index 0398c34931889..32e89394c8500 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/NoImplInPublicAPI.java +++ b/eng/code-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 implementationClassSet = new HashSet<>(); diff --git a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrowFromClientLoggerCheck.java b/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrowFromClientLoggerCheck.java index 1b98639f64183..1a590326f8e83 100644 --- a/eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrowFromClientLoggerCheck.java +++ b/eng/code-quality-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) { diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/clientcore-checkstyle.xml b/eng/code-quality-reports/src/main/resources/checkstyle/clientcore-checkstyle.xml new file mode 100644 index 0000000000000..fb503c81b3fe7 --- /dev/null +++ b/eng/code-quality-reports/src/main/resources/checkstyle/clientcore-checkstyle.xml @@ -0,0 +1,419 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/eng/code-quality-reports/src/test/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheckTest.java b/eng/code-quality-reports/src/test/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheckTest.java index d19d84cc3ebf3..a61c55ab4bf66 100644 --- a/eng/code-quality-reports/src/test/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheckTest.java +++ b/eng/code-quality-reports/src/test/java/com/azure/tools/checkstyle/checks/DenyListedWordsCheckTest.java @@ -33,8 +33,8 @@ protected String getPackageLocation() { @Test public void denyListedWordsTestData() throws Exception { String[] expected = { - expectedErrorMessage(3, 5, String.format(DenyListedWordsCheck.ERROR_MESSAGE, "errorHTTPMethod", "XML, HTTP, URL")), - expectedErrorMessage(9, 5, String.format(DenyListedWordsCheck.ERROR_MESSAGE, "invalidXMLMethod", "XML, HTTP, URL")) + expectedErrorMessage(3, 5, String.format(DenyListedWordsCheck.ERROR_MESSAGE, "errorHTTPMethod", "URL, HTTP, XML")), + expectedErrorMessage(9, 5, String.format(DenyListedWordsCheck.ERROR_MESSAGE, "invalidXMLMethod", "URL, HTTP, XML")) }; verify(checker, getPath("DenyListedWordsTestData.java"), expected); } diff --git a/sdk/clientcore/core/checkstyle-suppressions.xml b/sdk/clientcore/core/checkstyle-suppressions.xml index d5b026d94c0c2..e81feb3773645 100644 --- a/sdk/clientcore/core/checkstyle-suppressions.xml +++ b/sdk/clientcore/core/checkstyle-suppressions.xml @@ -5,8 +5,6 @@ - - @@ -15,27 +13,25 @@ - + - + + - - - diff --git a/sdk/clientcore/core/spotbugs-exclude.xml b/sdk/clientcore/core/spotbugs-exclude.xml index c0e54b82136bf..2235ca95cf865 100644 --- a/sdk/clientcore/core/spotbugs-exclude.xml +++ b/sdk/clientcore/core/spotbugs-exclude.xml @@ -16,7 +16,6 @@ - @@ -26,14 +25,15 @@ - + + @@ -41,8 +41,6 @@ - - @@ -71,9 +69,9 @@ + - @@ -217,7 +215,10 @@ - + + + + @@ -264,13 +265,17 @@ + + - - + + + + @@ -394,12 +399,4 @@ - - - - - - - - diff --git a/sdk/parents/clientcore-parent/pom.xml b/sdk/parents/clientcore-parent/pom.xml index d8ee1551d4100..799af97d104bc 100644 --- a/sdk/parents/clientcore-parent/pom.xml +++ b/sdk/parents/clientcore-parent/pom.xml @@ -759,7 +759,7 @@ ${checkstyle.skip} - ${project.basedir}/${relative.path.to.eng.folder}/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle.xml + ${project.basedir}/${relative.path.to.eng.folder}/eng/code-quality-reports/src/main/resources/checkstyle/clientcore-checkstyle.xml ${checkstyle.suppressionsLocation} ${project.basedir}/${relative.path.to.eng.folder}/eng/code-quality-reports/src/main/resources/checkstyle/java.header ${checkstyle.excludes}