Skip to content

Commit 9b95c14

Browse files
samvaitysamvaity
authored
Checkstyle version increment and checks update (Azure#18401)
1 parent d49a454 commit 9b95c14

File tree

49 files changed

+383
-489
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+383
-489
lines changed

‎eng/code-quality-reports/pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@
3434
<dependency>
3535
<groupId>com.puppycrawl.tools</groupId>
3636
<artifactId>checkstyle</artifactId>
37-
<version>8.29</version> <!-- {x-version-update;com.puppycrawl.tools:checkstyle;external_dependency} -->
37+
<version>8.37</version> <!-- {x-version-update;com.puppycrawl.tools:checkstyle;external_dependency} -->
3838
</dependency>
3939

4040
<dependency>
4141
<groupId>com.puppycrawl.tools</groupId>
4242
<artifactId>checkstyle</artifactId>
43-
<version>8.29</version> <!-- {x-version-update;com.puppycrawl.tools:checkstyle;external_dependency} -->
43+
<version>8.37</version> <!-- {x-version-update;com.puppycrawl.tools:checkstyle;external_dependency} -->
4444
<type>test-jar</type>
4545
<scope>test</scope>
4646
</dependency>

‎eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/BlacklistedWordsCheck.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
77
import com.puppycrawl.tools.checkstyle.api.DetailAST;
88
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
9-
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier;
9+
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption;
1010
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
1111

1212
import java.util.Arrays;
@@ -82,9 +82,9 @@ public void visitToken(DetailAST token) {
8282
private boolean isPublicApi(DetailAST token) {
8383
final DetailAST modifiersAST =
8484
token.findFirstToken(TokenTypes.MODIFIERS);
85-
final AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersAST);
85+
final AccessModifierOption accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersAST);
8686
final boolean isStatic = modifiersAST.findFirstToken(TokenTypes.LITERAL_STATIC) != null;
87-
return (accessModifier.equals(AccessModifier.PUBLIC) || accessModifier.equals(AccessModifier.PROTECTED)) && !isStatic;
87+
return (accessModifier.equals(AccessModifierOption.PUBLIC) || accessModifier.equals(AccessModifierOption.PROTECTED)) && !isStatic;
8888
}
8989

