diff --git a/README.md b/README.md index 646b1e822..ec18dea5c 100644 --- a/README.md +++ b/README.md @@ -198,6 +198,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `UnnecessarilyQualified`: Types should not be qualified if they are also imported. - `DeprecatedGuavaObjects`: `com.google.common.base.Objects` has been obviated by `java.util.Objects`. - `JavaTimeSystemDefaultTimeZone`: Avoid using the system default time zone. +- `IncubatingMethod`: Prevents calling Conjure incubating APIs unless you explicitly opt-out of the check on a per-use or per-project basis. ### Programmatic Application diff --git a/baseline-error-prone/build.gradle b/baseline-error-prone/build.gradle index dbaf3f88a..dd39a11f7 100644 --- a/baseline-error-prone/build.gradle +++ b/baseline-error-prone/build.gradle @@ -22,6 +22,7 @@ dependencies { testCompile 'org.assertj:assertj-core' testCompile 'org.jooq:jooq' testCompile 'com.palantir.tritium:tritium-registry' + testCompile 'com.palantir.conjure.java:conjure-lib' testCompileOnly 'org.immutables:value::annotations' testImplementation 'org.junit.jupiter:junit-jupiter' testRuntimeOnly 'org.junit.jupiter:junit-jupiter-migrationsupport' diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IncubatingMethod.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IncubatingMethod.java new file mode 100644 index 000000000..295da013d --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IncubatingMethod.java @@ -0,0 +1,67 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; + +@AutoService(BugChecker.class) +@BugPattern( + name = "IncubatingMethod", + providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION, + severity = BugPattern.SeverityLevel.ERROR, + summary = "You should avoid using incubating methods where possible, since they have very weak stability" + + " guarantees. You can explicitly disable this check on a case-by-case basis using" + + " @SuppressWarnings(\"IncubatingMethod\").") +public final class IncubatingMethod extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher, BugChecker.MemberReferenceTreeMatcher { + + /** Matcher for the Incubating annotation, using the full qualified path. */ + private static final Matcher INCUBATING_MATCHER = + Matchers.hasAnnotation("com.palantir.conjure.java.lib.internal.Incubating"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + return checkTree(tree, state); + } + + @Override + public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) { + return checkTree(tree, state); + } + + private Description checkTree(Tree tree, VisitorState state) { + if (!INCUBATING_MATCHER.matches(tree, state)) { + return Description.NO_MATCH; + } + + // Allow users to test incubating endpoints in test code without complaining. + if (TestCheckUtils.isTestCode(state)) { + return Description.NO_MATCH; + } + + return describeMatch(tree); + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IncubatingMethodTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IncubatingMethodTest.java new file mode 100644 index 000000000..615f751f5 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IncubatingMethodTest.java @@ -0,0 +1,104 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; + +public class IncubatingMethodTest { + + // A simple service with one incubating method and one non-incubating method. + private static final String[] SERVICE_DEFINITION = new String[] { + "package com.palantir;", + "import com.palantir.conjure.java.lib.internal.Incubating;", + "public interface Service {", + "@Incubating", + "int test();", + "int test2();", + "}" + }; + + @Test + public void testIncubatingMethod() { + CompilationTestHelper.newInstance(IncubatingMethod.class, getClass()) + .addSourceLines("Service.java", SERVICE_DEFINITION) + .addSourceLines( + "Main.java", + "package com.palantir;", + "public final class Main {", + "public static void main(String[] args) {", + "Service service = null;", + "// BUG: Diagnostic contains: You should avoid using incubating methods", + "service.test();", + "}", + "}") + .doTest(); + } + + @Test + public void testIncubatingMethodReference() { + CompilationTestHelper.newInstance(IncubatingMethod.class, getClass()) + .addSourceLines("Service.java", SERVICE_DEFINITION) + .addSourceLines( + "Main.java", + "package com.palantir;", + "import java.util.function.Supplier;", + "public final class Main {", + "public static void main(String[] args) {", + "Service service = null;", + "// BUG: Diagnostic contains: You should avoid using incubating methods", + "Supplier supp = service::test;", + "}", + "}") + .doTest(); + } + + @Test + public void testNonIncubatingMethod() { + CompilationTestHelper.newInstance(IncubatingMethod.class, getClass()) + .addSourceLines("Service.java", SERVICE_DEFINITION) + .addSourceLines( + "Main.java", + "package com.palantir;", + "import java.util.function.Supplier;", + "public final class Main {", + "public static void main(String[] args) {", + "Service service = null;", + "int result = service.test2();", + "Supplier supp = service::test2;", + "}", + "}") + .doTest(); + } + + @Test + public void testSuppressedIncubatingMethod() { + CompilationTestHelper.newInstance(IncubatingMethod.class, getClass()) + .addSourceLines("Service.java", SERVICE_DEFINITION) + .addSourceLines( + "Main.java", + "package com.palantir;", + "public final class Main {", + "@SuppressWarnings(\"IncubatingMethod\")", + "public static void main(String[] args) {", + "Service service = null;", + "service.test();", + "}", + "}") + .doTest(); + } +} diff --git a/changelog/@unreleased/pr-1554.v2.yml b/changelog/@unreleased/pr-1554.v2.yml new file mode 100644 index 000000000..95220c8ea --- /dev/null +++ b/changelog/@unreleased/pr-1554.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Add `IncubatingMethod` errorprone check, which prevents usage of conjure incubating APIs unless explicitly annotated. + links: + - https://github.com/palantir/gradle-baseline/pull/1554 diff --git a/versions.lock b/versions.lock index 70817d095..95116c064 100644 --- a/versions.lock +++ b/versions.lock @@ -14,9 +14,9 @@ com.google.auto.service:auto-service-annotations:1.0-rc6 (2 constraints: 422525b com.google.auto.value:auto-value:1.5.3 (1 constraints: 1c121afb) com.google.auto.value:auto-value-annotations:1.7 (3 constraints: 7c2d7cc7) com.google.code.findbugs:jFormatString:3.0.0 (1 constraints: f01022b8) -com.google.code.findbugs:jsr305:3.0.2 (5 constraints: c9524c6b) +com.google.code.findbugs:jsr305:3.0.2 (6 constraints: 60624d64) com.google.errorprone:error_prone_annotation:2.4.0 (3 constraints: 2e38d75e) -com.google.errorprone:error_prone_annotations:2.4.0 (9 constraints: 3b836072) +com.google.errorprone:error_prone_annotations:2.4.0 (10 constraints: 28937472) com.google.errorprone:error_prone_check_api:2.4.0 (2 constraints: 4e257bbd) com.google.errorprone:error_prone_core:2.4.0 (3 constraints: 61253108) com.google.errorprone:error_prone_refaster:2.4.0 (1 constraints: 08050136) @@ -38,8 +38,8 @@ com.googlecode.java-diff-utils:diffutils:1.3.0 (3 constraints: 3f226bf4) com.googlecode.javaewah:JavaEWAH:1.1.7 (1 constraints: 490e7f50) com.palantir.javaformat:gradle-palantir-java-format:1.1.0 (1 constraints: 0405f335) com.palantir.javaformat:palantir-java-format-spi:1.1.0 (1 constraints: 711560be) -com.palantir.safe-logging:preconditions:1.14.0 (3 constraints: fa23b494) -com.palantir.safe-logging:safe-logging:1.14.0 (3 constraints: 2827c7df) +com.palantir.safe-logging:preconditions:1.14.0 (5 constraints: dc43b195) +com.palantir.safe-logging:safe-logging:1.14.0 (4 constraints: ed368e50) com.palantir.tritium:tritium-registry:0.18.4 (1 constraints: 3f053f3b) com.typesafe:config:1.2.0 (1 constraints: d60cb924) commons-io:commons-io:2.6 (2 constraints: 1e213039) @@ -89,13 +89,18 @@ org.threeten:threeten-extra:1.5.0 (1 constraints: f31027b8) [Test dependencies] cglib:cglib-nodep:3.2.2 (1 constraints: 490ded24) -com.fasterxml.jackson.core:jackson-annotations:2.11.1 (2 constraints: bb17a070) +com.fasterxml.jackson.core:jackson-annotations:2.11.1 (3 constraints: 1f278afd) com.fasterxml.jackson.core:jackson-core:2.11.1 (1 constraints: 85123021) -com.fasterxml.jackson.core:jackson-databind:2.11.1 (1 constraints: 030e7d45) +com.fasterxml.jackson.core:jackson-databind:2.11.1 (3 constraints: c52d7961) com.github.stefanbirkner:system-rules:1.19.0 (1 constraints: 3d05443b) com.netflix.nebula:nebula-test:7.9.4 (1 constraints: 16052d36) -com.palantir.tokens:auth-tokens:3.6.2 (1 constraints: 0d050e36) +com.palantir.conjure.java:conjure-lib:5.37.1 (1 constraints: 42055f3b) +com.palantir.conjure.java.api:errors:2.16.2 (1 constraints: 23109ea9) +com.palantir.ri:resource-identifier:1.1.0 (1 constraints: ea0f6899) +com.palantir.tokens:auth-tokens:3.6.2 (2 constraints: ff14c8b7) commons-lang:commons-lang:2.6 (1 constraints: ac04232c) +jakarta.annotation:jakarta.annotation-api:1.3.5 (1 constraints: f10f7399) +jakarta.ws.rs:jakarta.ws.rs-api:2.1.6 (1 constraints: f10f7399) javax.activation:javax.activation-api:1.2.0 (1 constraints: 450a28bf) javax.xml.bind:jaxb-api:2.3.1 (1 constraints: c0069559) junit:junit-dep:4.11 (1 constraints: ba1063b3) diff --git a/versions.props b/versions.props index 837299ad8..d4ecbd8d6 100644 --- a/versions.props +++ b/versions.props @@ -21,6 +21,7 @@ com.fasterxml.jackson.*:* = 2.11.1 com.github.stefanbirkner:system-rules = 1.19.0 com.netflix.nebula:nebula-test = 7.9.4 com.palantir.tokens:auth-tokens = 3.6.2 +com.palantir.conjure.java:conjure-lib = 5.37.1 com.palantir.tritium:tritium-registry = 0.18.4 commons-lang:commons-lang = 2.6 junit:junit = 4.13.1