Skip to content

Commit

Permalink
Tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202 @rickie
Stephan202 authored and rickie committed Dec 11, 2024
1 parent 9135d25 commit d6e65b6
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 24 deletions.
46 changes: 23 additions & 23 deletions ...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumeration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.Visibility;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.Element;
import tech.picnic.errorprone.utils.SourceCode;

/**
Expand Down Expand Up @@ -91,6 +91,8 @@ public final class ExplicitArgumentEnumeration extends BugChecker
OBJECT_ENUMERABLE_ASSERT,
"containsExactlyInAnyOrderElementsOf",
"containsExactlyInAnyOrder")
.put(OBJECT_ENUMERABLE_ASSERT, "containsOnlyElementsOf", "containsOnly")
.put(OBJECT_ENUMERABLE_ASSERT, "containsOnlyOnceElementsOf", "containsOnlyOnce")
.put(OBJECT_ENUMERABLE_ASSERT, "doesNotContainAnyElementsOf", "doesNotContain")
.put(OBJECT_ENUMERABLE_ASSERT, "hasSameElementsAs", "containsOnly")
.put(STEP_VERIFIER_STEP, "expectNextSequence", "expectNext")
Expand All @@ -102,6 +104,7 @@ public ExplicitArgumentEnumeration() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (tree.getArguments().size() != 1) {

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

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 106 without causing a test to fail 

removed conditional - replaced equality check with false (covered by 2 tests RemoveConditionalMutator_EQUAL_ELSE)
/* Performance optimization: non-unary method invocations cannot be simplified. */
return Description.NO_MATCH;
}

Expand All @@ -111,8 +114,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
}

ExpressionTree argument = tree.getArguments().get(0);
if (!(argument instanceof MethodInvocationTree)
|| !EXPLICIT_ITERABLE_CREATOR.matches(argument, state)) {
if (!EXPLICIT_ITERABLE_CREATOR.matches(argument, state)) {
return Description.NO_MATCH;
}

Expand All @@ -131,10 +133,14 @@ private static boolean isUnaryIterableAcceptingMethod(MethodSymbol method, Visit

private static Optional<SuggestedFix> trySuggestCallingVarargsOverload(
MethodSymbol method, MethodInvocationTree argument, VisitorState state) {
/*
* Collect all overloads of the given method that we are sure to be able to call. Note that the
* `isAtLeastAsVisible` check is conservative heuristic.
*/
ImmutableList<MethodSymbol> overloads =
ASTHelpers.matchingMethods(
method.getSimpleName(),
m -> isPublic(m) && !m.equals(method),
m -> isAtLeastAsVisible(m, method),
method.enclClass().type,
state.getTypes())
.collect(toImmutableList());
Expand Down Expand Up @@ -176,25 +182,19 @@ private static Optional<SuggestedFix> trySuggestCallingCustomAlternative(
MethodInvocationTree argument,
VisitorState state,
Map<String, String> alternatives) {
String name = ASTHelpers.getSymbol(tree).getSimpleName().toString();
String alternative = alternatives.get(name);

if (alternative == null) {
return Optional.empty();
}

SuggestedFix fix = SourceCode.unwrapMethodInvocation(argument, state);
return Optional.of(
alternative.equals(name)
? fix
: SuggestedFix.builder()
.merge(SuggestedFixes.renameMethodInvocation(tree, alternative, state))
.merge(fix)
.build());
return Optional.ofNullable(
alternatives.get(ASTHelpers.getSymbol(tree).getSimpleName().toString()))
.map(
replacement ->
SuggestedFix.builder()
.merge(SuggestedFixes.renameMethodInvocation(tree, replacement, state))
.merge(SourceCode.unwrapMethodInvocation(argument, state))
.build());
}

// XXX: Once we target JDK 14+, drop this method in favour of `Symbol#isPublic()`.
private static boolean isPublic(Symbol symbol) {
return symbol.getModifiers().contains(Modifier.PUBLIC);
private static boolean isAtLeastAsVisible(Element symbol, Element reference) {
return Visibility.fromModifiers(symbol.getModifiers())
.compareTo(Visibility.fromModifiers(reference.getModifiers()))
>= 0;
}
}
21 changes: 20 additions & 1 deletion ...rib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitArgumentEnumerationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,21 @@ void identification() {
"",
" DSL.row(ImmutableList.of(1, 2));",
"",
" // BUG: Diagnostic contains:",
" unaryMethod(ImmutableList.of(1, 2));",
" unaryMethodWithLessVisibleOverload(ImmutableList.of(1, 2));",
" binaryMethod(ImmutableList.of(1, 2), 3);",
"",
" ImmutableList.builder()",
" // BUG: Diagnostic contains:",
" .addAll(ImmutableList.of())",
" .build();",
"",
" assertThat(ImmutableList.of(1))",
" // BUG: Diagnostic contains:",
" .containsAnyElementsOf(ImmutableList.of(1));",
" .containsAnyElementsOf(ImmutableList.of(1))",
" // BUG: Diagnostic contains:",
" .isSubsetOf(ImmutableList.of(1));",
"",
" Flux.just(1, 2)",
" .as(StepVerifier::create)",
Expand All @@ -49,6 +56,18 @@ void identification() {
" .setArgs(ImmutableList.of(\"foo\"))",
" .withClasspath();",
" }",
"",
" private void unaryMethod(ImmutableList<Integer> args) {}",
"",
" private void unaryMethod(Integer... args) {}",
"",
" void unaryMethodWithLessVisibleOverload(ImmutableList<Integer> args) {}",
"",
" private void unaryMethodWithLessVisibleOverload(Integer... args) {}",
"",
" private void binaryMethod(ImmutableList<Integer> args, int extraArg) {}",
"",
" private void binaryMethod(Integer... args) {}",
"}")
.doTest();
}
Expand Down

0 comments on commit d6e65b6

Please sign in to comment.