Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -48,7 +48,7 @@ public void visitToken(DetailAST token) {

switch (token.getType()) {
case TokenTypes.PACKAGE_DEF:
final String packageName = FullIdent.createFullIdentBelow(token).getText();
final String packageName = FullIdent.createFullIdent(token.findFirstToken(TokenTypes.DOT)).getText();
isImplementationPackage = packageName.contains("implementation");
break;
case TokenTypes.CLASS_DEF:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.tools.checkstyle.checks;

import com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter;
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.DetailNode;
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.BlockCommentPosition;
import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Locale;

/**
* Codesnippet description should match naming pattern requirement below:
* <ol>
* <li>Package, class, and method names should be concatenated with a dot '.'. Ex., packageName.className.methodName</li>
* <li>Methods arguments should be concatenated with a dash '-'. Ex. String-String for methodName(String s, String s2)</li>
* <li>Use '#' to concatenate 1) and 2), ex packageName.className.methodName#String-String</li>
* <li>Ignore identifier after method arguments</li>
* </ol>
*/
public class JavadocCodeSnippetCheck extends AbstractCheck {

private static final String CODE_SNIPPET_ANNOTATION = "@codesnippet";
private static final String MISSING_CODESNIPPET_TAG_MESSAGE = "There is a @codesnippet block in the JavaDoc, but it"
+ " does not refer to any sample.";

private static final int[] TOKENS = new int[] {
TokenTypes.PACKAGE_DEF,
TokenTypes.BLOCK_COMMENT_BEGIN,
TokenTypes.CLASS_DEF,
TokenTypes.METHOD_DEF
};

private String packageName;
// A container to contains all class name visited, remove the class name when leave the same token
private Deque<String> classNameStack = new ArrayDeque<>();
// Current METHOD_DEF token while traversal tree
private DetailAST methodDefToken = null;

@Override
public int[] getDefaultTokens() {
return getRequiredTokens();
}

@Override
public int[] getAcceptableTokens() {
return getRequiredTokens();
}

@Override
public int[] getRequiredTokens() {
return TOKENS;
}

@Override
public boolean isCommentNodesRequired() {
return true;
}

@Override
public void leaveToken(DetailAST token) {
if (token.getType() == TokenTypes.CLASS_DEF && !classNameStack.isEmpty()) {
classNameStack.pop();
}
}

@Override
public void visitToken(DetailAST token) {
switch (token.getType()) {
case TokenTypes.PACKAGE_DEF:
packageName = FullIdent.createFullIdent(token.findFirstToken(TokenTypes.DOT)).getText();
break;
case TokenTypes.CLASS_DEF:
classNameStack.push(token.findFirstToken(TokenTypes.IDENT).getText());
break;
case TokenTypes.METHOD_DEF:
methodDefToken = token;
break;
case TokenTypes.BLOCK_COMMENT_BEGIN:
checkNamingPattern(token);
break;
default:
// Checkstyle complains if there's no default block in switch
break;
}
}

/**
* Check if the given block comment is on method. If not, skip the check.
* Otherwise, check if the codesnippet has matching the naming pattern
*
* @param blockCommentToken BLOCK_COMMENT_BEGIN token
*/
private void checkNamingPattern(DetailAST blockCommentToken) {
if (!BlockCommentPosition.isOnMethod(blockCommentToken)) {
return;
}

// Turn the DetailAST into a Javadoc DetailNode.
DetailNode javadocNode = null;
try {
javadocNode = DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(blockCommentToken);
} catch (IllegalArgumentException ex) {
// Exceptions are thrown if the JavaDoc has invalid formatting.
}

if (javadocNode == null) {
return;
}

// Iterate through all the top level nodes in the Javadoc, looking for the @codesnippet tag.
for (DetailNode node : javadocNode.getChildren()) {
if (node.getType() != JavadocTokenTypes.JAVADOC_INLINE_TAG) {
continue;
}
// Skip if not codesnippet
DetailNode customNameNode = JavadocUtil.findFirstToken(node, JavadocTokenTypes.CUSTOM_NAME);
if (customNameNode == null || !CODE_SNIPPET_ANNOTATION.equals(customNameNode.getText())) {
return;
}
// Missing Description
DetailNode descriptionNode = JavadocUtil.findFirstToken(node, JavadocTokenTypes.DESCRIPTION);
if (descriptionNode == null) {
log(node.getLineNumber(), MISSING_CODESNIPPET_TAG_MESSAGE);
return;
}

// There will always have TEXT token if there is DESCRIPTION token exists.
String customDescription = JavadocUtil.findFirstToken(descriptionNode, JavadocTokenTypes.TEXT).getText();

// Find method name
final String methodName = methodDefToken.findFirstToken(TokenTypes.IDENT).getText();
final String className = classNameStack.isEmpty() ? "" : classNameStack.peek();
final String parameters = constructParametersString(methodDefToken);
String fullPath = packageName + "." + className + "." + methodName;
final String fullPathWithoutParameters = fullPath;
if (parameters != null) {
fullPath = fullPath + "#" + parameters;
}

// Check for CodeSnippet naming pattern matching
if (customDescription == null || customDescription.isEmpty()
|| !isNamingMatched(customDescription.toLowerCase(Locale.ROOT),
fullPathWithoutParameters.toLowerCase(Locale.ROOT), parameters)) {
log(node.getLineNumber(), String.format("Naming pattern mismatch. The @codesnippet description "
+ "''%s'' does not match ''%s''. Case Insensitive.", customDescription, fullPath));
}
}
}

/**
* Construct a parameters string if the method has arguments.
*
* @param methodDefToken METHOD_DEF token
* @return a valid parameter string or null if no method arguments exist.
*/
private String constructParametersString(DetailAST methodDefToken) {
final StringBuilder sb = new StringBuilder();
// Checks for the parameters of the method
final DetailAST parametersToken = methodDefToken.findFirstToken(TokenTypes.PARAMETERS);
for (DetailAST ast = parametersToken.getFirstChild(); ast != null; ast = ast.getNextSibling()) {
if (ast.getType() != TokenTypes.PARAMETER_DEF) {
continue;
}

final DetailAST typeToken = ast.findFirstToken(TokenTypes.TYPE);
final DetailAST identToken = typeToken.findFirstToken(TokenTypes.IDENT);
String parameterType = "";
if (identToken != null) {
// For example, Map, String, Mono types
parameterType = identToken.getText();
} else {

DetailAST arrayDeclarator = typeToken.findFirstToken(TokenTypes.ARRAY_DECLARATOR);
if (arrayDeclarator == null) {
// For example, int, boolean, byte primitive types
parameterType = typeToken.getFirstChild().getText();
}

DetailAST arrayDeclaratorIterator = arrayDeclarator;
while (arrayDeclaratorIterator != null) {
DetailAST temp = arrayDeclaratorIterator.findFirstToken(TokenTypes.ARRAY_DECLARATOR);
if (temp == null) {
// For example, int[][], byte[] types
parameterType = arrayDeclaratorIterator.getFirstChild().getText();
break;
}
arrayDeclaratorIterator = temp;
}
}
sb.append(parameterType).append("-");
}
int size = sb.length();
if (size == 0) {
return null;
}
return sb.substring(0, size - 1);
}

/**
* Check if the given customDescription from codesnippet matches the naming pattern rule.
*
* @param customDescription full sample code reference name from annotation codesnippet
* @param fullPathWithoutParameters a string contains package name, class name, and method name if exist.
* @param parameters parameters string which concatenate of argument types
* @return false if the given custom description not matched with naming rule. Otherwise, return true.
*/
private boolean isNamingMatched(String customDescription, String fullPathWithoutParameters, String parameters) {
// Two same codesnippet samples should have two different key names,
// For example, for method name methodName(string, string),
// (1) packagename.classname.methodname#string-string
// (2) packagename.classname.methodname#string-string-2
final String[] descriptionSegments = customDescription.split("#");
if (descriptionSegments.length == 1) {
// There exists parameters in the actual Java sample, but there is no custom parameters exist.
if (parameters != null) {
return false;
}

final String pathUntilMethodName = descriptionSegments[0].split("-")[0];
if (!fullPathWithoutParameters.equalsIgnoreCase(pathUntilMethodName)) {
return false;
}
}

if (descriptionSegments.length == 2) {
// Both of codesnippet name and the method has parameters
if (parameters != null) {
return descriptionSegments[1].toLowerCase().startsWith(parameters.toLowerCase());
}

// Codesnippet name has parameters but the method does not.
return false;
}
return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.tools.checkstyle.checks;

import com.puppycrawl.tools.checkstyle.api.DetailAST;
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.BlockCommentPosition;
import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

/**
* Javadoc Inline tag check:
* <ol>
* <li>Use {@literal {@codesnippet ...}} instead of {@literal <code>}, {@literal <pre>}, or {@literal {@code ...}}
* if these tags span multiple lines. Inline code sample are fine as-is</li>
* <li>No check on class-level Javadoc</li>
* </ol>
*/
public class JavadocInlineTagCheck extends AbstractJavadocCheck {
private static final String MULTIPLE_LINE_SPAN_ERROR = "Tag '%s' spans multiple lines. Use @codesnippet annotation"
+ " instead of '%s' to ensure that the code block always compiles.";

// HTML tag set that need to be checked to see if there tags span on multiple lines.
private static final Set<String> CHECK_TAGS = Collections.unmodifiableSet(new HashSet<>(
Arrays.asList("pre", "code")));

@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) {
DetailAST blockCommentToken = getBlockCommentAst();
// Skip check on class-level Javadoc
if (!BlockCommentPosition.isOnMethod(blockCommentToken) && !BlockCommentPosition.isOnConstructor(blockCommentToken)) {
return;
}

switch (token.getType()) {
case JavadocTokenTypes.HTML_ELEMENT_START:
checkHtmlElementStart(token);
break;
case JavadocTokenTypes.JAVADOC_INLINE_TAG:
checkJavadocInlineTag(token);
break;
default:
// Checkstyle complains if there's no default block in switch
break;
}
}

/**
* Use {@literal {@codesnippet ...}} instead of '<code>', '<pre>', or {@literal {@code ...}) if these tags span
* multiple lines. Inline code sample are fine as-is.
*
* @param htmlElementStartNode HTML_ELEMENT_START node
*/
private void checkHtmlElementStart(DetailNode htmlElementStartNode) {
final DetailNode tagNameNode = JavadocUtil.findFirstToken(htmlElementStartNode, JavadocTokenTypes.HTML_TAG_NAME);
// HTML tags are case-insensitive
final String tagName = tagNameNode.getText().toLowerCase();
if (!CHECK_TAGS.contains(tagName)) {
return;
}

final String tagNameBracket = "<" + tagName + ">";
final DetailNode htmlTagNode = htmlElementStartNode.getParent();
if (!isInlineCode(htmlTagNode)) {
log(htmlTagNode.getLineNumber(), htmlTagNode.getColumnNumber(),
String.format(MULTIPLE_LINE_SPAN_ERROR, tagNameBracket, tagNameBracket));
}
}

/**
* Check to see if the JAVADOC_INLINE_TAG node is {@literal @code} tag. If it is, check if the tag contains a new line
* or a leading asterisk, which implies the tag has spanned in multiple lines.
*
* @param inlineTagNode JAVADOC_INLINE_TAG javadoc node
*/
private void checkJavadocInlineTag(DetailNode inlineTagNode) {
final DetailNode codeLiteralNode = JavadocUtil.findFirstToken(inlineTagNode, JavadocTokenTypes.CODE_LITERAL);
if (codeLiteralNode == null) {
return;
}

final String codeLiteral = codeLiteralNode.getText();
if (!isInlineCode(inlineTagNode)) {
log(codeLiteralNode.getLineNumber(), codeLiteralNode.getColumnNumber(),
String.format(MULTIPLE_LINE_SPAN_ERROR, codeLiteral, codeLiteral));
}
}

/**
* Find if the given tag node is in-line code sample.
* @param node A given node that could be HTML_TAG or JAVADOC_INLINE_TAG
* @return false if it is a code block, otherwise, return true if it is a in-line code.
*/
private boolean isInlineCode(DetailNode node) {
for (final DetailNode child : node.getChildren()) {
final int childType = child.getType();
if (childType == JavadocTokenTypes.NEWLINE || childType == JavadocTokenTypes.LEADING_ASTERISK) {
return false;
}
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,13 @@
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files="com.azure.storage.blob.BlobOutputStream.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files="com.azure.storage.blob.BlockBlobClient.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files="com.azure.storage.blob.PageBlobAsyncClient.java"/>

<!-- JavadocInlineTagCheck Suppression for now, which need code owner's attention -->
Copy link
Member

Choose a reason for hiding this comment

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

Did you create issues for each one and link them here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Will create it. Thank you for remaining me

<suppress checks="com.azure.tools.checkstyle.checks.JavadocInlineTagCheck" files="com.azure.core.http.HttpResponse.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.JavadocInlineTagCheck" files="com.azure.security.keyvault.keys.KeyAsyncClient.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.JavadocInlineTagCheck" files="com.azure.security.keyvault.keys.KeyClient.java"/>

<!-- CodeSnippet Suppression for now, which need code owner's attention -->
<suppress checks="com.azure.tools.checkstyle.checks.JavadocCodeSnippetCheck" files="com.azure.data.appconfiguration.ConfigurationAsyncClient.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.JavadocCodeSnippetCheck" files="com.azure.messaging.eventhubs.EventData.java"/>
</suppressions>
Loading