-
Notifications
You must be signed in to change notification settings - Fork 2.2k
CheckStyle-Rule-Extension: 6, 7, 8 #4339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
67fb33d
Service Client Instantiation Check rule
e9db7bb
add Extenal Dependentcy Exposed checks and checkstyle xml
8dd060b
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
d8633dc
revert back pom file
c428780
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
ab7e6a2
refactor 1
5c9652e
Use CheckUtil to get modifier values
74fc583
remove an extra useless boolean variable and remove unsued import
a03a4a4
refactor 2
49ebc74
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
3519b5e
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
6daf85c
updates based on J's feedback
cf185d5
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
d4b2e37
revise of wording
36f5a17
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
664a950
draft version of rule 5, 6, 7
6e9ca65
refactor 1
183222a
refactor ServiceInterface annotation check
9fa4a16
refactor Service Client Builder check class
360699c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
d2377c7
resolve conflict
a5aac06
Refactor ServiceClientBuilder and ServiceInterface checks
036787c
merge Service Client Method check to Service Client check
484dd98
suppression implementation and test but not test my test yet
228a3ad
add license title
a4720d0
ServiceInterface check tested. Looks good
df06858
tested ServiceClientBuilder check
8610ddf
revive wording
634c1f6
revive wording
f27f579
rm rule #5 since it is not ready for review
0180b9f
resolve conflict
ea745cf
without ServiceMethod but keep ServiceClient annotation
fe12876
remove debugging and comment for rule 5
8f0a8c9
rule 8 added
4af0d09
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
3ee8361
resolve conflict
2563084
refacor based on srnagar's feedback
128690b
add rule 9
b6e2347
only track 2 and correct path
aba32d2
updates ThrowClientLoggerCheck
f77acdb
move out ThrownClientLoggerCheck
d18e3a1
comment out ThrownClientLoggerCheck
bdb6cde
fixes for KV and no other method has prefix build other than buildCli…
e1334b1
resolve conflict
66e4594
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
408b77c
starting with build and ending with Client
e3842ad
revert kv changes and update limit character to 20
4b98e06
updates comments
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
122 changes: 122 additions & 0 deletions
122
...ty-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.tools.checkstyle.checks; | ||
|
|
||
| import com.puppycrawl.tools.checkstyle.api.AbstractCheck; | ||
| import com.puppycrawl.tools.checkstyle.api.DetailAST; | ||
| import com.puppycrawl.tools.checkstyle.api.TokenTypes; | ||
|
|
||
| import java.util.Stack; | ||
|
|
||
| /** | ||
| * The @ServiceClientBuilder class should have the following rules: | ||
| * 1) All service client builder should be named <ServiceName>ClientBuilder and annotated with @ServiceClientBuilder. | ||
| * 2) No other method have prefix 'build' other than 'build*Client' or 'build*AsyncClient'. | ||
| */ | ||
| public class ServiceClientBuilderCheck extends AbstractCheck { | ||
| private static final String SERVICE_CLIENT_BUILDER = "ServiceClientBuilder"; | ||
|
|
||
| private Stack<Boolean> hasServiceClientBuilderAnnotationStack = new Stack(); | ||
| private Stack<Boolean> hasBuildMethodStack = new Stack<>(); | ||
| private boolean hasServiceClientBuilderAnnotation; | ||
| private boolean hasBuildMethod; | ||
|
|
||
| @Override | ||
| public int[] getDefaultTokens() { | ||
| return getRequiredTokens(); | ||
| } | ||
|
|
||
| @Override | ||
| public int[] getAcceptableTokens() { | ||
| return getRequiredTokens(); | ||
| } | ||
|
|
||
| @Override | ||
| public int[] getRequiredTokens() { | ||
| return new int[] { | ||
| TokenTypes.CLASS_DEF, | ||
| TokenTypes.METHOD_DEF | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public void leaveToken(DetailAST token) { | ||
| if (token.getType() == TokenTypes.CLASS_DEF) { | ||
| hasServiceClientBuilderAnnotation = hasServiceClientBuilderAnnotationStack.pop(); | ||
| hasBuildMethod = hasBuildMethodStack.pop(); | ||
| if (hasServiceClientBuilderAnnotation && !hasBuildMethod) { | ||
| log(token, "Class with @ServiceClientBuilder annotation must have a method starting with ''build'' and ending with ''Client''."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void visitToken(DetailAST token) { | ||
| switch (token.getType()) { | ||
| case TokenTypes.CLASS_DEF: | ||
| // Save the state of variable 'hasServiceClientBuilderAnnotation' to limit the scope of accessibility | ||
| hasServiceClientBuilderAnnotationStack.push(hasServiceClientBuilderAnnotation); | ||
| hasBuildMethodStack.push(hasBuildMethod); | ||
| final DetailAST serviceClientAnnotationBuilderToken = getServiceClientBuilderAnnotation(token); | ||
| final String className = token.findFirstToken(TokenTypes.IDENT).getText(); | ||
|
|
||
| hasServiceClientBuilderAnnotation = serviceClientAnnotationBuilderToken != null; | ||
| if (hasServiceClientBuilderAnnotation) { | ||
| // Don't need to check if the 'serviceClients' exist. It is required when using @ServiceClientBuilder | ||
|
|
||
| // HAS @ServiceClientBuilder annotation but NOT named the class <ServiceName>ClientBuilder | ||
| if (!className.endsWith("ClientBuilder")) { | ||
| log(token, String.format("Class annotated with @ServiceClientBuilder ''%s'' should be named <ServiceName>ClientBuilder.", className)); | ||
| } | ||
| } else { | ||
| // No @ServiceClientBuilder annotation but HAS named the class <ServiceName>ClientBuilder | ||
| if (className.endsWith("ClientBuilder")) { | ||
| log(token, String.format("Class ''%s'' should be annotated with @ServiceClientBuilder.", className)); | ||
| } | ||
| } | ||
| break; | ||
| case TokenTypes.METHOD_DEF: | ||
| if (!hasServiceClientBuilderAnnotation) { | ||
| return; | ||
| } | ||
|
|
||
| final String methodName = token.findFirstToken(TokenTypes.IDENT).getText(); | ||
| if (!methodName.startsWith("build")) { | ||
| break; | ||
| } | ||
|
|
||
| hasBuildMethod = true; | ||
| // 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)); | ||
| } | ||
| break; | ||
| default: | ||
| // Checkstyle complains if there's no default block in switch | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the class is annotated with @ServiceClientBuilder. | ||
| * | ||
| * @param classDefToken the CLASS_DEF AST node | ||
| * @return the annotation node if the class is annotated with @ServiceClientBuilder, null otherwise. | ||
| */ | ||
| private DetailAST getServiceClientBuilderAnnotation(DetailAST classDefToken) { | ||
| final DetailAST modifiersToken = classDefToken.findFirstToken(TokenTypes.MODIFIERS); | ||
|
|
||
| if (!modifiersToken.branchContains(TokenTypes.ANNOTATION)) { | ||
| return null; | ||
| } | ||
|
|
||
| DetailAST annotationToken = modifiersToken.findFirstToken(TokenTypes.ANNOTATION); | ||
| if (!SERVICE_CLIENT_BUILDER.equals(annotationToken.findFirstToken(TokenTypes.IDENT).getText())) { | ||
| return null; | ||
| } | ||
|
|
||
| return annotationToken; | ||
| } | ||
| } | ||
114 changes: 114 additions & 0 deletions
114
...uality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceInterfaceCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.tools.checkstyle.checks; | ||
|
|
||
| import com.puppycrawl.tools.checkstyle.api.AbstractCheck; | ||
| import com.puppycrawl.tools.checkstyle.api.DetailAST; | ||
| import com.puppycrawl.tools.checkstyle.api.TokenTypes; | ||
|
|
||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * The @ServiceInterface class should have the following rules: | ||
| * 1) The annotation property 'name' should be non-empty | ||
| * 2) The length of value of property 'name' should be less than 20 characters and without space | ||
| */ | ||
| public class ServiceInterfaceCheck extends AbstractCheck { | ||
| @Override | ||
| public int[] getDefaultTokens() { | ||
| return getRequiredTokens(); | ||
| } | ||
|
|
||
| @Override | ||
| public int[] getAcceptableTokens() { | ||
| return getRequiredTokens(); | ||
| } | ||
|
|
||
| @Override | ||
| public int[] getRequiredTokens() { | ||
| return new int[] { | ||
| TokenTypes.INTERFACE_DEF | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public void visitToken(DetailAST token) { | ||
| if (token.getType() == TokenTypes.INTERFACE_DEF) { | ||
| checkServiceInterface(token); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The @ServiceInterface class should have the following rules: | ||
| * 1) The annotation property 'name' should be non-empty | ||
| * 2) The length of value of property 'name' should be less than 20 characters and without space | ||
| * | ||
| * @param interfaceDefToken INTERFACE_DEF AST node | ||
| */ | ||
| private void checkServiceInterface(DetailAST interfaceDefToken) { | ||
| DetailAST serviceInterfaceAnnotationNode = null; | ||
| String nameValue = null; | ||
|
|
||
| DetailAST modifiersToken = interfaceDefToken.findFirstToken(TokenTypes.MODIFIERS); | ||
| // Find the @ServiceInterface and the property 'name' and the corresponding value | ||
| for (DetailAST ast = modifiersToken.getFirstChild(); ast != null; ast = ast.getNextSibling()) { | ||
|
mssfang marked this conversation as resolved.
|
||
| // We care about only the ANNOTATION type | ||
| if (ast.getType() != TokenTypes.ANNOTATION) { | ||
| continue; | ||
| } | ||
| // Skip if not @ServiceInterface annotation | ||
| if (!"ServiceInterface".equals(ast.findFirstToken(TokenTypes.IDENT).getText())) { | ||
| continue; | ||
| } | ||
|
|
||
| // Get the @ServiceInterface annotation node | ||
| serviceInterfaceAnnotationNode = ast; | ||
|
|
||
| // Get the 'name' property value of @ServiceInterface | ||
| // @ServiceInterface requires 'name' property | ||
| DetailAST annotationMemberValuePairToken = ast.findFirstToken(TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR); | ||
| if ("name".equals(annotationMemberValuePairToken.findFirstToken(TokenTypes.IDENT).getText())) { | ||
| nameValue = getNamePropertyValue(annotationMemberValuePairToken.findFirstToken(TokenTypes.EXPR)); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Checks the rules: | ||
| // Skip the check if no @ServiceInterface annotation found | ||
| if (serviceInterfaceAnnotationNode == null) { | ||
| return; | ||
| } | ||
|
|
||
| // 'name' is required at @ServiceInterface | ||
| // 'name' should not be empty, no Space allowed and the length should less than or equal to 20 characters | ||
| Pattern serviceNamePattern = Pattern.compile("^[a-zA-Z0-9]{1,20}$"); | ||
| if (!serviceNamePattern.matcher(nameValue).find()) { | ||
| log(serviceInterfaceAnnotationNode, String.format( | ||
| "The ''name'' property of @ServiceInterface, ''%s'' should be non-empty, alphanumeric and not more than 10 characters", | ||
| nameValue)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get the name property value from the EXPR node | ||
| * | ||
| * @param exprToken EXPR | ||
| * @return null if EXPR node doesn't exist or no STRING_LITERAL. Otherwise, returns the value of the property. | ||
| */ | ||
| private String getNamePropertyValue(DetailAST exprToken) { | ||
| if (exprToken == null) { | ||
| return null; | ||
| } | ||
|
|
||
| final DetailAST nameValueToken = exprToken.findFirstToken(TokenTypes.STRING_LITERAL); | ||
| if (nameValueToken == null) { | ||
| return null; | ||
| } | ||
|
|
||
| String nameValue = nameValueToken.getText(); | ||
|
|
||
| // remove the beginning and ending double quote | ||
| return nameValue.replaceAll("^\"|\"$", ""); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.