Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
4cac1b9
remove java code isImple check but move to suppression and add only c…
Jul 11, 2019
9dbeae5
resolve conflict
Jul 12, 2019
b52c5ab
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Jul 25, 2019
cdf50a0
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Jul 26, 2019
a41ffdc
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Jul 29, 2019
e7ea7f2
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Aug 2, 2019
9f7c156
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Aug 2, 2019
c3c3571
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
Aug 6, 2019
06380ad
fix javadoc error
Aug 8, 2019
6d6064d
fix javadoc errors2
Aug 8, 2019
a5e48df
javadoc code snippet init
Aug 8, 2019
7187b42
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 8, 2019
340c302
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 12, 2019
5af60e2
updates based on new understand and PR feedback
Aug 13, 2019
840a047
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 13, 2019
0c7109b
refactor to isInlineCode
Aug 13, 2019
d5f2576
minor change
Aug 13, 2019
2f9cb94
resolve conflict
Aug 13, 2019
a9223ae
no checking on class-level javadoc
Aug 14, 2019
f473be0
a fix for getting correct package name
Aug 15, 2019
0746ff7
code snippet check rule
Aug 15, 2019
d5f8359
inline check rule
Aug 15, 2019
e97c88c
one java file split to two java files
Aug 15, 2019
ea9dc37
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 15, 2019
464d111
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 15, 2019
af7f4e8
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 15, 2019
5f2c597
fixes for checkstyle error and Jon's feedback
Aug 16, 2019
8cf387c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 16, 2019
8cc72ea
more lenient naming pattern
Aug 16, 2019
f8d2228
resolve conflict
Aug 16, 2019
451f4bb
add java doc explainatino
Aug 16, 2019
513fbe8
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 18, 2019
3eea6dc
fixes for Connie's feedback
Aug 19, 2019
068e7a0
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 19, 2019
95bfe16
resolve conflict
Aug 20, 2019
ac69f96
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 21, 2019
be10e5c
updates J feedback
Aug 21, 2019
0dcaa7b
fixes and suppress errors for both checkers
Aug 22, 2019
99a5903
propose a fix for naming matching
Aug 22, 2019
59e53ba
resolve conflict
Aug 22, 2019
eb3fa99
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 22, 2019
36be65c
fixes based on V and C feedback
Aug 22, 2019
f38084e
resolve storage conflict
Aug 22, 2019
8faecd8
without Netty, redo the changes
Aug 23, 2019
88c6405
fiex errors
Aug 23, 2019
89e9a38
resolve conflict
Aug 26, 2019
5bd8728
fixes and remove comments and updates suppression
Aug 26, 2019
7997ae8
back to have suppression and remove new lines
Aug 26, 2019
ed59769
resolve conflict
Aug 26, 2019
25e36f8
add one more comment
Aug 26, 2019
d66b863
inline code
Aug 26, 2019
a5bc7fd
correct right begin end
Aug 26, 2019
6b5f663
Update eng/code-quality-reports/src/main/java/com/azure/tools/checkst…
mssfang Aug 27, 2019
f97767a
Update eng/code-quality-reports/src/main/java/com/azure/tools/checkst…
mssfang Aug 27, 2019
3e0fbd9
Update eng/code-quality-reports/src/main/java/com/azure/tools/checkst…
mssfang Aug 27, 2019
631e111
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
Aug 27, 2019
9a45b7a
not using switch but use if for single condition
Aug 27, 2019
b99e398
Merge branch 'CS-Rule-JavaDoc' of https://github.com/mssfang/azure-sd…
Aug 27, 2019
9b33a3b
storage codesnippet ID should match with begin and end code sample id…
Aug 27, 2019
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
@@ -0,0 +1,194 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.tools.checkstyle.checks;

import com.puppycrawl.tools.checkstyle.api.DetailNode;
import com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes;
import com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck;
import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;

import java.util.regex.Pattern;

/**
* Requirement of Javadoc annotation @codesnippet check:
Comment thread
mssfang marked this conversation as resolved.
Outdated
* (1) Use '{@codesnippet ...}' instead of '<code>', '<pre>'.
Comment thread
mssfang marked this conversation as resolved.
Outdated
* (2) No multiple lines span at the '{@code ...}'.
Comment thread
mssfang marked this conversation as resolved.
Outdated
* (3) naming pattern of codesnippet":
* 1, For package name, class name, they should all be lower case.
* For method name, the first letter of method name should be lower case.
* For parameters, they should only be letters. Concatenate all parameters with dash character.
* 2, Jonathon will add more requirement specs later.
Comment thread
mssfang marked this conversation as resolved.
Outdated
*/
public class JavadocCodeSnippetCheck extends AbstractJavadocCheck {

private static final String CLASS_PATH_REGEX = "[a-z.]+";
private static final String METHOD_NAME_REGEX = "^[a-z][a-zA-Z]+";
private static final String PARAMETERS_REGEX = "[a-zA-Z-]+";
Comment thread
conniey marked this conversation as resolved.
Outdated

@Override
public int[] getDefaultJavadocTokens() {
return getRequiredJavadocTokens();
}

@Override
public int[] getRequiredJavadocTokens() {
return new int[] {
JavadocTokenTypes.HTML_ELEMENT_START,
JavadocTokenTypes.JAVADOC_INLINE_TAG
};
}

@Override
public void visitJavadocToken(DetailNode token) {
switch (token.getType()) {
Comment thread
mssfang marked this conversation as resolved.
Outdated
case JavadocTokenTypes.HTML_ELEMENT_START:
checkHtmlElementStart(token);
break;
case JavadocTokenTypes.JAVADOC_INLINE_TAG:
checkJavadocInlineTag(token);
checkCodeSnippetNaming(token);
default:
// Checkstyle complains if there's no default block in switch
break;
}
}

/**
* Requirement of rules:
* (1) No usage of '<code>', '<pre>', use '{@codesnippet ...} instead'.
Comment thread
mssfang marked this conversation as resolved.
Outdated
* (2) No multiple lines span at the '{@code ...}'
*
* @param htmlElementStartNode
Comment thread
mssfang marked this conversation as resolved.
Outdated
*/
private void checkHtmlElementStart(DetailNode htmlElementStartNode) {
final DetailNode tagNameNode = JavadocUtil.findFirstToken(htmlElementStartNode, JavadocTokenTypes.HTML_TAG_NAME);
final String tagName = tagNameNode.getText();
if ("code".equals(tagName) || "pre".equals(tagName)) {
Comment thread
mssfang marked this conversation as resolved.
Outdated
log(tagNameNode.getLineNumber(), tagNameNode.getColumnNumber(),
String.format("Do not use <%s> html tag in javadoc. Use <codesnippet> instead.", tagName));
Comment thread
mssfang marked this conversation as resolved.
Outdated
}
}

/**
* Check to see if the JAVADOC_INLINE_TAG node is '@code' tag or '@codesnippet' tag.
* If the JAVADOC_INLINE_TAG is one of two, check if the tag contains new line or leading asterisk,
* which implies the tag has spanned in multiple lines.
Comment thread
mssfang marked this conversation as resolved.
Outdated
*
* @param inlineTagNode JAVADOC_INLINE_TAG javadoc node
*/
private void checkJavadocInlineTag(DetailNode inlineTagNode) {
boolean isCodeOrCodeSnippet = false;
Comment thread
mssfang marked this conversation as resolved.
Outdated
if (JavadocUtil.findFirstToken(inlineTagNode, JavadocTokenTypes.CODE_LITERAL) != null) {
isCodeOrCodeSnippet = true;
}
isCodeOrCodeSnippet |= isCodeSnippet(inlineTagNode);
if (!isCodeOrCodeSnippet) {
return;
}

for (final DetailNode child : inlineTagNode.getChildren()) {
final int childType = child.getType();

if (childType == JavadocTokenTypes.NEWLINE || childType == JavadocTokenTypes.LEADING_ASTERISK) {
log(child.getLineNumber(), child.getColumnNumber(), "No multiple lines in @code annotation");
}

// This code section duplicates the checking on @codesnippet, maven build already failed it
Comment thread
mssfang marked this conversation as resolved.
Outdated
if (childType == JavadocTokenTypes.DESCRIPTION
&& (!JavadocUtil.containsInBranch(child, JavadocTokenTypes.NEWLINE)
|| !JavadocUtil.containsInBranch(child, JavadocTokenTypes.LEADING_ASTERISK))) {
log(child.getLineNumber(), child.getColumnNumber(), "No multiple lines in @codesnippet annotation");
Comment thread
mssfang marked this conversation as resolved.
Outdated
}
}
Comment thread
mssfang marked this conversation as resolved.
return;
}

/**
* Check to see if the code snippet name matched the naming pattern.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show us an example of a valid naming pattern? And what conditions need to be met for this to fit the naming pattern.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

com.azure.security.keyvault.keys.cryptography.cryptographyclient.encrypt#symmetric-encrypt. More description added.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a valid naming pattern. The #symmetric-encrypt does not match the parameter types of the method here: https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/keyvault/azure-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/CryptographyClient.java#L99

My expectation that this name would actually be com.azure.security.keyvault.keys.cryptography.cryptographyclient.encrypt#encryptionalgorithm-byte-byte-byte

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should enforce that the part after the # matches the argument types of the method.

*
* @param inlineTagNode JAVADOC_INLINE_TAG javadoc node
*/
private void checkCodeSnippetNaming(DetailNode inlineTagNode) {
if (!isCodeSnippet(inlineTagNode)) {
return;
}

final DetailNode descriptionNode = JavadocUtil.findFirstToken(inlineTagNode, JavadocTokenTypes.DESCRIPTION);
if (descriptionNode == null) {
return;
Comment thread
mssfang marked this conversation as resolved.
Outdated
}

final DetailNode textNode = JavadocUtil.findFirstToken(descriptionNode, JavadocTokenTypes.TEXT);
if (textNode == null) {
return;
}

final String namingPattern = textNode.getText();
// Verify naming pattern spec:
// 1, For package name, class name, they should all be lower case;
Comment thread
mssfang marked this conversation as resolved.
Outdated
// For method name, the first letter of method name should be lower case.
// For parameters, they should only be letters. Concatenate all parameters with dash character
// 2, Jonathon will add more requirement specs later
if (!isNamingMatch(namingPattern)) {
log(textNode.getLineNumber(), textNode.getColumnNumber(),
String.format("Naming pattern mismatch. It should only contain lower case for the package path and class name for matching regular expression ''%s''" +
" and the first letter of method name must be small case for matching regular expression ''%s'', and the parameters should be all small case for matching " +
"regular expression ''%s''.", CLASS_PATH_REGEX, METHOD_NAME_REGEX, PARAMETERS_REGEX));
}
}

/**
* Find if the given JAVADOC_INLINE_TAG is a @codesnippet tag.
*
* @param inlineTagNode JAVADOC_INLINE_TAG javadoc node
* @return true if it is a '@codesnippet' tag, false otherwise.
*/
private boolean isCodeSnippet(DetailNode inlineTagNode) {
final DetailNode customNameNode = JavadocUtil.findFirstToken(inlineTagNode, JavadocTokenTypes.CUSTOM_NAME);
if (customNameNode == null) {
Comment thread
mssfang marked this conversation as resolved.
Outdated
return false;
}
return customNameNode.getText().equals("@codesnippet");
}

/**
* Find if the given name of code snippet matched the naming pattern specs.
*
* @param name code snippet name
* @return true if match, otherwise, false
*/
private boolean isNamingMatch(String name) {
final String[] str = name.split("#");
boolean isCorrectNamingPattern = true;
// invalid naming pattern
if (str.length == 0 || str.length > 2) {
return false;
}

final String classPath = str[0];
// corner case: empty string will also be false
if (classPath.isEmpty()) {
return false;
}

final int lastIndexOf = classPath.lastIndexOf(".");
if (lastIndexOf == -1) {
return false;
}

// full path of the class, check to see if there path only has lower case letters and dot
final String className = classPath.substring(0, lastIndexOf);
isCorrectNamingPattern &= Pattern.compile(CLASS_PATH_REGEX).matcher(className).matches();
Comment thread
mssfang marked this conversation as resolved.
Outdated
// method name
final String methodName = classPath.substring(lastIndexOf + 1);
isCorrectNamingPattern &= Pattern.compile(METHOD_NAME_REGEX).matcher(methodName).matches();

// parameters of method function
if (str.length == 2){
Comment thread
mssfang marked this conversation as resolved.
Outdated
isCorrectNamingPattern &= Pattern.compile(PARAMETERS_REGEX).matcher(str[1]).matches();
}

return isCorrectNamingPattern;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,5 +312,16 @@ page at http://checkstyle.sourceforge.net/config.html -->
<!--CUSTOM CHECKS-->
<!-- Must use 'logger.logAndThrow' but not directly calling 'throw exception' -->
<!-- <module name="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck"/>-->

<!--CUSTOM CHECKS-->
Comment thread
mssfang marked this conversation as resolved.
<!-- Requirement of Javadoc annotation @codesnippet check:
1) Use '{@codesnippet ...}' instead of '<code>', '<pre>'.
2) No multiple lines span at the '{@code ...}'.
3) naming pattern of @codesnippet:
a) For package name, class name, they should all be lower case.
For method name, the first letter of method name should be lower case.
For parameters, they should only be letters. Concatenate all parameters with dash character.
b) Jonathon will add more requirement specs later. -->
<module name="com.azure.tools.checkstyle.checks.JavadocCodeSnippetCheck"/>
</module>
</module>
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public abstract class AadCredentialBuilderBase<T extends AadCredentialBuilderBas
/**
* Specifies the Azure Active Directory endpoint to acquire tokens.
* @param authorityHost the Azure Active Directory endpoint
* @return {@link <T>} itself
* @return {@code <T>} itself
*/
@SuppressWarnings("unchecked")
public T authorityHost(String authorityHost) {
Expand All @@ -24,7 +24,7 @@ public T authorityHost(String authorityHost) {
/**
* Sets the client ID of the application.
* @param clientId the client ID of the application.
* @return {@link <T>} itself
* @return {@code <T>} itself
*/
@SuppressWarnings("unchecked")
public T clientId(String clientId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public abstract class CredentialBuilderBase<T extends CredentialBuilderBase<T>>
/**
* Specifies the max number of retries when an authentication request fails.
* @param maxRetry the number of retries
* @return {@link <T>} itself
* @return {@code <T>} itself
*/
@SuppressWarnings("unchecked")
public T maxRetry(int maxRetry) {
Expand All @@ -33,7 +33,7 @@ public T maxRetry(int maxRetry) {
/**
* Specifies a Function to calculate seconds of timeout on every retried request.
* @param retryTimeout the Function that returns a timeout in seconds given the number of retry
* @return {@link <T>} itself
* @return {@code <T>} itself
*/
@SuppressWarnings("unchecked")
public T retryTimeout(Function<Integer, Integer> retryTimeout) {
Expand All @@ -44,7 +44,7 @@ public T retryTimeout(Function<Integer, Integer> retryTimeout) {
/**
* Specifies he options for proxy configuration.
* @param proxyOptions the options for proxy configuration
* @return {@link <T>} itself
* @return {@code <T>} itself
*/
@SuppressWarnings("unchecked")
public T proxyOptions(ProxyOptions proxyOptions) {
Expand Down