Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.";
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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();
}

Expand All @@ -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
Expand Down Expand Up @@ -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()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. "
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading