diff --git a/error-prone-contrib/README.md b/error-prone-contrib/README.md index cf5d592597..5b360a3f29 100644 --- a/error-prone-contrib/README.md +++ b/error-prone-contrib/README.md @@ -125,9 +125,6 @@ The following is a list of checks we'd like to see implemented: statement. Idem for other exception types. - A Guava-specific check that replaces simple anonymous `CacheLoader` subclass declarations with `CacheLoader.from(someLambda)`. -- A Spring-specific check that enforces that methods with the `@Scheduled` - annotation are also annotated with New Relic's `@Trace` annotation. Such - methods should ideally not also represent Spring MVC endpoints. - A Spring-specific check that enforces that `@RequestMapping` annotations, when applied to a method, explicitly specify one or more target HTTP methods. - A Spring-specific check that looks for classes in which all `@RequestMapping` diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index cd4016dfe8..1c70012716 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -81,11 +81,6 @@ guava provided - - com.newrelic.agent.java - newrelic-api - test - io.projectreactor reactor-core diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTrace.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTrace.java deleted file mode 100644 index 73a9ec5f50..0000000000 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTrace.java +++ /dev/null @@ -1,94 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import static com.google.errorprone.BugPattern.LinkType.CUSTOM; -import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; -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.hasAnnotation; -import static com.google.errorprone.matchers.Matchers.isType; -import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; - -import com.google.auto.common.AnnotationMirrors; -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.MethodTreeMatcher; -import com.google.errorprone.fixes.SuggestedFix; -import com.google.errorprone.fixes.SuggestedFixes; -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.MethodTree; -import com.sun.source.tree.Tree; -import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary; - -/** - * A {@link BugChecker} that flags methods with Spring's {@code @Scheduled} annotation that lack New - * Relic Agent's {@code @Trace(dispatcher = true)}. - */ -@AutoService(BugChecker.class) -@BugPattern( - summary = "Scheduled operation must start a new New Relic transaction", - link = BUG_PATTERNS_BASE_URL + "ScheduledTransactionTrace", - linkType = CUSTOM, - severity = ERROR, - tags = LIKELY_ERROR) -public final class ScheduledTransactionTrace extends BugChecker implements MethodTreeMatcher { - private static final long serialVersionUID = 1L; - private static final String TRACE_ANNOTATION_FQCN = "com.newrelic.api.agent.Trace"; - private static final Matcher IS_SCHEDULED = - hasAnnotation("org.springframework.scheduling.annotation.Scheduled"); - private static final MultiMatcher TRACE_ANNOTATION = - annotations(AT_LEAST_ONE, isType(TRACE_ANNOTATION_FQCN)); - - /** Instantiates a new {@link ScheduledTransactionTrace} instance. */ - public ScheduledTransactionTrace() {} - - @Override - public Description matchMethod(MethodTree tree, VisitorState state) { - if (!ThirdPartyLibrary.NEW_RELIC_AGENT_API.isIntroductionAllowed(state) - || !IS_SCHEDULED.matches(tree, state)) { - return Description.NO_MATCH; - } - - ImmutableList traceAnnotations = - TRACE_ANNOTATION.multiMatchResult(tree, state).matchingNodes(); - if (traceAnnotations.isEmpty()) { - /* This method completely lacks the `@Trace` annotation; add it. */ - return describeMatch( - tree, - SuggestedFix.builder() - .addImport(TRACE_ANNOTATION_FQCN) - .prefixWith(tree, "@Trace(dispatcher = true)") - .build()); - } - - AnnotationTree traceAnnotation = Iterables.getOnlyElement(traceAnnotations); - if (isCorrectAnnotation(traceAnnotation)) { - return Description.NO_MATCH; - } - - /* - * The `@Trace` annotation is present but does not specify `dispatcher = true`. Add or update - * the `dispatcher` annotation element. - */ - return describeMatch( - traceAnnotation, - SuggestedFixes.updateAnnotationArgumentValues( - traceAnnotation, state, "dispatcher", ImmutableList.of("true")) - .build()); - } - - private static boolean isCorrectAnnotation(AnnotationTree traceAnnotation) { - return Boolean.TRUE.equals( - AnnotationMirrors.getAnnotationValue( - ASTHelpers.getAnnotationMirror(traceAnnotation), "dispatcher") - .getValue()); - } -} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibrary.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibrary.java index ad47462880..51678cd335 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibrary.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibrary.java @@ -32,13 +32,6 @@ public enum ThirdPartyLibrary { * @see Guava on GitHub */ GUAVA("com.google.common.collect.ImmutableList"), - /** - * New Relic's Java agent API. - * - * @see New Relic - * Java agent API on GitHub - */ - NEW_RELIC_AGENT_API("com.newrelic.api.agent.Agent"), /** * VMWare's Project Reactor. * diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java deleted file mode 100644 index c61a99a8ca..0000000000 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceTest.java +++ /dev/null @@ -1,105 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import com.google.errorprone.BugCheckerRefactoringTestHelper; -import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; -import com.google.errorprone.CompilationTestHelper; -import org.junit.jupiter.api.Test; -import org.springframework.scheduling.annotation.Scheduled; - -final class ScheduledTransactionTraceTest { - @Test - void identification() { - CompilationTestHelper.newInstance(ScheduledTransactionTrace.class, getClass()) - .addSourceLines( - "A.java", - "import com.newrelic.api.agent.Trace;", - "import org.springframework.scheduling.annotation.Scheduled;", - "", - "class A {", - " void notScheduled() {}", - "", - " @Scheduled(fixedDelay = 1)", - " // BUG: Diagnostic contains:", - " void scheduledButNotTraced() {}", - "", - " @Scheduled(fixedDelay = 1)", - " // BUG: Diagnostic contains:", - " @Trace", - " void scheduledButImproperlyTraced1() {}", - "", - " @Scheduled(fixedDelay = 1)", - " // BUG: Diagnostic contains:", - " @Trace(dispatcher = false)", - " void scheduledButImproperlyTraced2() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(dispatcher = true)", - " void scheduledAndProperlyTraced() {}", - "}") - .doTest(); - } - - @Test - void identificationWithoutNewRelicAgentApiOnClasspath() { - CompilationTestHelper.newInstance(ScheduledTransactionTrace.class, getClass()) - .withClasspath(Scheduled.class) - .addSourceLines( - "A.java", - "import org.springframework.scheduling.annotation.Scheduled;", - "", - "class A {", - " @Scheduled(fixedDelay = 1)", - " void scheduledButNotTraced() {}", - "}") - .doTest(); - } - - @Test - void replacement() { - BugCheckerRefactoringTestHelper.newInstance(ScheduledTransactionTrace.class, getClass()) - .addInputLines( - "A.java", - "import com.newrelic.api.agent.Trace;", - "import org.springframework.scheduling.annotation.Scheduled;", - "", - "class A {", - " @Scheduled(fixedDelay = 1)", - " void scheduledButNotTraced() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace", - " void scheduledButImproperlyTraced1() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(dispatcher = false)", - " void scheduledButImproperlyTraced2() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(leaf = true)", - " void scheduledButImproperlyTraced3() {}", - "}") - .addOutputLines( - "A.java", - "import com.newrelic.api.agent.Trace;", - "import org.springframework.scheduling.annotation.Scheduled;", - "", - "class A {", - " @Trace(dispatcher = true)", - " @Scheduled(fixedDelay = 1)", - " void scheduledButNotTraced() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(dispatcher = true)", - " void scheduledButImproperlyTraced1() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(dispatcher = true)", - " void scheduledButImproperlyTraced2() {}", - "", - " @Scheduled(fixedDelay = 1)", - " @Trace(dispatcher = true, leaf = true)", - " void scheduledButImproperlyTraced3() {}", - "}") - .doTest(TestMode.TEXT_MATCH); - } -} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java index 090ef579b5..4338f9af35 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java @@ -23,7 +23,7 @@ void isIntroductionAllowed() { CompilationTestHelper.newInstance(TestChecker.class, getClass()) .addSourceLines( "A.java", - "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true", + "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, REACTOR: true", "class A {}") .doTest(); } @@ -34,16 +34,14 @@ void isIntroductionAllowedWitnessClassesInSymtab() { .addSourceLines( "A.java", "import com.google.common.collect.ImmutableList;", - "import com.newrelic.api.agent.Agent;", "import org.assertj.core.api.Assertions;", "import reactor.core.publisher.Flux;", "", - "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true", + "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, REACTOR: true", "class A {", " void m(Class clazz) {", " m(Assertions.class);", " m(ImmutableList.class);", - " m(Agent.class);", " m(Flux.class);", " }", "}") @@ -56,7 +54,7 @@ void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() { .withClasspath(ImmutableList.class, Flux.class) .addSourceLines( "A.java", - "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: true, NEW_RELIC_AGENT_API: false, REACTOR: true", + "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: true, REACTOR: true", "class A {}") .doTest(); } @@ -67,7 +65,7 @@ void isIntroductionAllowedWitnessClassesNotOnClassPath() { .withClasspath() .addSourceLines( "A.java", - "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: false, NEW_RELIC_AGENT_API: false, REACTOR:", + "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: false, REACTOR:", "// false", "class A {}") .doTest(); @@ -82,8 +80,8 @@ void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) { .addSourceLines( "A.java", String.format( - "// BUG: Diagnostic contains: ASSERTJ: %s, GUAVA: true, NEW_RELIC_AGENT_API: %s, REACTOR: true", - ignoreClassPath, ignoreClassPath), + "// BUG: Diagnostic contains: ASSERTJ: %s, GUAVA: true, REACTOR: true", + ignoreClassPath), "class A {}") .doTest(); } diff --git a/pom.xml b/pom.xml index 6d7a8f0738..1bacddc2af 100644 --- a/pom.xml +++ b/pom.xml @@ -323,11 +323,6 @@ nopen-checker ${version.nopen-checker} - - com.newrelic.agent.java - newrelic-api - 8.6.0 - @@ -728,7 +723,6 @@ -