Skip to content

Commit

Permalink
Add refactorings to unconditionally migrate various exception testing…
Browse files Browse the repository at this point in the history
… idioms to assertThrows

RELNOTES: Add refactorings to unconditionally migrate various exception
testing idioms to assertThrows

MOE_MIGRATED_REVID=207625165
  • Loading branch information
cushon committed Aug 6, 2018
1 parent 8ad5a3c commit 42a9a65
Show file tree
Hide file tree
Showing 7 changed files with 607 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright 2018 The Error Prone Authors.
*
* 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.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.Category.JUNIT;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.ASTHelpers.isSameType;

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import java.util.List;
import javax.annotation.Nullable;

/** @author cushon@google.com (Liam Miller-Cushon) */
@BugPattern(
name = "ExpectedExceptionRefactoring",
category = JUNIT,
summary = "Prefer assertThrows to ExpectedException",
severity = SUGGESTION,
tags = REFACTORING,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION
)
public class ExpectedExceptionRefactoring extends AbstractExpectedExceptionChecker
implements VariableTreeMatcher {
@Override
protected Description handleMatch(
MethodTree tree,
VisitorState state,
List<Tree> expectations,
List<StatementTree> suffix,
@Nullable StatementTree failure) {
return describeMatch(tree, buildBaseFix(state, expectations, failure).build(suffix));
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
if (!hasAnnotation(getSymbol(tree), "org.junit.Rule", state)) {
return NO_MATCH;
}
if (!isSameType(
getType(tree), state.getTypeFromString("org.junit.rules.ExpectedException"), state)) {
return NO_MATCH;
}
return describeMatch(tree, SuggestedFix.delete(tree));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2018 The Error Prone Authors.
*
* 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.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.Category.JUNIT;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.tree.JCTree.JCExpression;

/** @author cushon@google.com (Liam Miller-Cushon) */
@BugPattern(
name = "TestExceptionRefactoring",
category = JUNIT,
summary = "Prefer assertThrows to @Test(expected=...)",
severity = SUGGESTION,
tags = REFACTORING,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
public class TestExceptionRefactoring extends AbstractTestExceptionChecker {
@Override
protected Description handleStatements(
MethodTree tree, VisitorState state, JCExpression expectedException, SuggestedFix baseFix) {
return describeMatch(
tree,
buildFix(
state,
SuggestedFix.builder().merge(baseFix),
expectedException,
tree.getBody().getStatements()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Copyright 2018 The Error Prone Authors.
*
* 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.google.errorprone.bugpatterns;

import static com.google.common.collect.Iterables.getLast;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.errorprone.BugPattern.Category.JUNIT;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.REFACTORING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.expressionStatement;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static com.sun.source.tree.Tree.Kind.UNION_TYPE;

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.TryTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.CatchTree;
import com.sun.source.tree.ExpressionStatementTree;
import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.TryTree;
import com.sun.tools.javac.tree.JCTree;
import java.util.List;

/** @author cushon@google.com (Liam Miller-Cushon) */
@BugPattern(
name = "TryFailRefactoring",
category = JUNIT,
summary = "Prefer assertThrows to try/fail",
severity = SUGGESTION,
tags = REFACTORING,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION
)
public class TryFailRefactoring extends BugChecker implements TryTreeMatcher {

private static final Matcher<StatementTree> FAIL_METHOD =
expressionStatement(staticMethod().anyClass().named("fail"));

@Override
public Description matchTry(TryTree tree, VisitorState state) {
List<? extends StatementTree> body = tree.getBlock().getStatements();
if (body.isEmpty() || tree.getFinallyBlock() != null || tree.getCatches().size() != 1) {
// TODO(cushon): support finally
// TODO(cushon): support multiple catch blocks
return NO_MATCH;
}
CatchTree catchTree = getOnlyElement(tree.getCatches());
if (catchTree.getParameter().getType().getKind() == UNION_TYPE) {
// TODO(cushon): handle multi-catch
return NO_MATCH;
}
if (!FAIL_METHOD.matches(getLast(body), state)) {
return NO_MATCH;
}
SuggestedFix.Builder fix = SuggestedFix.builder();
StringBuilder fixPrefix = new StringBuilder();
// try body statements, excluding the trailing `fail()`
List<? extends StatementTree> throwingStatements =
tree.getBlock().getStatements().subList(0, tree.getBlock().getStatements().size() - 1);
if (throwingStatements.isEmpty()) {
return NO_MATCH;
}
List<? extends StatementTree> catchStatements = catchTree.getBlock().getStatements();
fix.addStaticImport("org.junit.Assert.assertThrows");
if (!catchStatements.isEmpty()) {
// TODO(cushon): pick a fresh name for the variable, if necessary
fixPrefix.append(String.format("%s = ", state.getSourceForNode(catchTree.getParameter())));
}
fixPrefix.append(
String.format(
"assertThrows(%s.class, () -> ",
state.getSourceForNode(catchTree.getParameter().getType())));
boolean useExpressionLambda = throwingStatements.size() == 1
&& getOnlyElement(throwingStatements).getKind() == Kind.EXPRESSION_STATEMENT;
if (!useExpressionLambda) {
fixPrefix.append("{");
}
fix.replace(
((JCTree) tree).getStartPosition(),
((JCTree) throwingStatements.iterator().next()).getStartPosition(),
fixPrefix.toString());
if (useExpressionLambda) {
fix.postfixWith(
((ExpressionStatementTree) throwingStatements.iterator().next()).getExpression(), ")");
} else {
fix.postfixWith(getLast(throwingStatements), "});");
}
if (catchStatements.isEmpty()) {
fix.replace(
state.getEndPosition(getLast(throwingStatements)), state.getEndPosition(tree), "");
} else {
fix.replace(
/* startPos= */ state.getEndPosition(getLast(throwingStatements)),
/* endPos= */ ((JCTree) catchStatements.get(0)).getStartPosition(),
"\n");
fix.replace(state.getEndPosition(getLast(catchStatements)), state.getEndPosition(tree), "");
}
return describeMatch(tree, fix.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
import com.google.errorprone.bugpatterns.EqualsUnsafeCast;
import com.google.errorprone.bugpatterns.EqualsWrongThing;
import com.google.errorprone.bugpatterns.ExpectedExceptionChecker;
import com.google.errorprone.bugpatterns.ExpectedExceptionRefactoring;
import com.google.errorprone.bugpatterns.ExtendingJUnitAssert;
import com.google.errorprone.bugpatterns.FallThrough;
import com.google.errorprone.bugpatterns.FieldCanBeFinal;
Expand Down Expand Up @@ -227,6 +228,7 @@
import com.google.errorprone.bugpatterns.SwitchDefault;
import com.google.errorprone.bugpatterns.SystemExitOutsideMain;
import com.google.errorprone.bugpatterns.TestExceptionChecker;
import com.google.errorprone.bugpatterns.TestExceptionRefactoring;
import com.google.errorprone.bugpatterns.ThreadJoinLoop;
import com.google.errorprone.bugpatterns.ThreadLocalUsage;
import com.google.errorprone.bugpatterns.ThreeLetterTimeZoneID;
Expand All @@ -237,6 +239,7 @@
import com.google.errorprone.bugpatterns.TruthAssertExpected;
import com.google.errorprone.bugpatterns.TruthConstantAsserts;
import com.google.errorprone.bugpatterns.TruthSelfEquals;
import com.google.errorprone.bugpatterns.TryFailRefactoring;
import com.google.errorprone.bugpatterns.TryFailThrowable;
import com.google.errorprone.bugpatterns.TypeNameShadowing;
import com.google.errorprone.bugpatterns.TypeParameterNaming;
Expand Down Expand Up @@ -618,18 +621,18 @@ public static ScannerSupplier errorChecks() {
public static final ImmutableSet<BugCheckerInfo> DISABLED_CHECKS =
getSuppliers(
AndroidJdkLibsChecker.class,
AutoFactoryAtInject.class,
AnnotateFormatMethod.class,
AssertFalse.class,
AssistedInjectAndInjectOnConstructors.class,
AssistedInjectAndInjectOnSameConstructor.class,
AutoFactoryAtInject.class,
BinderIdentityRestoredDangerously.class, // TODO: enable this by default.
BindingToUnqualifiedCommonType.class,
BooleanParameter.class,
ClassName.class,
ClassNamedLikeTypeParameter.class,
ComparisonContractViolated.class,
ConstantField.class,
BooleanParameter.class,
ConstructorInvokesOverridable.class,
ConstructorLeaksThis.class,
DepAnn.class,
Expand All @@ -639,8 +642,9 @@ public static ScannerSupplier errorChecks() {
EmptyTopLevelDeclaration.class,
EqualsBrokenForNull.class,
ExpectedExceptionChecker.class,
FieldMissingNullable.class,
ExpectedExceptionRefactoring.class,
FieldCanBeFinal.class,
FieldMissingNullable.class,
FunctionalInterfaceClash.class,
FuzzyEqualsShouldNotBeUsedInEqualsMethod.class,
HardCodedSdCardPath.class,
Expand All @@ -651,8 +655,8 @@ public static ScannerSupplier errorChecks() {
InvalidTargetingOnScopingAnnotation.class,
IterablePathParameter.class,
JMockTestWithoutRunWithOrRuleAnnotation.class,
JavaxInjectOnFinalField.class,
Java7ApiChecker.class,
JavaxInjectOnFinalField.class,
LambdaFunctionalInterface.class,
LockMethodChecker.class,
LongLiteralLowerCaseSuffix.class,
Expand All @@ -661,10 +665,10 @@ public static ScannerSupplier errorChecks() {
MixedArrayDimensions.class,
ModifiedButNotUsed.class,
MoreThanOneQualifier.class,
MutableMethodReturnType.class,
MultiVariableDeclaration.class,
MultipleTopLevelClasses.class,
MultipleUnaryOperatorsInMethodCall.class,
MutableMethodReturnType.class,
NoAllocationChecker.class,
NoFunctionalReturnType.class,
NonCanonicalStaticMemberImport.class,
Expand All @@ -673,8 +677,8 @@ public static ScannerSupplier errorChecks() {
ParameterComment.class,
ParameterNotNullable.class,
PrimitiveArrayPassedToVarargsMethod.class,
PrivateConstructorForUtilityClass.class,
PrivateConstructorForNoninstantiableModule.class,
PrivateConstructorForUtilityClass.class,
ProvidesFixChecker.class,
QualifierWithTypeUse.class,
RedundantThrows.class,
Expand All @@ -684,13 +688,15 @@ public static ScannerSupplier errorChecks() {
ScopeAnnotationOnInterfaceOrAbstractClass.class,
ScopeOnModule.class,
ScopeOrQualifierAnnotationRetention.class,
StaticQualifiedUsingExpression.class,
StaticOrDefaultInterfaceMethod.class,
StaticQualifiedUsingExpression.class,
StringEquality.class,
SwitchDefault.class,
SystemExitOutsideMain.class,
TestExceptionChecker.class,
TestExceptionRefactoring.class,
ThrowsUncheckedException.class,
TryFailRefactoring.class,
TypeParameterNaming.class,
UngroupedOverloads.class,
UnlockMethodChecker.class,
Expand Down

0 comments on commit 42a9a65

Please sign in to comment.