Provide possible assignment solutions

This commit is contained in:
Stephan Schroevers
2023-11-09 06:40:22 +01:00
committed by Rick Ossendrijver
parent 8b6293c21f
commit c3de7e9eb9
18 changed files with 137 additions and 65 deletions

View File

@@ -8,7 +8,9 @@ import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodTree;
/** A {@link BugChecker} that flags empty methods that seemingly can simply be deleted. */
@@ -25,9 +27,12 @@ public final class Assignment0DeleteEmptyMethod extends BugChecker implements Me
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
// XXX: Part 1: Ensure that we only delete methods that contain no statements.
// XXX: Part 2: Don't delete methods that are annotated with `@Override`.
if (tree.getBody() == null
|| !tree.getBody().getStatements().isEmpty()
|| ASTHelpers.hasAnnotation(tree, Override.class.getName(), state)) {
return Description.NO_MATCH;
}
return Description.NO_MATCH;
return describeMatch(tree, SuggestedFix.delete(tree));
}
}

View File

@@ -13,6 +13,7 @@ import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.MethodInvocationTree;
@@ -49,11 +50,8 @@ public final class Assignment1AssertJIsNullMethod extends BugChecker
return Description.NO_MATCH;
}
SuggestedFix.Builder fix = SuggestedFix.builder();
// XXX: Using `fix.merge(<some code>);` make sure we rename the method invocation to `isNull`.
// See the `SuggestedFixes` class ;).
SuggestedFix.Builder fix =
SuggestedFix.builder().merge(SuggestedFixes.renameMethodInvocation(tree, "isNull", state));
tree.getArguments().forEach(arg -> fix.merge(SuggestedFix.delete(arg)));
return describeMatch(tree, fix.build());

View File

@@ -14,6 +14,7 @@ import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import java.util.List;
import tech.picnic.errorprone.utils.SourceCode;
/**
@@ -37,9 +38,13 @@ public final class Assignment2DropMockitoEq extends BugChecker
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
// XXX: Make sure to return `Description.NO_MATCH` if the `tree` doesn't have arguments, or if
// the `isEqInvocation` method below returns `false` for at least one of the arguments.
List<? extends ExpressionTree> arguments = tree.getArguments();
if (arguments.isEmpty() || !arguments.stream().allMatch(arg -> isEqInvocation(arg, state))) {
return Description.NO_MATCH;
}
SuggestedFix.Builder suggestedFix = SuggestedFix.builder();
for (ExpressionTree arg : tree.getArguments()) {
for (ExpressionTree arg : arguments) {
suggestedFix.replace(
arg,
SourceCode.treeToString(
@@ -49,7 +54,6 @@ public final class Assignment2DropMockitoEq extends BugChecker
return describeMatch(tree, suggestedFix.build());
}
@SuppressWarnings("UnusedMethod" /* Recommended to use when implementing this assignment. */)
private static boolean isEqInvocation(ExpressionTree tree, VisitorState state) {
return tree instanceof MethodInvocationTree && MOCKITO_EQ_METHOD.matches(tree, state);
}

View File

