Skip to content

Commit

Permalink
Split InvalidInlineTag out of InlineTag and add checks for common mis…
Browse files Browse the repository at this point in the history 
…takes when writing inline tags:

@link{Foo} and `@param foo`.

RELNOTES: Split InvalidTag into another check for invalid inline tags.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229149694
  • Loading branch information
@graememorgan @ronshapiro
graememorgan authored and ronshapiro committed Jan 15, 2019
1 parent 8564524 commit 5791504
Show file tree
Hide file tree
Showing 7 changed files with 484 additions and 215 deletions.
223 changes: 35 additions & 188 deletions ...prone/bugpatterns/javadoc/InvalidTag.java → .../bugpatterns/javadoc/InvalidBlockTag.java
View file
Expand Up @@ -18,10 +18,11 @@

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.bugpatterns.javadoc.JavadocTag.blockTag;
import static com.google.errorprone.bugpatterns.javadoc.JavadocTag.inlineTag;
import static com.google.errorprone.bugpatterns.javadoc.Utils.diagnosticPosition;
import static com.google.errorprone.bugpatterns.javadoc.Utils.replace;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
Expand All @@ -31,128 +32,56 @@
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.bugpatterns.javadoc.InvalidTag.JavadocTag.TagType;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.FindIdentifiers;
import com.sun.source.doctree.DocTree;
import com.sun.source.doctree.EndElementTree;
import com.sun.source.doctree.ErroneousTree;
import com.sun.source.doctree.StartElementTree;
import com.sun.source.doctree.UnknownBlockTagTree;
import com.sun.source.doctree.UnknownInlineTagTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.DocTreePath;
import com.sun.source.util.DocTreePathScanner;
import com.sun.tools.javac.code.Kinds.KindSelector;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.tree.DCTree.DCBlockTag;
import com.sun.tools.javac.tree.DCTree.DCDocComment;
import com.sun.tools.javac.tree.DCTree.DCErroneous;
import com.sun.tools.javac.tree.DCTree.DCInlineTag;
import com.sun.tools.javac.util.JCDiagnostic;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* Matches invalid Javadoc tags, and tries to suggest fixes.
*
* @author ghm@google.com (Graeme Morgan)
*/
@BugPattern(
name = "InvalidTag",
name = "InvalidBlockTag",
summary = "This tag is invalid.",
severity = WARNING,
tags = StandardTags.STYLE,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
public final class InvalidTag extends BugChecker
public final class InvalidBlockTag extends BugChecker
implements ClassTreeMatcher, MethodTreeMatcher, VariableTreeMatcher {

private static final String INVALID_TAG_IS_PARAMETER_NAME =
"@%1$s is not a valid tag, but is a parameter name. "
+ "Use {@code %1%s} to refer to parameter names inline.";

/** Non-standard commonly-used tags which we should allow. */
private static final ImmutableSet<JavadocTag> KNOWN_OTHER_TAGS =
ImmutableSet.of(
blockTag("apiNote"),
blockTag("attr"), // commonly used by Android
blockTag("contact"),
blockTag("hide"),
blockTag("implNote"),
blockTag("implSpec"),
blockTag("required"),
blockTag("team"));

/**
* HTML tags which imply we're showing code, and should therefore probably escape unknown block
* tags.
*/
private static final ImmutableSet<String> CODE_TAGS = ImmutableSet.of("code", "pre");

private static final ImmutableSet<JavadocTag> COMMON_TAGS =
ImmutableSet.of(
inlineTag("code"),
blockTag("deprecated"),
inlineTag("docRoot"),
inlineTag("link"),
inlineTag("linkplain"),
inlineTag("literal"),
blockTag("see"),
blockTag("since"));

private static final ImmutableSet<JavadocTag> VALID_METHOD_TAGS =
ImmutableSet.<JavadocTag>builder()
.addAll(COMMON_TAGS)
.add(
blockTag("author"),
blockTag("exception"),
inlineTag("inheritDoc"),
blockTag("param"),
blockTag("return"),
blockTag("serial"),
blockTag("throws"),
blockTag("serialData"),
blockTag("serialField"),
inlineTag("value"),
blockTag("version"))
.build();

private static final ImmutableSet<JavadocTag> VALID_VARIABLE_TAGS =
ImmutableSet.<JavadocTag>builder()
.addAll(COMMON_TAGS)
.add(
blockTag("serial"),
blockTag("serialData"),
blockTag("serialField"),
inlineTag("value"))
.build();

private static final ImmutableSet<JavadocTag> VALID_CLASS_TAGS =
ImmutableSet.<JavadocTag>builder()
.addAll(COMMON_TAGS)
.add(
blockTag("author"),
inlineTag("inheritDoc"),
blockTag("param"),
inlineTag("value"),
blockTag("version"))
.build();

private static final Pattern PARAM_MATCHER = Pattern.compile("\\{@param (.*)}");

@Override
public Description matchClass(ClassTree classTree, VisitorState state) {
DocTreePath path = Utils.getDocTreePath(state);
if (path != null) {
ImmutableSet<String> parameters = ImmutableSet.of();
new InvalidTagChecker(state, VALID_CLASS_TAGS, parameters).scan(path, null);
new InvalidTagChecker(state, JavadocTag.VALID_CLASS_TAGS, parameters).scan(path, null);
}
return Description.NO_MATCH;
}
Expand All @@ -165,7 +94,7 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
methodTree.getParameters().stream()
.map(v -> v.getName().toString())
.collect(toImmutableSet());
new InvalidTagChecker(state, VALID_METHOD_TAGS, parameters).scan(path, null);
new InvalidTagChecker(state, JavadocTag.VALID_METHOD_TAGS, parameters).scan(path, null);
}
return Description.NO_MATCH;
}
Expand All @@ -174,7 +103,8 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) {
public Description matchVariable(VariableTree variableTree, VisitorState state) {
DocTreePath path = Utils.getDocTreePath(state);
if (path != null) {
new InvalidTagChecker(state, VALID_VARIABLE_TAGS, /* parameters= */ ImmutableSet.of())
new InvalidTagChecker(
state, JavadocTag.VALID_VARIABLE_TAGS, /* parameters= */ ImmutableSet.of())
.scan(path, null);
}
return Description.NO_MATCH;
Expand Down Expand Up @@ -215,47 +145,30 @@ public Void visitEndElement(EndElementTree endElementTree, Void unused) {
@Override
public Void visitErroneous(ErroneousTree erroneousTree, Void unused) {
JCDiagnostic diagnostic = ((DCErroneous) erroneousTree).diag;
if (diagnostic.getCode().equals("compiler.err.dc.bad.inline.tag")) {
JavadocTag tag = inlineTag(erroneousTree.toString().replace("@", ""));
SuggestedFix fix =
validTags.contains(tag)
? replace(erroneousTree, String.format("{%s}", erroneousTree), state)
: SuggestedFix.builder().build();
String message =
String.format(
"%s is not a valid block tag. Should it be an inline tag instead?", erroneousTree);
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(fix)
.build());
if (!diagnostic.getCode().equals("compiler.err.dc.bad.inline.tag")) {
return null;
}
Matcher matcher = PARAM_MATCHER.matcher(erroneousTree.getBody());
if (matcher.find()) {
String parameterName = matcher.group(1);
if (parameters.contains(parameterName)) {
String message =
String.format(
"@param cannot be used inline to refer to parameters; {@code %s} is recommended",
parameterName);
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(
Utils.replace(
erroneousTree, String.format("{@code %s}", parameterName), state))
.build());
}
}
JavadocTag tag = inlineTag(erroneousTree.toString().replace("@", ""));
SuggestedFix fix =
validTags.contains(tag)
? replace(erroneousTree, String.format("{%s}", erroneousTree), state)
: SuggestedFix.builder().build();
String message =
String.format(
"%s is not a valid block tag. Should it be an inline tag instead?", erroneousTree);
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(fix)
.build());
return null;
}

@Override
public Void visitUnknownBlockTag(UnknownBlockTagTree unknownBlockTagTree, Void unused) {
String tagName = unknownBlockTagTree.getTagName();
JavadocTag tag = blockTag(tagName);
if (KNOWN_OTHER_TAGS.contains(tag)) {
if (JavadocTag.KNOWN_OTHER_TAGS.contains(tag)) {
return super.visitUnknownBlockTag(unknownBlockTagTree, null);
}
if (codeTagNestedDepth > 0) {
Expand Down Expand Up @@ -322,46 +235,6 @@ private boolean parentIsErroneousCodeTag() {
dc -> dc instanceof DCErroneous && ((DCErroneous) dc).body.startsWith("{@code"));
}

@Override
public Void visitUnknownInlineTag(UnknownInlineTagTree unknownInlineTagTree, Void unused) {
String name = unknownInlineTagTree.getTagName();
if (parameters.contains(name)) {
String message = String.format(INVALID_TAG_IS_PARAMETER_NAME, name);
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(
Utils.replace(unknownInlineTagTree, String.format("{@code %s}", name), state))
.build());
fixedTags.add(unknownInlineTagTree);
return super.visitUnknownInlineTag(unknownInlineTagTree, null);
}
if (isProbablyType(name)) {
int startPos = Utils.getStartPosition(unknownInlineTagTree, state);
String message =
String.format(
"The tag {@%1$s} is not valid, and will not display or cross-link "
+ "to the type %1$s correctly. Prefer {@link %1$s}.",
name);
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(SuggestedFix.replace(startPos, startPos + 2, "{@link "))
.build());
fixedTags.add(unknownInlineTagTree);
return super.visitUnknownInlineTag(unknownInlineTagTree, null);
}
reportUnknownTag(unknownInlineTagTree, inlineTag(name));
return super.visitUnknownInlineTag(unknownInlineTagTree, null);
}

private boolean isProbablyType(String name) {
Symbol typeSymbol = FindIdentifiers.findIdent(name, state, KindSelector.TYP);
return typeSymbol instanceof TypeSymbol
|| name.chars().filter(c -> c == '.').count() >= 3
|| name.contains("#");
}

private void reportUnknownTag(DocTree docTree, JavadocTag tag) {
Optional<String> bestMatch =
Utils.getBestMatch(
Expand Down Expand Up @@ -397,47 +270,21 @@ public Void scan(DocTree docTree, Void unused) {
if (fixedTags.contains(docTree)) {
return null;
}
JavadocTag tag = null;
if (docTree instanceof DCBlockTag) {
tag = blockTag(((DCBlockTag) docTree).getTagName());
}
if (docTree instanceof DCInlineTag) {
tag = inlineTag(((DCInlineTag) docTree).getTagName());
if (!(docTree instanceof DCBlockTag)) {
return null;
}
if (tag != null && !validTags.contains(tag) && !KNOWN_OTHER_TAGS.contains(tag)) {
String message =
String.format("The tag @%s is not allowed on this type of element.", tag.name());
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(Utils.replace(docTree, "", state))
.build());
JavadocTag tag = blockTag(((DCBlockTag) docTree).getTagName());
if (validTags.contains(tag) || JavadocTag.KNOWN_OTHER_TAGS.contains(tag)) {
return null;
}
String message =
String.format("The tag @%s is not allowed on this type of element.", tag.name());
state.reportMatch(
buildDescription(diagnosticPosition(getCurrentPath(), state))
.setMessage(message)
.addFix(Utils.replace(docTree, "", state))
.build());
return null;
}
}

@AutoValue
abstract static class JavadocTag {
abstract String name();

abstract TagType type();

enum TagType {
BLOCK,
INLINE
}

private static JavadocTag of(String name, TagType type) {
return new AutoValue_InvalidTag_JavadocTag(name, type);
}
}

private static JavadocTag blockTag(String name) {
return JavadocTag.of(name, JavadocTag.TagType.BLOCK);
}

private static JavadocTag inlineTag(String name) {
return JavadocTag.of(name, TagType.INLINE);
}
}

0 comments on commit 5791504

Please sign in to comment.