Skip to content

Commit

Permalink
deprecated conjure check (#2693)
Browse files Browse the repository at this point in the history
New errorprone check that will flag usage of deprecated-for-removal conjure endpoints.
  • Loading branch information
esword authored Jan 8, 2024
1 parent ab7376f commit 5ab384f
Show file tree
Hide file tree
Showing 3 changed files with 222 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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<Tree> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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<Integer> supp = service::deprecatedForRemovalNotConjure;")
.doTest();
}

@Test
public void testPlainDeprecatedConjure() {
buildStandardTestHelper(
"service.deprecatedConjureEndpoint();",
"Supplier<Integer> supp = service::deprecatedConjureEndpoint;")
.doTest();
}

@Test
public void testPlainMethod() {
buildStandardTestHelper("service.unannotatedMethod();", "Supplier<Integer> supp = service::unannotatedMethod;")
.doTest();
}

@Test
public void testDeprecatedForRemovalConjureReference() {
buildStandardTestHelper(
"// BUG: Diagnostic contains: You should not use Conjure endpoints that are marked for removal",
"Supplier<Integer> supp = service::deprecatedForRemovalConjureEndpoint;")
.doTest();
}

@Test
public void testNonDeprecatedConjure() {
buildStandardTestHelper(
"int result = service.nonDeprecatedConjure();",
"Supplier<Integer> 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<String> 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<String> 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[] {});
}
}

0 comments on commit 5ab384f

Please sign in to comment.