-
Notifications
You must be signed in to change notification settings - Fork 134
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Migrate WebApplicationExceptions to ServiceException
- Loading branch information
1 parent
493ed42
commit 54c7ee6
Showing
4 changed files
with
254 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
145 changes: 145 additions & 0 deletions
145
...e-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferConjureExceptions.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
/* | ||
* (c) Copyright 2019 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.common.collect.ImmutableMap; | ||
import com.google.common.collect.Streams; | ||
import com.google.errorprone.BugPattern; | ||
import com.google.errorprone.VisitorState; | ||
import com.google.errorprone.bugpatterns.BugChecker; | ||
import com.google.errorprone.fixes.SuggestedFix; | ||
import com.google.errorprone.fixes.SuggestedFixes; | ||
import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher; | ||
import com.google.errorprone.matchers.Description; | ||
import com.google.errorprone.matchers.Matcher; | ||
import com.google.errorprone.util.ASTHelpers; | ||
import com.sun.source.tree.ExpressionTree; | ||
import com.sun.source.tree.NewClassTree; | ||
import com.sun.tools.javac.code.Type; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
@AutoService(BugChecker.class) | ||
@BugPattern( | ||
name = "PreferConjureExceptions", | ||
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", | ||
linkType = BugPattern.LinkType.CUSTOM, | ||
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION, | ||
severity = BugPattern.SeverityLevel.SUGGESTION, | ||
summary = "Prefer throwing a ServiceException instead of jax-rs WebApplicationException subtypes.") | ||
public final class PreferConjureExceptions extends BugChecker implements BugChecker.NewClassTreeMatcher { | ||
|
||
private static final String JAXRS_PACKAGE = "javax.ws.rs"; | ||
private static final String WEB_APP_EXCEPTION_NAME = JAXRS_PACKAGE + ".WebApplicationException"; | ||
private static final String CONJURE_ERROR_PACKAGE = "com.palantir.conjure.java.api.errors"; | ||
private static final String CONJURE_ERROR_TYPE = CONJURE_ERROR_PACKAGE + ".ErrorType"; | ||
private static final String CONJURE_SERVICE_EXCEPTION = CONJURE_ERROR_PACKAGE + ".ServiceException"; | ||
|
||
private static final ImmutableMap<String, String> EXCEPTION_TO_ERROR_TYPE = ImmutableMap.<String, String>builder() | ||
// This should include the 401 UNAUTHENTICATED type once | ||
// https://github.com/palantir/conjure/pull/367 is released. | ||
.put("BadRequestException", "INVALID_ARGUMENT") | ||
.put("ForbiddenException", "PERMISSION_DENIED") | ||
.put("InternalServerErrorException", "INTERNAL") | ||
.put("NotFoundException", "NOT_FOUND") | ||
.build(); | ||
|
||
private static final Matcher<ExpressionTree> compileTimeConstExpressionMatcher = | ||
new CompileTimeConstantExpressionMatcher(); | ||
|
||
@Override | ||
@SuppressWarnings("CyclomaticComplexity") | ||
public Description matchNewClass(NewClassTree tree, VisitorState state) { | ||
Type type = ASTHelpers.getResultType(tree); | ||
if (type == null) { | ||
return Description.NO_MATCH; | ||
} | ||
if (!ASTHelpers.isCastable(type, state.getTypeFromString(WEB_APP_EXCEPTION_NAME), state)) { | ||
return Description.NO_MATCH; | ||
} | ||
// Conjure ServiceException cannot be subclassed. | ||
if (tree.getClassBody() != null) { | ||
return describeMatch(tree); | ||
} | ||
// Conjure ServiceException does not support type arguments. | ||
if (!tree.getTypeArguments().isEmpty()) { | ||
return describeMatch(tree); | ||
} | ||
Optional<String> maybeErrorType = replacementErrorType(type, state); | ||
if (!maybeErrorType.isPresent()) { | ||
return describeMatch(tree); | ||
} | ||
String errorType = maybeErrorType.get(); | ||
List<? extends ExpressionTree> arguments = tree.getArguments(); | ||
SuggestedFix.Builder fix = SuggestedFix.builder(); | ||
String qualifiedErrorType = SuggestedFixes.qualifyType(state, fix, CONJURE_ERROR_TYPE); | ||
String qualifiedServiceException = SuggestedFixes.qualifyType(state, fix, CONJURE_SERVICE_EXCEPTION); | ||
String errorArgument = qualifiedErrorType + '.' + errorType; | ||
// Supports no-arg and single-arg (cause) WebApplicationExceptions. | ||
if (arguments.isEmpty() || (arguments.size() == 1 && isCastable(arguments.get(0), Throwable.class, state))) { | ||
String replacementArguments = Streams.concat( | ||
Stream.of(errorArgument), | ||
tree.getArguments().stream().map(state::getSourceForNode)) | ||
.collect(Collectors.joining(", ")); | ||
fix.replace(tree, "new " + qualifiedServiceException + '(' + replacementArguments + ')'); | ||
return buildDescription(tree) | ||
.addFix(fix.build()) | ||
.build(); | ||
} | ||
// Exception includes a message string. In this case we provide an intermediate exception to escort the | ||
// message to the logger without serializing it to to clients, providing parity with the replaced code. | ||
if ((arguments.size() == 1 && isCastable(arguments.get(0), String.class, state)) | ||
|| (arguments.size() == 2 && isCastable(arguments.get(0), String.class, state) | ||
&& isCastable(arguments.get(1), Throwable.class, state))) { | ||
ExpressionTree firstArgument = arguments.get(0); | ||
String qualifiedCauseName = SuggestedFixes.qualifyType(state, fix, | ||
compileTimeConstExpressionMatcher.matches(firstArgument, state) | ||
? "com.palantir.logsafe.exceptions.SafeRuntimeException" | ||
: RuntimeException.class.getName()); | ||
fix | ||
.replace(tree.getIdentifier(), qualifiedServiceException) | ||
.replace(firstArgument, errorArgument + ", new " + qualifiedCauseName + '(' | ||
+ state.getSourceForNode(firstArgument) + (arguments.size() == 1 ? ")" : "")); | ||
if (arguments.size() == 2) { | ||
fix.replace(arguments.get(1), state.getSourceForNode(arguments.get(1)) + ')'); | ||
} | ||
return buildDescription(tree) | ||
.addFix(fix.build()) | ||
.build(); | ||
} | ||
return describeMatch(tree); | ||
} | ||
|
||
private static boolean isCastable(ExpressionTree argument, Class<?> expected, VisitorState state) { | ||
return ASTHelpers.isCastable( | ||
ASTHelpers.getResultType(argument), | ||
state.getTypeFromString(expected.getName()), | ||
state); | ||
} | ||
|
||
private static Optional<String> replacementErrorType(Type type, VisitorState state) { | ||
return EXCEPTION_TO_ERROR_TYPE.entrySet().stream() | ||
.filter(entry -> ASTHelpers.isSameType( | ||
type, state.getTypeFromString(JAXRS_PACKAGE + '.' + entry.getKey()), state)) | ||
.map(Map.Entry::getValue) | ||
.findFirst(); | ||
} | ||
} |
104 changes: 104 additions & 0 deletions
104
...ror-prone/src/test/java/com/palantir/baseline/errorprone/PreferConjureExceptionsTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
/* | ||
* (c) Copyright 2019 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.BugCheckerRefactoringTestHelper; | ||
import com.google.errorprone.CompilationTestHelper; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class PreferConjureExceptionsTest { | ||
@Test | ||
void testNotAuthorizedException() { | ||
helper().addSourceLines( | ||
"Test.java", | ||
"import javax.ws.rs.NotSupportedException;", | ||
"class Test {", | ||
" void f() {", | ||
" // BUG: Diagnostic contains: Prefer throwing a ServiceException", | ||
" throw new NotSupportedException();", | ||
" }", | ||
"}" | ||
).doTest(); | ||
} | ||
|
||
@Test | ||
void testFix() { | ||
fix() | ||
.addInputLines( | ||
"Test.java", | ||
"import javax.ws.rs.*;", | ||
"class Test {", | ||
" void f1() {", | ||
" throw new BadRequestException();", | ||
" }", | ||
" void f2(Throwable cause) {", | ||
" throw new BadRequestException(cause);", | ||
" }", | ||
" void f3() {", | ||
" throw new BadRequestException(\"message\");", | ||
" }", | ||
" void f4(String message) {", | ||
" throw new InternalServerErrorException(message);", | ||
" }", | ||
" void f5(Throwable cause) {", | ||
" throw new ForbiddenException(\"message\", cause);", | ||
" }", | ||
" void f6(String message, Throwable cause) {", | ||
" throw new NotFoundException(message, cause);", | ||
" }", | ||
"}") | ||
.addOutputLines( | ||
"Test.java", | ||
"import com.palantir.conjure.java.api.errors.ErrorType;", | ||
"import com.palantir.conjure.java.api.errors.ServiceException;", | ||
"import com.palantir.logsafe.exceptions.SafeRuntimeException;", | ||
"import javax.ws.rs.*;", | ||
"class Test {", | ||
" void f1() {", | ||
" throw new ServiceException(ErrorType.INVALID_ARGUMENT);", | ||
" }", | ||
" void f2(Throwable cause) {", | ||
" throw new ServiceException(ErrorType.INVALID_ARGUMENT, cause);", | ||
" }", | ||
" void f3() {", | ||
" throw new ServiceException(ErrorType.INVALID_ARGUMENT,", | ||
" new SafeRuntimeException(\"message\"));", | ||
" }", | ||
" void f4(String message) {", | ||
" throw new ServiceException(ErrorType.INTERNAL,", | ||
" new RuntimeException(message));", | ||
" }", | ||
" void f5(Throwable cause) {", | ||
" throw new ServiceException(ErrorType.PERMISSION_DENIED,", | ||
" new SafeRuntimeException(\"message\", cause));", | ||
" }", | ||
" void f6(String message, Throwable cause) {", | ||
" throw new ServiceException(ErrorType.NOT_FOUND,", | ||
" new RuntimeException(message, cause));", | ||
" }", | ||
"}") | ||
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); | ||
} | ||
|
||
private CompilationTestHelper helper() { | ||
return CompilationTestHelper.newInstance(PreferConjureExceptions.class, getClass()); | ||
} | ||
|
||
private BugCheckerRefactoringTestHelper fix() { | ||
return BugCheckerRefactoringTestHelper.newInstance(new PreferConjureExceptions(), getClass()); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters