Skip to content

Commit

Permalink
Handle redundant curly braces properly
Browse files Browse the repository at this point in the history
  • Loading branch information
@oxkitsune
oxkitsune committed Nov 22, 2022
1 parent 6ceb2b4 commit be56254
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 41 deletions.
19 changes: 10 additions & 9 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private static Optional<SuggestedFix> tryConstructValueSourceFix(
.replace(
methodSourceAnnotation,
String.format(
"@ValueSource(%s = {%s})",
"@ValueSource(%s = %s)",
toValueSourceAttributeName(parameterType), valueSourceAttributeValue))
.delete(valueFactoryMethod)
.build());
Expand Down Expand Up @@ -265,14 +265,15 @@ private static Optional<String> tryExtractValueSourceAttributeValue(
}

return Optional.of(
arguments.stream()
.map(
arg ->
arg instanceof MethodInvocationTree
? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())
: arg)
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", ")));
arguments.stream()
.map(
arg ->
arg instanceof MethodInvocationTree
? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())
: arg)
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", ")))
.map(value -> String.format(arguments.size() > 1 ? "{%s}" : "%s", value));
}

private static String toValueSourceAttributeName(Type type) {
Expand Down
90 changes: 58 additions & 32 deletions ...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitValueSourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,16 @@ final class JUnitValueSourceTest {

// XXX: Combine `identification` tests, focussing on what should _not_ be flagged.
// XXX: Still to test (likely not exhaustive):
// - Make sure multi-dim arrays aren't flagged.
// - Verify that same-name factory methods are handled properly.
// - Verify that redundant curly braces in `@ValueSource` are handled properly.
// - Make sure that `return` statement inside local classes, anonymous classes and lambda
// expressions inside value factories are ignored.
// - Run PITest and check what other cases should be covered.

@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import static org.assertj.core.api.Assertions.assertThat;",
"import static org.junit.jupiter.params.provider.Arguments.arguments;",
"",
"import java.util.Optional;",
"import java.util.stream.Stream;",
"import org.junit.jupiter.params.ParameterizedTest;",
"import org.junit.jupiter.params.provider.Arguments;",
Expand All @@ -41,40 +36,40 @@ void identification() {
" @ParameterizedTest",
" // BUG: Diagnostic contains:",
" @MethodSource(\"identificationTestCases\")",
" void identification(int foo) {",
" assertThat(foo).isNotNull();",
" }",
" void identification(int foo) {}",
"",
" private static Stream<Arguments> multipleParametersTestCases() {",
" return Stream.of(arguments(1, 2), arguments(3, 4));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"multipleParametersTestCases\")",
" void multipleParameters(int first, int second) {",
" assertThat(first).isNotNull();",
" void multipleParameters(int first, int second) {}",
"",
" private static Stream<Arguments> multiDimArrayTestCases() {",
" return Stream.of(arguments(new int[] {1, 2}), arguments(new int[] {3, 4}));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"multiDimArrayTestCases\")",
" void multiDimArray(int[] foo) {}",
"",
" private static Stream<Arguments> runtimeValueTestCases() {",
" int second = 1 + 2;",
" return Stream.of(arguments(1), arguments(second));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"runtimeValueTestCases\")",
" void runtimeValue(int foo) {",
" assertThat(foo).isNotNull();",
" }",
" void runtimeValue(int foo) {}",
"",
" private static Stream<Arguments> streamChainTestCases() {",
" return Stream.of(1, 2).map(Arguments::arguments);",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"streamChainTestCases\")",
" void streamChain(int number) {",
" assertThat(number).isNotNull();",
" }",
" void streamChain(int number) {}",
"",
" private static Stream<Arguments> multipleReturnsTestCases() {",
" if (10 > 0) {",
Expand All @@ -86,9 +81,7 @@ void identification() {
"",
" @ParameterizedTest",
" @MethodSource(\"multipleReturnsTestCases\")",
" void multipleReturns(int number) {",
" assertThat(number).isNotNull();",
" }",
" void multipleReturns(int number) {}",
"",
" private static Stream<Arguments> multipleFactoriesFooTestCases() {",
" return Stream.of(arguments(1));",
Expand All @@ -100,30 +93,43 @@ void identification() {
"",
" @ParameterizedTest",
" @MethodSource({\"multipleFactoriesFooTestCases\", \"multipleFactoriesBarTestCases\"})",
" void multipleFactories(int i) {",
" assertThat(i).isNotNull();",
" }",
" void multipleFactories(int i) {}",
"",
" private static Stream<Arguments> extraArgsTestCasts() {",
" private static Stream<Arguments> extraArgsTestCases() {",
" return Stream.of(arguments(1), arguments(1, 2));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"extraArgsTestCasts\")",
" void extraArgsTestCasts(int... i) {",
" assertThat(i).isNotNull();",
" }",
" @MethodSource(\"extraArgsTestCases\")",
" void extraArgs(int... i) {}",
"",
" private static Stream<Arguments> localClassTestCases() {",
" class Foo {}",
" class Foo {",
" Stream<Arguments> foo() {",
" return Stream.of(arguments(1), arguments(2));",
" }",
" }",
" return Stream.of(arguments(1), arguments(1, 2));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"extraArgsTestCasts\")",
" void extraArgsTestCasts(int... i) {",
" assertThat(i).isNotNull();",
" @MethodSource(\"localClassTestCases\")",
" void localClass(int... i) {}",
"",
" private static Stream<Arguments> lambdaReturnTestCases() {",
" int foo =",
" Optional.of(10)",
" .map(",
" i -> {",
" return i / 2;",
" })",
" .orElse(0);",
" return Stream.of(arguments(1), arguments(1, 2));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"lambdaReturnTestCases\")",
" void lambdaReturn(int... i) {}",
"}")
.doTest();
}
Expand Down Expand Up @@ -301,6 +307,18 @@ void replacement() {
" @ParameterizedTest",
" @MethodSource(\"streamOfClassesAndClassArguments\")",
" void string(Class<?> c) {}",
"",
" private static Stream<Arguments> sameNameFactoryTestCases() {",
" return Stream.of(arguments(1));",
" }",
"",
" private static Stream<Arguments> sameNameFactoryTestCases(int overload) {",
" return Stream.of(arguments(overload));",
" }",
"",
" @ParameterizedTest",
" @MethodSource(\"sameNameFactoryTestCases\")",
" void sameNameFactory(int i) {}",
"}")
.addOutputLines(
"A.java",
Expand Down Expand Up @@ -401,6 +419,14 @@ void replacement() {
" @ParameterizedTest",
" @ValueSource(classes = {Stream.class, java.util.Map.class})",
" void string(Class<?> c) {}",
"",
" private static Stream<Arguments> sameNameFactoryTestCases(int overload) {",
" return Stream.of(arguments(overload));",
" }",
"",
" @ParameterizedTest",
" @ValueSource(ints = 1)",
" void sameNameFactory(int i) {}",
"}")
.doTest(TestMode.TEXT_MATCH);
}
Expand Down

0 comments on commit be56254

Please sign in to comment.