Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce IsRefasterAsVarargs matcher for use by Refaster templates #623

Merged
merged 4 commits into from
May 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ int after(T value1, T value2) {
*/
static final class MinOfVarargs<T> {
@BeforeTemplate
@SuppressWarnings("StreamOfArray" /* In practice individual values are provided. */)
T before(@Repeated T value, Comparator<T> cmp) {
return Stream.of(Refaster.asVarargs(value)).min(cmp).orElseThrow();
}
Expand Down Expand Up @@ -287,7 +286,6 @@ T after(T value1, T value2, Comparator<T> cmp) {
*/
static final class MaxOfVarargs<T> {
@BeforeTemplate
@SuppressWarnings("StreamOfArray" /* In practice individual values are provided. */)
T before(@Repeated T value, Comparator<T> cmp) {
return Stream.of(Refaster.asVarargs(value)).max(cmp).orElseThrow();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.NotMatches;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
Expand All @@ -46,6 +47,7 @@
import java.util.stream.Stream;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
import tech.picnic.errorprone.refaster.matchers.IsLambdaExpressionOrMethodReference;
import tech.picnic.errorprone.refaster.matchers.IsRefasterAsVarargs;

/** Refaster rules related to expressions dealing with {@link Stream}s. */
@OnlineDocumentation
Expand Down Expand Up @@ -100,12 +102,9 @@ Stream<T> after(T object) {
* Prefer {@link Arrays#stream(Object[])} over {@link Stream#of(Object[])}, as the former is
* clearer.
*/
// XXX: Introduce a `Matcher` that identifies `Refaster.asVarargs(...)` invocations and annotate
// the `array` parameter as `@NotMatches(IsRefasterAsVarargs.class)`. Then elsewhere
// `@SuppressWarnings("StreamOfArray")` annotations can be dropped.
static final class StreamOfArray<T> {
@BeforeTemplate
Stream<T> before(T[] array) {
Stream<T> before(@NotMatches(IsRefasterAsVarargs.class) T[] array) {
return Stream.of(array);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package tech.picnic.errorprone.refaster.matchers;

import static com.google.errorprone.matchers.Matchers.staticMethod;

import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.refaster.Refaster;
import com.sun.source.tree.ExpressionTree;

/** A matcher of {@link Refaster#asVarargs} method invocations. */
public final class IsRefasterAsVarargs implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DELEGATE =
staticMethod().onClass(Refaster.class.getName()).named("asVarargs");

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

@Override
public boolean matches(ExpressionTree expressionTree, VisitorState state) {
return DELEGATE.matches(expressionTree, state);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package tech.picnic.errorprone.refaster.matchers;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;

import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.BugChecker;
import org.junit.jupiter.api.Test;

final class IsRefasterAsVarargsTest {
@Test
void matches() {
CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import com.google.errorprone.refaster.Refaster;",
"",
"class A {",
" int[] negative1() {",
" return new int[4];",
" }",
"",
" String[] negative2() {",
" return \"foo\".split(\"o\");",
" }",
"",
" String[] negative3() {",
" return asVarArgs(\"bar\");",
" }",
"",
" String[] positive1() {",
" // BUG: Diagnostic contains:",
" return Refaster.asVarargs(\"o\");",
" }",
"",
" private static String[] asVarArgs(String s) {",
" return s.split(\"a\");",
" }",
"}")
.doTest();
}

/** A {@link BugChecker} that simply delegates to {@link IsRefasterAsVarargs}. */
@BugPattern(summary = "Flags expressions matched by `IsRefasterAsVarargs`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;

// XXX: This is a false positive reported by Checkstyle. See
// https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
@SuppressWarnings("RedundantModifier")
public MatcherTestChecker() {
super(new IsRefasterAsVarargs());
}
}
}