Skip to content

Commit

Permalink
Introduce ClassLiteralCast bug checker
Browse files Browse the repository at this point in the history
  • Loading branch information
mohamedsamehsalah committed Oct 28, 2024
1 parent 2b1dbd9 commit b1fa928
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.LambdaExpressionTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.TypeCastTree;
import com.sun.source.tree.VariableTree;
import tech.picnic.errorprone.utils.SourceCode;

/**
* A {@link BugChecker} that flags lambda expressions that can be replaced with a method reference
* of the form {@code T.class::cast}, if applicable.
*/
// XXX: Consider folding this logic into the `MethodReferenceUsage` check of the
// `error-prone-experimental` module.
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Prefer `Class::cast` method reference over equivalent lambda expression, if applicable",
link = BUG_PATTERNS_BASE_URL + "ClassCast",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class ClassCast extends BugChecker implements LambdaExpressionTreeMatcher {
private static final long serialVersionUID = 1L;

/** Instantiates a new {@link ClassCast} instance. */
public ClassCast() {}

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
if (tree.getParameters().size() != 1 || !(tree.getBody() instanceof TypeCastTree typeCast)) {
return Description.NO_MATCH;
}

String classCast = SourceCode.treeToString(typeCast.getType(), state);
if (isGenericCastExpression(classCast)) {
return Description.NO_MATCH;
}

VariableTree param = Iterables.getOnlyElement(tree.getParameters());
if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(typeCast.getExpression()))) {
return Description.NO_MATCH;
}

return describeMatch(tree, SuggestedFix.replace(tree, classCast + ".class::cast"));
}

private static boolean isGenericCastExpression(String expression) {
return expression.contains("<") && expression.contains(">");

Check warning on line 62 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassCast.java

View workflow job for this annotation

GitHub Actions / pitest

2 different changes can be made to line 62 without causing a test to fail

removed conditional - replaced equality check with true (covered by 2 tests RemoveConditionalMutator_EQUAL_IF) removed conditional - replaced equality check with true (covered by 1 tests RemoveConditionalMutator_EQUAL_IF)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,6 @@ Predicate<S> after(Class<T> clazz) {
}
}

/**
* Prefer {@link Class#cast(Object)} method references over lambda expressions that require naming
* a variable.
*/
// XXX: Once the `ClassReferenceCast` rule is dropped, rename this rule to just `ClassCast`.
static final class ClassLiteralCast<T, S> {
@BeforeTemplate
@SuppressWarnings("unchecked")
Function<T, S> before() {
return t -> (S) t;
}

@AfterTemplate
Function<T, S> after() {
return Refaster.<S>clazz()::cast;
}
}

/**
* Prefer {@link Class#cast(Object)} method references over lambda expressions that require naming
* a variable.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class ClassCastTest {
@Test
void identification() {
CompilationTestHelper.newInstance(ClassCast.class, getClass())
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"import com.google.common.collect.ImmutableSet;",
"import java.util.Optional;",
"import java.util.Set;",
"",
"class A {",
" void m() {",
" Number foo = 0;",
" Number bar = 1;",
"",
" // BUG: Diagnostic contains:",
" Optional.of(foo).map(i -> (Integer) i);",
"",
" Optional.of(foo).map(i -> 2);",
" Optional.of(foo).map(i -> (Integer) 2);",
" Optional.of(foo).map(i -> bar);",
" Optional.of(foo).map(i -> (Integer) bar);",
"",
" ImmutableList.of(Set.of(foo)).stream().map(l -> (ImmutableSet<Number>) l);",
" ImmutableList.of(Set.of(foo)).stream().map(l -> (ImmutableSet<?>) l);",
" }",
"}")
.doTest();
}

@Test
void replacement() {
BugCheckerRefactoringTestHelper.newInstance(ClassCast.class, getClass())
.addInputLines(
"A.java",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" Stream.of(1).map(i -> (Integer) i);",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" Stream.of(1).map(Integer.class::cast);",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ Predicate<String> testClassReferenceIsInstancePredicate() {
return s -> clazz.isInstance(s);
}

Function<Number, Integer> testClassLiteralCast() {
return i -> (Integer) i;
}

Function<Number, Integer> testClassReferenceCast() {
return i -> Integer.class.cast(i);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ Predicate<String> testClassReferenceIsInstancePredicate() {
return clazz::isInstance;
}

Function<Number, Integer> testClassLiteralCast() {
return Integer.class::cast;
}

Function<Number, Integer> testClassReferenceCast() {
return Integer.class::cast;
}
Expand Down

0 comments on commit b1fa928

Please sign in to comment.