From dd37677ad11fced945f9741960b220e2666d90e4 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Wed, 7 Jun 2023 15:15:08 -0700 Subject: [PATCH] Introduce `MissingRefasterAnnotation` checker See the `MissingRefasterAnnotation.md` for more details. Feedback is welcome! Fixes #2232 FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/error-prone/pull/2232 from PicnicSupermarket:rossendrijver/bugpattern_refaster_annotations 211966ef3d57931b9c1f5399c0ac9f62a2abbb76 PiperOrigin-RevId: 538605421 --- .../MissingRefasterAnnotation.java | 64 ++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + .../MissingRefasterAnnotationTest.java | 112 ++++++++++++++++++ docs/bugpattern/MissingRefasterAnnotation.md | 23 ++++ 4 files changed, 201 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotation.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotationTest.java create mode 100644 docs/bugpattern/MissingRefasterAnnotation.md diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotation.java b/core/src/main/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotation.java new file mode 100644 index 00000000000..244b847d71c --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotation.java @@ -0,0 +1,64 @@ +/* + * Copyright 2022 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +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.Matchers.annotations; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.isType; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.MultiMatcher; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern( + summary = "The Refaster template contains a method without any Refaster annotations", + severity = WARNING, + tags = LIKELY_ERROR) +public final class MissingRefasterAnnotation extends BugChecker implements ClassTreeMatcher { + private static final MultiMatcher HAS_REFASTER_ANNOTATION = + annotations( + AT_LEAST_ONE, + anyOf( + isType("com.google.errorprone.refaster.annotation.Placeholder"), + isType("com.google.errorprone.refaster.annotation.BeforeTemplate"), + isType("com.google.errorprone.refaster.annotation.AfterTemplate"))); + + @Override + public Description matchClass(ClassTree tree, VisitorState state) { + long methodTypeCount = + tree.getMembers().stream() + .filter(member -> member.getKind() == Tree.Kind.METHOD) + .map(MethodTree.class::cast) + .filter(method -> !ASTHelpers.isGeneratedConstructor(method)) + .map(method -> HAS_REFASTER_ANNOTATION.matches(method, state)) + .distinct() + .count(); + + return methodTypeCount < 2 ? Description.NO_MATCH : describeMatch(tree); + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index c9ba0d806bc..a0eee7e0808 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -227,6 +227,7 @@ import com.google.errorprone.bugpatterns.MissingFail; import com.google.errorprone.bugpatterns.MissingImplementsComparable; import com.google.errorprone.bugpatterns.MissingOverride; +import com.google.errorprone.bugpatterns.MissingRefasterAnnotation; import com.google.errorprone.bugpatterns.MissingSuperCall; import com.google.errorprone.bugpatterns.MissingTestCall; import com.google.errorprone.bugpatterns.MisusedDayOfYear; @@ -940,6 +941,7 @@ public static ScannerSupplier errorChecks() { MissingFail.class, MissingImplementsComparable.class, MissingOverride.class, + MissingRefasterAnnotation.class, MissingSummary.class, MixedMutabilityReturnType.class, MockNotUsedInProduction.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotationTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotationTest.java new file mode 100644 index 00000000000..29f8ccb9ba4 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/MissingRefasterAnnotationTest.java @@ -0,0 +1,112 @@ +/* + * Copyright 2022 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import com.google.common.base.Predicates; +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link MissingRefasterAnnotation}. */ +@RunWith(JUnit4.class) +public final class MissingRefasterAnnotationTest { + private final CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(MissingRefasterAnnotation.class, getClass()) + .expectErrorMessage( + "X", + Predicates.containsPattern( + "The Refaster template contains a method without any Refaster annotations")); + + @Test + public void testIdentification() { + compilationTestHelper + .addSourceLines( + "A.java", + "import com.google.errorprone.refaster.annotation.AfterTemplate;", + "import com.google.errorprone.refaster.annotation.AlsoNegation;", + "import com.google.errorprone.refaster.annotation.BeforeTemplate;", + "import java.util.Map;", + "", + "class A {", + " // BUG: Diagnostic matches: X", + " static final class MethodLacksBeforeTemplateAnnotation {", + " @BeforeTemplate", + " boolean before1(String string) {", + " return string.equals(\"\");", + " }", + "", + " // @BeforeTemplate is missing", + " boolean before2(String string) {", + " return string.length() == 0;", + " }", + "", + " @AfterTemplate", + " @AlsoNegation", + " boolean after(String string) {", + " return string.isEmpty();", + " }", + " }", + "", + " // BUG: Diagnostic matches: X", + " static final class MethodLacksAfterTemplateAnnotation {", + " @BeforeTemplate", + " boolean before(String string) {", + " return string.equals(\"\");", + " }", + "", + " // @AfterTemplate is missing", + " boolean after(String string) {", + " return string.isEmpty();", + " }", + " }", + "", + " // BUG: Diagnostic matches: X", + " abstract class MethodLacksPlaceholderAnnotation {", + " // @Placeholder is missing", + " abstract V function(K key);", + "", + " @BeforeTemplate", + " void before(Map map, K key) {", + " if (!map.containsKey(key)) {", + " map.put(key, function(key));", + " }", + " }", + "", + " @AfterTemplate", + " void after(Map map, K key) {", + " map.computeIfAbsent(key, k -> function(k));", + " }", + " }", + "", + " static final class ValidRefasterTemplate {", + " @BeforeTemplate", + " void unusedPureFunctionCall(Object o) {", + " o.toString();", + " }", + " }", + "", + " static final class NotARefasterTemplate {", + " @Override", + " public String toString() {", + " return \"This is not a Refaster template\";", + " }", + " }", + "}") + .doTest(); + } +} diff --git a/docs/bugpattern/MissingRefasterAnnotation.md b/docs/bugpattern/MissingRefasterAnnotation.md new file mode 100644 index 00000000000..4d80d5b3f8d --- /dev/null +++ b/docs/bugpattern/MissingRefasterAnnotation.md @@ -0,0 +1,23 @@ +A Refaster template consists of multiple methods. Typically, each method in the +class has an annotation. If a method has no annotation, this is likely an +oversight. + +```java +static final class MethodLacksBeforeTemplateAnnotation { + @BeforeTemplate + boolean before1(String string) { + return string.equals(""); + } + + // @BeforeTemplate is missing + boolean before2(String string) { + return string.length() == 0; + } + + @AfterTemplate + @AlsoNegation + boolean after(String string) { + return string.isEmpty(); + } +} +```