mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 08:11:25 +00:00
Introduce BugPatternLink check (#1026)
This check validates that checks defined by Error Prone Support reference the public website, unless they explicitly specify that their diagnostics output must not include a link.
This commit is contained in:
committed by
GitHub
parent
1fe67677b4
commit
28bb4f6895
@@ -0,0 +1,140 @@
|
|||||||
|
package tech.picnic.errorprone.guidelines.bugpatterns;
|
||||||
|
|
||||||
|
import static com.google.common.base.Verify.verify;
|
||||||
|
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
|
||||||
|
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||||
|
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
|
||||||
|
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.AT_LEAST_ONE;
|
||||||
|
import static com.google.errorprone.matchers.FieldMatchers.staticField;
|
||||||
|
import static com.google.errorprone.matchers.Matchers.annotations;
|
||||||
|
import static com.google.errorprone.matchers.Matchers.isType;
|
||||||
|
import static com.google.errorprone.matchers.Matchers.packageStartsWith;
|
||||||
|
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
|
||||||
|
|
||||||
|
import com.google.auto.service.AutoService;
|
||||||
|
import com.google.common.collect.ImmutableList;
|
||||||
|
import com.google.common.collect.Iterables;
|
||||||
|
import com.google.errorprone.BugPattern;
|
||||||
|
import com.google.errorprone.VisitorState;
|
||||||
|
import com.google.errorprone.bugpatterns.BugChecker;
|
||||||
|
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
|
||||||
|
import com.google.errorprone.fixes.SuggestedFix;
|
||||||
|
import com.google.errorprone.fixes.SuggestedFixes;
|
||||||
|
import com.google.errorprone.matchers.AnnotationMatcherUtils;
|
||||||
|
import com.google.errorprone.matchers.Description;
|
||||||
|
import com.google.errorprone.matchers.Matcher;
|
||||||
|
import com.google.errorprone.matchers.MultiMatcher;
|
||||||
|
import com.google.errorprone.util.ASTHelpers;
|
||||||
|
import com.sun.source.tree.AnnotationTree;
|
||||||
|
import com.sun.source.tree.BinaryTree;
|
||||||
|
import com.sun.source.tree.ClassTree;
|
||||||
|
import com.sun.source.tree.ExpressionTree;
|
||||||
|
import com.sun.source.tree.Tree.Kind;
|
||||||
|
import com.sun.tools.javac.util.Constants;
|
||||||
|
import javax.lang.model.element.Name;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A {@link BugChecker} that flags {@link BugChecker} declarations inside {@code
|
||||||
|
* tech.picnic.errorprone.*} packages that do not reference the Error Prone Support website.
|
||||||
|
*/
|
||||||
|
// XXX: Introduce a similar check to enforce the Refaster `@OnlineDocumentation` annotation. (Or
|
||||||
|
// update the website generation to document Refaster collections by default, and provide an
|
||||||
|
// exclusion annotation instead. This may make more sense.)
|
||||||
|
@AutoService(BugChecker.class)
|
||||||
|
@BugPattern(
|
||||||
|
summary = "Error Prone Support checks must reference their online documentation",
|
||||||
|
link = BUG_PATTERNS_BASE_URL + "BugPatternLink",
|
||||||
|
linkType = CUSTOM,
|
||||||
|
severity = SUGGESTION,
|
||||||
|
tags = LIKELY_ERROR)
|
||||||
|
public final class BugPatternLink extends BugChecker implements ClassTreeMatcher {
|
||||||
|
private static final long serialVersionUID = 1L;
|
||||||
|
private static final Matcher<ClassTree> IS_ERROR_PRONE_SUPPORT_CLASS =
|
||||||
|
packageStartsWith("tech.picnic.errorprone");
|
||||||
|
private static final Matcher<ExpressionTree> IS_LINK_TYPE_NONE =
|
||||||
|
staticField(BugPattern.LinkType.class.getCanonicalName(), "NONE");
|
||||||
|
private static final Matcher<ExpressionTree> IS_BUG_PATTERNS_BASE_URL =
|
||||||
|
staticField("tech.picnic.errorprone.utils.Documentation", "BUG_PATTERNS_BASE_URL");
|
||||||
|
private static final MultiMatcher<ClassTree, AnnotationTree> HAS_BUG_PATTERN_ANNOTATION =
|
||||||
|
annotations(AT_LEAST_ONE, isType(BugPattern.class.getCanonicalName()));
|
||||||
|
|
||||||
|
/** Instantiates a new {@link BugPatternLink} instance. */
|
||||||
|
public BugPatternLink() {}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public Description matchClass(ClassTree tree, VisitorState state) {
|
||||||
|
if (ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class) != null) {
|
||||||
|
/*
|
||||||
|
* This is a nested class; even if it's bug checker, then it's likely declared within a test
|
||||||
|
* class.
|
||||||
|
*/
|
||||||
|
return Description.NO_MATCH;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!IS_ERROR_PRONE_SUPPORT_CLASS.matches(tree, state)) {
|
||||||
|
/*
|
||||||
|
* Bug checkers defined elsewhere are unlikely to be documented on the Error Prone Support
|
||||||
|
* website.
|
||||||
|
*/
|
||||||
|
return Description.NO_MATCH;
|
||||||
|
}
|
||||||
|
|
||||||
|
ImmutableList<AnnotationTree> bugPatternAnnotations =
|
||||||
|
HAS_BUG_PATTERN_ANNOTATION.multiMatchResult(tree, state).matchingNodes();
|
||||||
|
if (bugPatternAnnotations.isEmpty()) {
|
||||||
|
/* This isn't a bug checker. */
|
||||||
|
return Description.NO_MATCH;
|
||||||
|
}
|
||||||
|
|
||||||
|
AnnotationTree annotation = Iterables.getOnlyElement(bugPatternAnnotations);
|
||||||
|
if (isCompliant(annotation, tree.getSimpleName(), state)) {
|
||||||
|
/* The bug checker is correctly configured. */
|
||||||
|
return Description.NO_MATCH;
|
||||||
|
}
|
||||||
|
|
||||||
|
return describeMatch(annotation, suggestFix(tree, state, annotation));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static boolean isCompliant(
|
||||||
|
AnnotationTree annotation, Name className, VisitorState state) {
|
||||||
|
ExpressionTree linkType = AnnotationMatcherUtils.getArgument(annotation, "linkType");
|
||||||
|
if (IS_LINK_TYPE_NONE.matches(linkType, state)) {
|
||||||
|
/* This bug checker explicitly declares that there is no link. */
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
ExpressionTree link = AnnotationMatcherUtils.getArgument(annotation, "link");
|
||||||
|
if (!(link instanceof BinaryTree binary)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
verify(binary.getKind() == Kind.PLUS, "Unexpected binary operator");
|
||||||
|
return IS_BUG_PATTERNS_BASE_URL.matches(binary.getLeftOperand(), state)
|
||||||
|
&& className.contentEquals(ASTHelpers.constValue(binary.getRightOperand(), String.class));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static SuggestedFix suggestFix(
|
||||||
|
ClassTree tree, VisitorState state, AnnotationTree annotation) {
|
||||||
|
SuggestedFix.Builder fix = SuggestedFix.builder();
|
||||||
|
|
||||||
|
String linkPrefix =
|
||||||
|
SuggestedFixes.qualifyStaticImport(
|
||||||
|
"tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL", fix, state);
|
||||||
|
fix.merge(
|
||||||
|
SuggestedFixes.updateAnnotationArgumentValues(
|
||||||
|
annotation,
|
||||||
|
state,
|
||||||
|
"link",
|
||||||
|
ImmutableList.of(
|
||||||
|
linkPrefix + " + " + Constants.format(tree.getSimpleName().toString()))));
|
||||||
|
|
||||||
|
String linkType =
|
||||||
|
SuggestedFixes.qualifyStaticImport(
|
||||||
|
BugPattern.LinkType.class.getCanonicalName() + ".CUSTOM", fix, state);
|
||||||
|
fix.merge(
|
||||||
|
SuggestedFixes.updateAnnotationArgumentValues(
|
||||||
|
annotation, state, "linkType", ImmutableList.of(linkType)));
|
||||||
|
|
||||||
|
return fix.build();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,193 @@
|
|||||||
|
package tech.picnic.errorprone.guidelines.bugpatterns;
|
||||||
|
|
||||||
|
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||||
|
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||||
|
import com.google.errorprone.CompilationTestHelper;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
final class BugPatternLinkTest {
|
||||||
|
@Test
|
||||||
|
void identification() {
|
||||||
|
CompilationTestHelper.newInstance(BugPatternLink.class, getClass())
|
||||||
|
.addSourceLines(
|
||||||
|
"A.java",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(summary = \"Class in default package\", severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class A {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"com/example/B.java",
|
||||||
|
"package com.example;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(summary = \"Class in custom package\", severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class B {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/C.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Class explicitly without link\",",
|
||||||
|
" linkType = BugPattern.LinkType.NONE,",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class C {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/subpackage/D.java",
|
||||||
|
"package tech.picnic.errorprone.subpackage;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"import tech.picnic.errorprone.utils.Documentation;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class in subpackage with proper link\",",
|
||||||
|
" link = Documentation.BUG_PATTERNS_BASE_URL + \"D\",",
|
||||||
|
" linkType = BugPattern.LinkType.CUSTOM,",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class D {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/E.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
|
||||||
|
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
|
||||||
|
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class with proper link and static imports\",",
|
||||||
|
" link = BUG_PATTERNS_BASE_URL + \"E\",",
|
||||||
|
" linkType = CUSTOM,",
|
||||||
|
" severity = ERROR)",
|
||||||
|
"class E {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/F.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"class F {",
|
||||||
|
" @BugPattern(",
|
||||||
|
" summary = \"Nested Error Prone Support class\",",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
" class Inner {}",
|
||||||
|
"}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/G.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"// BUG: Diagnostic contains:",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class lacking link\",",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class G {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/H.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"import tech.picnic.errorprone.utils.Documentation;",
|
||||||
|
"",
|
||||||
|
"// BUG: Diagnostic contains:",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class with incorrect link\",",
|
||||||
|
" link = Documentation.BUG_PATTERNS_BASE_URL + \"NotH\",",
|
||||||
|
" linkType = BugPattern.LinkType.CUSTOM,",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class H {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/I.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"// BUG: Diagnostic contains:",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class with non-canonical link\",",
|
||||||
|
" link = \"https://error-prone.picnic.tech/bugpatterns/I\",",
|
||||||
|
" linkType = BugPattern.LinkType.CUSTOM,",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class I {}")
|
||||||
|
.addSourceLines(
|
||||||
|
"tech/picnic/errorprone/J.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"// BUG: Diagnostic contains:",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class in with non-canonical link\",",
|
||||||
|
" link = \"https://error-prone.picnic.tech/bugpatterns/\" + \"J\",",
|
||||||
|
" linkType = BugPattern.LinkType.CUSTOM,",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class J {}")
|
||||||
|
.doTest();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void replacement() {
|
||||||
|
BugCheckerRefactoringTestHelper.newInstance(BugPatternLink.class, getClass())
|
||||||
|
.addInputLines(
|
||||||
|
"tech/picnic/errorprone/A.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class lacking link\",",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class A {}")
|
||||||
|
.addOutputLines(
|
||||||
|
"tech/picnic/errorprone/A.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
|
||||||
|
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" link = BUG_PATTERNS_BASE_URL + \"A\",",
|
||||||
|
" linkType = CUSTOM,",
|
||||||
|
" summary = \"Error Prone Support class lacking link\",",
|
||||||
|
" severity = BugPattern.SeverityLevel.ERROR)",
|
||||||
|
"class A {}")
|
||||||
|
.addInputLines(
|
||||||
|
"tech/picnic/errorprone/B.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
|
||||||
|
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class with incorrect link\",",
|
||||||
|
" link = \"Not the right link\",",
|
||||||
|
" linkType = CUSTOM,",
|
||||||
|
" severity = ERROR)",
|
||||||
|
"class B {}")
|
||||||
|
.addOutputLines(
|
||||||
|
"tech/picnic/errorprone/B.java",
|
||||||
|
"package tech.picnic.errorprone;",
|
||||||
|
"",
|
||||||
|
"import static com.google.errorprone.BugPattern.LinkType.CUSTOM;",
|
||||||
|
"import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;",
|
||||||
|
"import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;",
|
||||||
|
"",
|
||||||
|
"import com.google.errorprone.BugPattern;",
|
||||||
|
"",
|
||||||
|
"@BugPattern(",
|
||||||
|
" summary = \"Error Prone Support class with incorrect link\",",
|
||||||
|
" link = BUG_PATTERNS_BASE_URL + \"B\",",
|
||||||
|
" linkType = CUSTOM,",
|
||||||
|
" severity = ERROR)",
|
||||||
|
"class B {}")
|
||||||
|
.doTest(TestMode.TEXT_MATCH);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -4,6 +4,7 @@ import static com.google.common.base.Preconditions.checkState;
|
|||||||
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
|
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
|
||||||
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
import static com.google.common.collect.ImmutableSet.toImmutableSet;
|
||||||
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
|
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
|
||||||
|
import static com.google.errorprone.BugPattern.LinkType.NONE;
|
||||||
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
|
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
|
||||||
import static java.util.Comparator.naturalOrder;
|
import static java.util.Comparator.naturalOrder;
|
||||||
import static tech.picnic.errorprone.refaster.runner.Refaster.INCLUDED_RULES_PATTERN_FLAG;
|
import static tech.picnic.errorprone.refaster.runner.Refaster.INCLUDED_RULES_PATTERN_FLAG;
|
||||||
@@ -60,7 +61,7 @@ import tech.picnic.errorprone.refaster.runner.Refaster;
|
|||||||
// XXX: This check currently only validates that one `Refaster.anyOf` branch in one
|
// XXX: This check currently only validates that one `Refaster.anyOf` branch in one
|
||||||
// `@BeforeTemplate` method is covered by a test. Review how we can make sure that _all_
|
// `@BeforeTemplate` method is covered by a test. Review how we can make sure that _all_
|
||||||
// `@BeforeTemplate` methods and `Refaster.anyOf` branches are covered.
|
// `@BeforeTemplate` methods and `Refaster.anyOf` branches are covered.
|
||||||
@BugPattern(summary = "Exercises a Refaster rule collection", severity = ERROR)
|
@BugPattern(summary = "Exercises a Refaster rule collection", linkType = NONE, severity = ERROR)
|
||||||
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
|
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
|
||||||
public final class RefasterRuleCollection extends BugChecker implements CompilationUnitTreeMatcher {
|
public final class RefasterRuleCollection extends BugChecker implements CompilationUnitTreeMatcher {
|
||||||
private static final long serialVersionUID = 1L;
|
private static final long serialVersionUID = 1L;
|
||||||
|
|||||||
Reference in New Issue
Block a user