From 5ab384f20d6e18ee6f694ce95caa2e07960b3257 Mon Sep 17 00:00:00 2001 From: esword Date: Mon, 8 Jan 2024 11:26:03 -0500 Subject: [PATCH] deprecated conjure check (#2693) New errorprone check that will flag usage of deprecated-for-removal conjure endpoints. --- README.md | 1 + .../ConjureEndpointDeprecatedForRemoval.java | 70 ++++++++ ...njureEndpointDeprecatedForRemovalTest.java | 151 ++++++++++++++++++ 3 files changed, 222 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemoval.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemovalTest.java diff --git a/README.md b/README.md index 48e45bb91..8d8e99d5d 100644 --- a/README.md +++ b/README.md @@ -230,6 +230,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `JooqBatchWithoutBindArgs`: jOOQ batch methods that execute without bind args can cause performance problems. - `InvocationTargetExceptionGetTargetException`: InvocationTargetException.getTargetException() predates the general-purpose exception chaining facility. The Throwable.getCause() method is now the preferred means of obtaining this information. [(source)](https://docs.oracle.com/en/java/javase/17/docs/api//java.base/java/lang/reflect/InvocationTargetException.html#getTargetException()) - `PreferInputStreamTransferTo`: Prefer JDK `InputStream.transferTo(OutputStream)` over utility methods such as `com.google.common.io.ByteStreams.copy(InputStream, OutputStream)`, `org.apache.commons.io.IOUtils.copy(InputStream, OutputStream)`, `org.apache.commons.io.IOUtils.copyLong(InputStream, OutputStream)`. +- `ConjureEndpointDeprecatedForRemoval`: Conjure endpoints marked with Deprecated and `forRemoval = true` should not be used as they are scheduled to be removed. ### Programmatic Application diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemoval.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemoval.java new file mode 100644 index 000000000..2a2331c32 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemoval.java @@ -0,0 +1,70 @@ +/* + * (c) Copyright 2024 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.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; + +@AutoService(BugChecker.class) +@BugPattern( + severity = BugPattern.SeverityLevel.ERROR, + summary = "You should not use Conjure endpoints that are marked for removal as that may block" + + " upgrades in the near future. You can explicitly disable this check on a case-by-case basis using" + + " @SuppressWarnings(\"ConjureEndpointDeprecatedForRemoval\").") +public final class ConjureEndpointDeprecatedForRemoval extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher, BugChecker.MemberReferenceTreeMatcher { + + private static final String CONJURE_CLIENT_ENDPOINT = "com.palantir.conjure.java.lib.internal.ClientEndpoint"; + + private static final Matcher FULL_DEPRECATED_CONJURE_ENDPOINT_MATCHER = + Matchers.symbolMatcher((symbol, state) -> { + return symbol.isDeprecatedForRemoval() + && ASTHelpers.hasAnnotation(symbol, CONJURE_CLIENT_ENDPOINT, state); + }); + + @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 (!FULL_DEPRECATED_CONJURE_ENDPOINT_MATCHER.matches(tree, state)) { + return Description.NO_MATCH; + } + + // Allow users to test deprecated 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/ConjureEndpointDeprecatedForRemovalTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemovalTest.java new file mode 100644 index 000000000..12c8dcfcb --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ConjureEndpointDeprecatedForRemovalTest.java @@ -0,0 +1,151 @@ +/* + * (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 java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +public class ConjureEndpointDeprecatedForRemovalTest { + + private static final String[] SERVICE_DEFINITION = new String[] { + "package com.palantir;", + "import com.palantir.conjure.java.lib.internal.ClientEndpoint;", + "public interface Service {", + "int unannotatedMethod();", + "@Deprecated(forRemoval = true)", + "@ClientEndpoint(method = \"GET\", path = \"/deprecatedForRemovalConjureEndpoint\")", + "int deprecatedForRemovalConjureEndpoint();", + "@Deprecated", + "@ClientEndpoint(method = \"GET\", path = \"/deprecatedConjureEndpoint\")", + "int deprecatedConjureEndpoint();", + "@ClientEndpoint(method = \"GET\", path = \"/nonDeprecatedConjure\")", + "int nonDeprecatedConjure();", + "@Deprecated(forRemoval = true)", + "int deprecatedForRemovalNotConjure();", + "@Deprecated", + "int deprecatedNotConjure();", + "}" + }; + + @Test + public void testDeprecatedForRemovalConjure() { + buildStandardTestHelper( + "// BUG: Diagnostic contains: You should not use Conjure endpoints that are marked for removal", + "service.deprecatedForRemovalConjureEndpoint();") + .doTest(); + } + + @Test + public void testDeprecatedForRemovalNotConjure() { + buildStandardTestHelper( + "service.deprecatedForRemovalNotConjure();", + "Supplier supp = service::deprecatedForRemovalNotConjure;") + .doTest(); + } + + @Test + public void testPlainDeprecatedConjure() { + buildStandardTestHelper( + "service.deprecatedConjureEndpoint();", + "Supplier supp = service::deprecatedConjureEndpoint;") + .doTest(); + } + + @Test + public void testPlainMethod() { + buildStandardTestHelper("service.unannotatedMethod();", "Supplier supp = service::unannotatedMethod;") + .doTest(); + } + + @Test + public void testDeprecatedForRemovalConjureReference() { + buildStandardTestHelper( + "// BUG: Diagnostic contains: You should not use Conjure endpoints that are marked for removal", + "Supplier supp = service::deprecatedForRemovalConjureEndpoint;") + .doTest(); + } + + @Test + public void testNonDeprecatedConjure() { + buildStandardTestHelper( + "int result = service.nonDeprecatedConjure();", + "Supplier supp = service::nonDeprecatedConjure;") + .doTest(); + } + + @Test + public void testSuppressedDeprecatedConjure() { + buildTestHelper(buildMain( + "public final class Main {", + "@SuppressWarnings(\"ConjureEndpointDeprecatedForRemoval\")", + "public static void main(String[] args) {", + "Service service = null;", + "service.deprecatedForRemovalConjureEndpoint();")) + .doTest(); + } + + private CompilationTestHelper buildStandardTestHelper(String... testLines) { + return buildTestHelper(buildStandardMain(testLines)); + } + + private CompilationTestHelper buildTestHelper(String... mainContents) { + return CompilationTestHelper.newInstance(ConjureEndpointDeprecatedForRemoval.class, getClass()) + // do not issue warning for the deprecated for removal annotation because it messes up other checks + .setArgs("-Xlint:-removal") + .addSourceLines("Service.java", SERVICE_DEFINITION) + .addSourceLines("Main.java", mainContents); + } + + /** + * Creates contents of the Main.java class, inserting the given test lines at the end of the main method. + */ + private static String[] buildStandardMain(String... testLines) { + List result = new ArrayList<>(); + + result.add("package com.palantir;"); + result.add("import java.util.function.Supplier;"); + result.add("public final class Main {"); + result.add("public static void main(String[] args) {"); + result.add("Service service = null;"); + for (String line : testLines) { + result.add(line); + } + result.add("}"); + result.add("}"); + return result.toArray(new String[] {}); + } + + /** + * Creates contents of the Main.java class, putting it in the com.palantir package and closing off the main method + * and class definition. + */ + private static String[] buildMain(String... testLines) { + List result = new ArrayList<>(); + + result.add("package com.palantir;"); + for (String line : testLines) { + result.add(line); + } + // closing lines + result.add("}"); + result.add("}"); + + return result.toArray(new String[] {}); + } +}