9090
/**

‎eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ExternalDependencyExposedCheck.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import com.puppycrawl.tools.checkstyle.api.DetailAST;
88
import com.puppycrawl.tools.checkstyle.api.FullIdent;
99
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
10-
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier;
10+
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption;
1111
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
1212

1313
import java.util.Arrays;
@@ -67,10 +67,10 @@ public void visitToken(DetailAST token) {
6767
break;
6868
case TokenTypes.CLASS_DEF:
6969
// CLASS_DEF always has MODIFIERS
70-
final AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(
70+
final AccessModifierOption accessModifier = CheckUtil.getAccessModifierFromModifiersToken(
7171
token.findFirstToken(TokenTypes.MODIFIERS));
7272
isPublicClass =
73-
accessModifier.equals(AccessModifier.PUBLIC) || accessModifier.equals(AccessModifier.PROTECTED);
73+
accessModifier.equals(AccessModifierOption.PUBLIC) || accessModifier.equals(AccessModifierOption.PROTECTED);
7474
break;
7575
case TokenTypes.METHOD_DEF:
7676
if (!isPublicClass) {
@@ -94,8 +94,8 @@ private void checkNoExternalDependencyExposed(DetailAST methodDefToken) {
9494

9595
// Getting the modifier of the method to determine if it is 'public' or 'protected'.
9696
// Ignore the check if it is neither of 'public' nor 'protected',
97-
final AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersToken);
98-
if (!accessModifier.equals(AccessModifier.PUBLIC) && !accessModifier.equals(AccessModifier.PROTECTED)) {
97+
final AccessModifierOption accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersToken);
98+
if (!accessModifier.equals(AccessModifierOption.PUBLIC) && !accessModifier.equals(AccessModifierOption.PROTECTED)) {
9999
return;
100100
}
101101

‎eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/GoodLoggingCheck.java‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
/**
2020
* Good Logging Practice:
2121
* <ol>
22-
* <li>A non-static instance logger.</li>
22+
* <li>A non-static instance logger in a non-static method.</li>
2323
* <li>ClientLogger in public API should all named 'logger', public API classes are those classes that are declared
2424
* as public and that do not exist in an implementation package or subpackage.</li>
2525
* <li>Should not use any external logger class, only use ClientLogger. No slf4j, log4j, or other logging imports are
@@ -31,10 +31,11 @@ public class GoodLoggingCheck extends AbstractCheck {
3131
private static final String CLIENT_LOGGER_PATH = "com.azure.core.util.logging.ClientLogger";
3232
private static final String CLIENT_LOGGER = "ClientLogger";
3333
private static final String LOGGER = "logger";
34+
private static final String STATIC_LOGGER_ERROR = "Use a static ClientLogger instance in a static method.";
3435

3536
private static final String LOGGER_NAME_ERROR =
3637
"ClientLogger instance naming: use ''%s'' instead of ''%s'' for consistency.";
37-
private static final String STATIC_LOGGER_ERROR = "ClientLogger should not be static. Remove static modifier.";
38+
3839
private static final String NOT_CLIENT_LOGGER_ERROR =
3940
"Do not use %s class. Use ''%s'' as a logging mechanism instead of ''%s''.";
4041

@@ -64,7 +65,8 @@ public int[] getRequiredTokens() {
6465
TokenTypes.CLASS_DEF,
6566
TokenTypes.LITERAL_NEW,
6667
TokenTypes.VARIABLE_DEF,
67-
TokenTypes.METHOD_CALL
68+
TokenTypes.METHOD_CALL,
69+
TokenTypes.METHOD_DEF
6870
};
6971
}
7072

@@ -112,6 +114,9 @@ public void visitToken(DetailAST ast) {
112114
log(ast, String.format(NOT_CLIENT_LOGGER_ERROR, "Java System", CLIENT_LOGGER_PATH, methodCallName));
113115
}
114116
break;
117+
case TokenTypes.METHOD_DEF:
118+
checkForInvalidStaticLoggerUsage(ast);
119+
break;
115120
default:
116121
// Checkstyle complains if there's no default block in switch
117122
break;
@@ -172,16 +177,40 @@ private void checkLoggerNameMatch(DetailAST varToken) {
172177
if (!hasClientLoggerImported || !isTypeClientLogger(varToken)) {
173178
return;
174179
}
175-
// Check if the Logger instance named as 'logger'.
180+
// Check if the Logger instance named as 'logger/LOGGER'.
176181
final DetailAST identAST = varToken.findFirstToken(TokenTypes.IDENT);
177-
if (identAST != null && !identAST.getText().equals(LOGGER)) {
182+
if (identAST != null && !identAST.getText().equalsIgnoreCase(LOGGER)) {
178183
log(varToken, String.format(LOGGER_NAME_ERROR, LOGGER, identAST.getText()));
179184
}
180-
// Check if the Logger is static instance, log as error if it is static instance logger.
181-
if (TokenUtil.findFirstTokenByPredicate(varToken,
182-
node -> node.getType() == TokenTypes.MODIFIERS
183-
&& node.branchContains(TokenTypes.LITERAL_STATIC)).isPresent()) {
184-
log(varToken, STATIC_LOGGER_ERROR);
185+
}
186+
187+
/**
188+
* Report error if a static ClientLogger instance used in a non-static method.
189+
*
190+
* @param methodDefToken METHOD_DEF AST node
191+
*/
192+
private void checkForInvalidStaticLoggerUsage(DetailAST methodDefToken) {
193+
194+
// if not a static method
195+
if (!(TokenUtil.findFirstTokenByPredicate(methodDefToken,
196+
node -> node.branchContains(TokenTypes.LITERAL_STATIC)).isPresent())) {
197+
198+
// error if static `LOGGER` present, LOGGER.*
199+
if (methodDefToken.findFirstToken(TokenTypes.SLIST) != null) {
200+
TokenUtil
201+
.forEachChild(methodDefToken.findFirstToken(TokenTypes.SLIST), TokenTypes.EXPR, (exprToken) -> {
202+
if (exprToken != null) {
203+
DetailAST methodCallToken = exprToken.findFirstToken(TokenTypes.METHOD_CALL);
204+
if (methodCallToken != null && methodCallToken.findFirstToken(TokenTypes.DOT) != null) {
205+
if (methodCallToken.findFirstToken(TokenTypes.DOT)
206+
.findFirstToken(TokenTypes.IDENT).getText().equals(LOGGER.toUpperCase())) {
207+
log(methodDefToken, STATIC_LOGGER_ERROR);
208+
}
209+
}
210+
}
211+
});
212+
}
185213
}
186214
}
215+
187216
}

‎eng/code-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/HttpPipelinePolicyCheck.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import com.puppycrawl.tools.checkstyle.api.DetailAST;
88
import com.puppycrawl.tools.checkstyle.api.FullIdent;
99
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
10-
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifier;
10+
import com.puppycrawl.tools.checkstyle.checks.naming.AccessModifierOption;
1111
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
1212
import com.puppycrawl.tools.checkstyle.utils.TokenUtil;
1313

@@ -83,10 +83,10 @@ private void checkPublicNonImplementationPolicyClass(DetailAST classDefToken) {
8383
}
8484

8585
final DetailAST modifiersToken = classDefToken.findFirstToken(TokenTypes.MODIFIERS);
86-
final AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersToken);
86+
final AccessModifierOption accessModifier = CheckUtil.getAccessModifierFromModifiersToken(modifiersToken);
8787
final String className = classDefToken.findFirstToken(TokenTypes.IDENT).getText();
8888
// Public class check
89-
if (!accessModifier.equals(AccessModifier.PUBLIC)) {
89+
if (!accessModifier.equals(AccessModifierOption.PUBLIC)) {
9090
log(modifiersToken, String.format("Class ''%s'' implementing ''%s'' and should be a public class",
9191
className, HTTP_PIPELINE_POLICY));
9292
}

0 commit comments

Comments
 (0)