Skip to content

Commit

Permalink
Add PreventUsingIncubatingMethods error-prone check (conjure) (#1554)
Browse files Browse the repository at this point in the history
* Add `PreventUsingIncubatingMethods` error-prone check

An explicit check which warns against using incubating APIs; you can
disable it on a per-project basis, or on a per-usage basis using
@SuppressWarnings.

* Rename check to 'IncubatingMethod'

To be more in line with other checks.

* Add conjure-lib test dependency, reusable matcher

To ensure we get a compile break if @Incubating ever moves.

* Fix entry in README, add changelog

* Fix version lock & formatting
  • Loading branch information
Michael "Tres" Brenan authored Nov 20, 2020
1 parent b613a1f commit 8ed48f5
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 7 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions baseline-error-prone/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Tree> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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<Integer> 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<Integer> 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();
}
}
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1554.v2.yml
Original file line number Diff line number Diff line change
@@ -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
19 changes: 12 additions & 7 deletions versions.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8ed48f5

Please sign in to comment.