Skip to content

Commit

Permalink
PSM-442 Flag likely-wrong @JsonCreator usages (#2)
Browse files Browse the repository at this point in the history
  • Loading branch information
hisener authored Mar 1, 2021
1 parent e4e3ade commit cbc886d
Show file tree
Hide file tree
Showing 2 changed files with 219 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.matchers.Matchers.isType;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.LinkType;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
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;
import com.sun.tools.javac.code.Symbol;
import java.util.Map;
import javax.lang.model.element.AnnotationValue;

/** A {@link BugChecker} which flags ambiguous {@code @JsonCreator}s in enums. */
@AutoService(BugChecker.class)
@BugPattern(
name = "AmbiguousJsonCreator",
summary = "`JsonCreator.Mode` should be set for single-argument creators",
linkType = LinkType.NONE,
severity = SeverityLevel.WARNING,
tags = StandardTags.LIKELY_ERROR,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
public final class AmbiguousJsonCreatorCheck extends BugChecker implements AnnotationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<AnnotationTree> IS_JSON_CREATOR_ANNOTATION =
isType("com.fasterxml.jackson.annotation.JsonCreator");

@Override
public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
if (!IS_JSON_CREATOR_ANNOTATION.matches(tree, state)) {
return Description.NO_MATCH;
}

ClassTree clazz = state.findEnclosing(ClassTree.class);
if (clazz == null || clazz.getKind() != Tree.Kind.ENUM) {
return Description.NO_MATCH;
}

MethodTree method = state.findEnclosing(MethodTree.class);
if (method == null || method.getParameters().size() != 1) {
return Description.NO_MATCH;
}

boolean customMode =
ASTHelpers.getAnnotationMirror(tree).getElementValues().entrySet().stream()
.filter(entry -> entry.getKey().getSimpleName().contentEquals("mode"))
.map(Map.Entry::getValue)
.map(AnnotationValue::getValue)
.filter(Symbol.VarSymbol.class::isInstance)
.map(Symbol.VarSymbol.class::cast)
.anyMatch(varSymbol -> !varSymbol.getSimpleName().contentEquals("DEFAULT"));

return customMode
? Description.NO_MATCH
: describeMatch(
tree, SuggestedFix.replace(tree, "@JsonCreator(mode = JsonCreator.Mode.DELEGATING)"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.common.base.Predicates;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class AmbiguousJsonCreatorCheckTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(AmbiguousJsonCreatorCheck.class, getClass())
.expectErrorMessage(
"X",
Predicates.containsPattern(
"`JsonCreator.Mode` should be set for single-argument creators"));
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(new AmbiguousJsonCreatorCheck(), getClass());

@Test
void testIdentification() {
compilationTestHelper
.addSourceLines(
"Container.java",
"import com.fasterxml.jackson.annotation.JsonCreator;",
"import com.fasterxml.jackson.annotation.JsonValue;",
"",
"interface Container {",
" enum A {",
" FOO(1);",
"",
" private final int i;",
"",
" A(int i) {",
" this.i = i;",
" }",
"",
" // BUG: Diagnostic matches: X",
" @JsonCreator",
" public static A of(int i) {",
" return FOO;",
" }",
" }",
"",
" enum B {",
" FOO(1);",
"",
" private final int i;",
"",
" B(int i) {",
" this.i = i;",
" }",
"",
" @JsonCreator(mode = JsonCreator.Mode.DELEGATING)",
" public static B of(int i) {",
" return FOO;",
" }",
" }",
"",
" enum C {",
" FOO(1, \"s\");",
"",
" @JsonValue private final int i;",
" private final String s;",
"",
" C(int i, String s) {",
" this.i = i;",
" this.s = s;",
" }",
"",
" // BUG: Diagnostic matches: X",
" @JsonCreator",
" public static C of(int i) {",
" return FOO;",
" }",
" }",
"",
" enum D {",
" FOO(1, \"s\");",
"",
" private final int i;",
" private final String s;",
"",
" D(int i, String s) {",
" this.i = i;",
" this.s = s;",
" }",
"",
" @JsonCreator",
" public static D of(int i, String s) {",
" return FOO;",
" }",
" }",
"",
" enum E {",
" FOO;",
"",
" // BUG: Diagnostic matches: X",
" @JsonCreator",
" public static E of(String s) {",
" return FOO;",
" }",
" }",
"",
" class F {",
" private final String s;",
"",
" F(String s) {",
" this.s = s;",
" }",
"",
" @JsonCreator",
" public static F of(String s) {",
" return new F(s);",
" }",
" }",
"",
"}")
.doTest();
}

@Test
public void testReplacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",
"import com.fasterxml.jackson.annotation.JsonCreator;",
"",
"enum A {",
" FOO;",
"",
" @JsonCreator",
" public static A of(String s) {",
" return FOO;",
" }",
"}")
.addOutputLines(
"out/A.java",
"import com.fasterxml.jackson.annotation.JsonCreator;",
"",
"enum A {",
" FOO;",
"",
" @JsonCreator(mode = JsonCreator.Mode.DELEGATING)",
" public static A of(String s) {",
" return FOO;",
" }",
"}")
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}
}

0 comments on commit cbc886d

Please sign in to comment.