Skip to content

Commit

Permalink
Introduce MissingRefasterAnnotation checker
Browse files Browse the repository at this point in the history
See the `MissingRefasterAnnotation.md` for more details.

Feedback is welcome!

Fixes #2232

FUTURE_COPYBARA_INTEGRATE_REVIEW=#2232 from PicnicSupermarket:rossendrijver/bugpattern_refaster_annotations 211966e
PiperOrigin-RevId: 538605421
  • Loading branch information
rickie authored and Error Prone Team committed Jun 7, 2023
1 parent 35e6687 commit dd37677
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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<Tree, AnnotationTree> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -940,6 +941,7 @@ public static ScannerSupplier errorChecks() {
MissingFail.class,
MissingImplementsComparable.class,
MissingOverride.class,
MissingRefasterAnnotation.class,
MissingSummary.class,
MixedMutabilityReturnType.class,
MockNotUsedInProduction.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -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<K, V> {",
" // @Placeholder is missing",
" abstract V function(K key);",
"",
" @BeforeTemplate",
" void before(Map<K, V> map, K key) {",
" if (!map.containsKey(key)) {",
" map.put(key, function(key));",
" }",
" }",
"",
" @AfterTemplate",
" void after(Map<K, V> 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();
}
}
23 changes: 23 additions & 0 deletions docs/bugpattern/MissingRefasterAnnotation.md
Original file line number Diff line number Diff line change
@@ -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();
}
}
```

0 comments on commit dd37677

Please sign in to comment.