@@ -14,11 +14,11 @@ import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.matchers.MultiMatcher.MultiMatchResult;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.ArrayList;
import java.util.List;
import tech.picnic.errorprone.utils.SourceCode;
@@ -40,7 +40,10 @@ public final class Assignment3DropAutowiredConstructorAnnotation extends BugChec
public Description matchClass(ClassTree tree, VisitorState state) {
// XXX: Using the `ASTHelpers#getConstructors` method, return `Description.NO_MATCH` if we do
// not have exactly 1 constructor (start by dropping `new ArrayList<>()` on the next line).
List<MethodTree> constructors = new ArrayList<>();
List<MethodTree> constructors = ASTHelpers.getConstructors(tree);
if (constructors.size() != 1) {
return Description.NO_MATCH;
}
MultiMatchResult<AnnotationTree> hasAutowiredAnnotation =
AUTOWIRED_ANNOTATION.multiMatchResult(Iterables.getOnlyElement(constructors), state);

View File

@@ -15,6 +15,7 @@ import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.MultiMatcher;
import com.sun.source.tree.AnnotationTree;
@@ -44,13 +45,17 @@ public final class Assignment4JUnitTestMethodDeclaration extends BugChecker
@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
// XXX: Part 1: Return `Description.NO_MATCH` if the method is not a `TEST_METHOD`.
if (!TEST_METHOD.matches(tree, state)) {
return Description.NO_MATCH;
}
SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
// XXX: Part 2: Make sure that JUnit test methods don't use any of the modifiers from the
// `ILLEGAL_MODIFIERS` field, by using `SuggestedFixes#removeModifiers` and
// `SuggestedFix.Builder#merge`.
SuggestedFixes.removeModifiers(tree.getModifiers(), state, ILLEGAL_MODIFIERS)
.ifPresent(fixBuilder::merge);
if (fixBuilder.isEmpty()) {
return Description.NO_MATCH;

View File

@@ -13,6 +13,7 @@ import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.bugpatterns.TypesWithUndefinedEquality;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
@@ -24,6 +25,7 @@ import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import java.util.Arrays;
import java.util.List;
import tech.picnic.errorprone.utils.SourceCode;
/** A {@link BugChecker} that flags redundant identity conversions. */
@BugPattern(
@@ -66,9 +68,7 @@ public final class Assignment5DeleteIdentityConversion extends BugChecker
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
List<? extends ExpressionTree> arguments = tree.getArguments();
// XXX: Make sure we skip invocations that do not pass exactly one argument, by using the
// `tree`.
if (!IS_CONVERSION_METHOD.matches(tree, state)) {
if (arguments.size() != 1 || !IS_CONVERSION_METHOD.matches(tree, state)) {
return Description.NO_MATCH;
}
@@ -91,9 +91,7 @@ public final class Assignment5DeleteIdentityConversion extends BugChecker
}
return buildDescription(tree)
// XXX: Use the `.addFix()` to suggest replacing the original `tree` with the `sourceTree`.
// Tip: You can get the actual String representation of a Tree by using the
// `SourceCode#treeToString`.
.addFix(SuggestedFix.replace(tree, SourceCode.treeToString(sourceTree, state)))
.build();
}

View File

@@ -1,9 +1,22 @@
package tech.picnic.errorprone.workshop.refasterrules;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
/** Refaster rule used as example for the assignments of the workshop. */
final class WorkshopAssignment0Rules {
private WorkshopAssignment0Rules() {}
/** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */
static final class ExampleStringIsEmpty {}
static final class ExampleStringIsEmpty {
@BeforeTemplate
boolean before(String s) {
return s.length() == 0;
}
@AfterTemplate
boolean after(String s) {
return s.isEmpty();
}
}
}

View File

@@ -1,11 +1,22 @@
package tech.picnic.errorprone.workshop.refasterrules;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
/** Refaster rules for the first assignment of the workshop. */
final class WorkshopAssignment1Rules {
private WorkshopAssignment1Rules() {}
/** Prefer {@link String#String(char[])} over {@link String#copyValueOf(char[])}. */
static final class NewStringCharArray {
// XXX: Implement this Refaster rule.
@BeforeTemplate
String before(char[] chars) {
return String.copyValueOf(chars);
}
@AfterTemplate
String after(char[] chars) {
return new String(chars);
}
}
}

View File

@@ -1,9 +1,13 @@
package tech.picnic.errorprone.workshop.refasterrules;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Collections;
import java.util.List;
/** Refaster rules for the second assignment of the workshop. */
@SuppressWarnings("UnusedTypeParameter" /* Ignore this for demo purposes. */)
final class WorkshopAssignment2Rules {
private WorkshopAssignment2Rules() {}
@@ -12,7 +16,14 @@ final class WorkshopAssignment2Rules {
* immutability of the resulting list at the type level.
*/
static final class ImmutableListOfOne<T> {
// XXX: Implement this Refaster rule, preferably by using only one `@BeforeTemplate`.
// Tip: use the type argument.
@BeforeTemplate
List<T> before(T element) {
return Refaster.anyOf(Collections.singletonList(element), List.of(element));
}
@AfterTemplate
ImmutableList<T> after(T element) {
return ImmutableList.of(element);
}
}
}

View File

@@ -1,21 +1,46 @@
package tech.picnic.errorprone.workshop.refasterrules;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import com.google.common.base.Preconditions;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
/** Refaster rules for the third assignment of the workshop. */
@SuppressWarnings("UnusedTypeParameter" /* Ignore this for demo purposes. */)
final class WorkshopAssignment3Rules {
private WorkshopAssignment3Rules() {}
// XXX: Tip: check the input and output files to see the *expected* refactoring.
/** Prefer {@link Preconditions#checkArgument(boolean)} over if statements. */
static final class CheckArgumentWithoutMessage {
// XXX: Implement the Refaster rule to get the test green.
@BeforeTemplate
void before(boolean expression) {
if (expression) {
throw new IllegalArgumentException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean expression) {
checkArgument(!expression);
}
}
/** Prefer {@link Preconditions#checkArgument(boolean, Object)} over if statements. */
static final class CheckArgumentWithMessage {
// XXX: Implement the Refaster rule to get the test green.
@BeforeTemplate
void before(boolean expression, String message) {
if (expression) {
throw new IllegalArgumentException(message);
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(boolean expression, String message) {
checkArgument(!expression, message);
}
}
}

View File

@@ -1,25 +1,26 @@
package tech.picnic.errorprone.workshop.refasterrules;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Objects;
/** Refaster rules for the fourth assignment of the workshop. */
@SuppressWarnings("java:S1698" /* Reference comparison is valid for enums. */)
final class WorkshopAssignment4Rules {
private WorkshopAssignment4Rules() {}
// The test fails because non Enum comparisons are also rewritten.
// Fix the test by tweaking the type parameters.
static final class PrimitiveOrReferenceEqualityEnum<T extends Enum<T>> {
@BeforeTemplate
boolean before(T a, T b) {
return Refaster.anyOf(a.equals(b), Objects.equals(a, b));
}
// XXX: Get the test to pass by improving the Refaster rule (uncommented it first).
// static final class PrimitiveOrReferenceEqualityEnum<T> {
// @BeforeTemplate
// boolean before(T a, T b) {
// return Refaster.anyOf(a.equals(b), Objects.equals(a, b));
// }
//
// @AfterTemplate
// @AlsoNegation
// boolean after(T a, T b) {
// return a == b;
// }
// }
@AfterTemplate
@AlsoNegation
boolean after(T a, T b) {
return a == b;
}
}
}

View File

@@ -1,12 +1,27 @@
package tech.picnic.errorprone.workshop.refasterrules;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.Placeholder;
import java.util.stream.Stream;
/** Refaster rules for the fifth assignment of the workshop. */
@SuppressWarnings("UnusedTypeParameter" /* Ignore this for demo purposes. */)
final class WorkshopAssignment5Rules {
private WorkshopAssignment5Rules() {}
abstract static class StreamDoAllMatch<T> {
// XXX: Implement the Refaster rule to get the test green.
// Tip: use the `@Placeholder` annotation.
@Placeholder
abstract boolean test(@MayOptionallyUse T element);
@BeforeTemplate
boolean before(Stream<T> stream) {
return stream.noneMatch(v -> !test(v));
}
@AfterTemplate
boolean after(Stream<T> stream) {
return stream.allMatch(v -> test(v));
}
}
}

View File

@@ -3,10 +3,8 @@ package tech.picnic.errorprone.workshop.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@Disabled("Enable this when implementing the BugChecker.")
final class Assignment1AssertJIsNullMethodTest {
@Test
void identification() {

View File

@@ -3,10 +3,8 @@ package tech.picnic.errorprone.workshop.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@Disabled("Enable this when implementing the BugChecker.")
final class Assignment2DropMockitoEqTest {
@Test
void identification() {

View File

@@ -3,10 +3,8 @@ package tech.picnic.errorprone.workshop.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@Disabled("Enable this when implementing the BugChecker.")
final class Assignment3DropAutowiredConstructorAnnotationTest {
@Test
void identification() {

View File

@@ -3,10 +3,8 @@ package tech.picnic.errorprone.workshop.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@Disabled("Enable this to validate part 1.")
final class Assignment4JUnitTestMethodDeclarationTest {
@Test
void identificationIllegalModifiers() {

View File

@@ -3,10 +3,8 @@ package tech.picnic.errorprone.workshop.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@Disabled("Enable this when implementing the BugChecker.")
final class Assignment5DeleteIdentityConversionTest {
@Test
void identification() {

View File

@@ -1,41 +1,34 @@
package tech.picnic.errorprone.workshop.refasterrules;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollection;
final class WorkshopRefasterRulesTest {
@Disabled("Needs to be implemented in `WorkshopAssignment0Rules.java`.")
@Test
void validateExampleRuleCollection() {
RefasterRuleCollection.validate(WorkshopAssignment0Rules.class);
}
@Disabled("Needs to be implemented in `WorkshopAssignment1Rules.java`.")
@Test
void validateFirstWorkshopAssignment() {
RefasterRuleCollection.validate(WorkshopAssignment1Rules.class);
}
@Disabled("Needs to be implemented in `WorkshopAssignment2Rules.java`.")
@Test
void validateSecondWorkshopAssignment() {
RefasterRuleCollection.validate(WorkshopAssignment2Rules.class);
}
@Disabled("Needs to be implemented in `WorkshopAssignment3Rules.java`.")
@Test
void validateThirdWorkshopAssignment() {
RefasterRuleCollection.validate(WorkshopAssignment3Rules.class);
}
@Disabled("Needs to be implemented in `WorkshopAssignment4Rules.java`.")
@Test
void validateFourthWorkshopAssignment() {
RefasterRuleCollection.validate(WorkshopAssignment4Rules.class);
}
@Disabled("Needs to be implemented in `WorkshopAssignment5Rules.java`.")
@Test
void validateFifthWorkshopAssignment() {
RefasterRuleCollection.validate(WorkshopAssignment5Rules.class);