Introduce `JUnitMethodSourceGenericParams` bug checker #1786

mohamedsamehsalah · 2025-07-23T07:13:07Z

JUnit's parameterized tests, annotated with @MethodSource, do not have generic type information at runtime. This means that we can pass a Set of Integers as an argument to a test method which accepts a Set of Strings, and JUnit would disregard the accepted type.

This bug checker flags such cases.

Potential Bug ⚠️

  private static ImmutableSet<Arguments> demoTestCases() {
    return ImmutableSet.of(arguments(ImmutableSet.of(1)));
  }

  @ParameterizedTest
  @MethodSource("demoTestCases")
  void demo(ImmutableSet<String> setOfStrings) {
    ImmutableSet<Integer> setOfIntegers = ImmutableSet.of(1);

    assertThat(setOfStrings).isEqualTo(setOfIntegers); // true
  }

Suggested commit message

Introduce `JUnitMethodSourceGenericParams` bug checker

github-actions · 2025-07-23T07:16:23Z

Surviving mutants in this change: 31
Killed mutants in this change: 78

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	17	78
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	12	0
🧟`tech.picnic.errorprone.utils.MoreJUnitMatchers`	1	0
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie

Nice check idea! Mostly testing that should be improved. I didn't go through the code yet properly, will do that after my holiday :)

...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParams.java

rickie · 2025-07-23T10:41:31Z

...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParams.java

+    }
+
+    Optional<AnnotationTree> methodSourceAnnotation = findMethodSourceAnnotation(tree, state);
+    if (methodSourceAnnotation.isEmpty()) {


Make sure to go over the mutation tests by adding some more coverage :)

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreASTHelpers.java

github-actions · 2025-08-16T11:11:43Z

Surviving mutants in this change: 21
Killed mutants in this change: 85

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	13	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.utils.MoreJUnitMatchers`	1	0
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2025-08-16T11:58:19Z

Surviving mutants in this change: 20
Killed mutants in this change: 86

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	13	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0
🎉`tech.picnic.errorprone.utils.MoreJUnitMatchers`	0	1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2025-08-16T12:23:38Z

Surviving mutants in this change: 22
Killed mutants in this change: 86

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	15	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0
🎉`tech.picnic.errorprone.utils.MoreJUnitMatchers`	0	1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

github-actions · 2025-08-16T12:31:17Z

Surviving mutants in this change: 20
Killed mutants in this change: 86

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	13	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0
🎉`tech.picnic.errorprone.utils.MoreJUnitMatchers`	0	1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

mohamedsamehsalah

Thanks for review @rickie 🙌

I tried to address the mutation test reports, but I will need help to cover all of them. I tried to put the reasoning in the comments.

mohamedsamehsalah · 2025-08-16T11:57:08Z

...rib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParamsTest.java

+            "class A {",
+            // Used to explicitly highlight that we ignore non-parameterized types during this test,
+            // as they are not subject to the check.
+            "  static class IrrelevantNonParameterizedType {}",


(1) I want to ignore non-parameterised types explicitly. The reasoning: These type checks are either performed by Java during compile time or checked by JUnit at runtime.

The checker, as the documentation mentions, should flag cases that are not covered by either.

mohamedsamehsalah · 2025-08-16T12:04:12Z

...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParams.java

+    List<? extends ExpressionTree> arguments = argumentMethodInvocationTree.getArguments();
+    if (treeToString(argumentMethodInvocationTree.getMethodSelect(), state)
+        .contentEquals(ARGUMENT_SET)) {
+      // We skip the first argument, because it's the argument set name.


(2) This and the next mutation test will be hard to cover because the first argument of the argumentSet API is non-parametrised, thus, ignored.

mohamedsamehsalah · 2025-08-16T12:04:36Z

...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParams.java

+  private static boolean allParameterisedArgumentsHaveApplicableTypes(
+      Type testMethodParameterType, Type methodSourceProviderType, Types types) {
+    if (!testMethodParameterType.isParameterized() || !methodSourceProviderType.isParameterized()) {
+      // Skip non-parameterised arguments, as any type incompatibility will be flagged by JUnit at


(3) This is intended, to limit the scope of the checker.

mohamedsamehsalah · 2025-08-16T12:35:07Z

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreASTHelpers.java

+   * @see ASTHelpers#getUpperBound(Type, Types)
+   */
+  public static Type getLowerBound(Type type, Types types) {
+    if (type.hasTag(TypeTag.WILDCARD)) {


(This should be covered by the BugChecker test) 🤔

If I drop the second and third if statement, the test + the BugCheck test succeeds 🤔.

- if (type.hasTag(TypeTag.TYPEVAR) && ((TypeVar) type).isCaptured()) { - return types.cvarLowerBound(type); - } - - if (type.getLowerBound() != null) { - return type.getLowerBound(); - } +// if (type.hasTag(TypeTag.TYPEVAR) && ((TypeVar) type).isCaptured()) { +// return types.cvarLowerBound(type); +// } +// +// if (type.getLowerBound() != null) { +// return type.getLowerBound(); +// }

I think we should figure out if we need these statements or not. Otherwise we should find how to test them. If we can kill more mutants, we can be more sure about what we are actually testing / trying to catch here.

github-actions · 2025-09-03T14:15:43Z

Surviving mutants in this change: 20
Killed mutants in this change: 86

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	13	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0
🎉`tech.picnic.errorprone.utils.MoreJUnitMatchers`	0	1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie

Only some small comments so far. Takes a while to review, but flashing already.

rickie · 2025-09-03T14:30:37Z

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreJUnitMatchers.java

+   *
+   * @param methodTree The method tree on which to find the annotation from.
+   * @param state The {@link VisitorState} from which to derive the AST location of interest.
+   * @return Returns the {@code @MethodSource} annotation on the given method, if any.


Double word if we keep "Returns". And we can use link with fully qualified name as we do above in this class :)

rickie · 2025-09-03T14:35:48Z

error-prone-utils/src/test/java/tech/picnic/errorprone/utils/MoreJUnitMatchersTest.java

        .doTest();
  }

+  @Test


Great coverage, looks good.

rickie · 2025-09-03T14:38:31Z

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreASTHelpers.java

+   * @return The lower bound of the type, or the type itself if it has no lower bound.
+   * @see ASTHelpers#getUpperBound(Type, Types)
+   */
+  public static Type getLowerBound(Type type, Types types) {


We could argue this belongs in a class MoreTypes as it's on the Types class, but I think it's fine to leave it here TBH.

rickie · 2025-09-03T14:48:54Z

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreASTHelpers.java

+   * @see ASTHelpers#getUpperBound(Type, Types)
+   */
+  public static Type getLowerBound(Type type, Types types) {
+    if (type.hasTag(TypeTag.WILDCARD)) {


If I drop the second and third if statement, the test + the BugCheck test succeeds 🤔.

- if (type.hasTag(TypeTag.TYPEVAR) && ((TypeVar) type).isCaptured()) { - return types.cvarLowerBound(type); - } - - if (type.getLowerBound() != null) { - return type.getLowerBound(); - } +// if (type.hasTag(TypeTag.TYPEVAR) && ((TypeVar) type).isCaptured()) { +// return types.cvarLowerBound(type); +// } +// +// if (type.getLowerBound() != null) { +// return type.getLowerBound(); +// }

rickie · 2025-09-04T09:33:19Z

...contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodSourceGenericParams.java

+  private static final Matcher<ExpressionTree> SUPPORTED_METHOD_SOURCE_PROVIDERS =
+      staticMethod()
+          .onDescendantOfAny(
+              List.class.getCanonicalName(),


Let's sort those :)

sonarqubecloud · 2025-09-11T07:25:34Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2025-09-11T07:26:38Z

Surviving mutants in this change: 20
Killed mutants in this change: 86

class	surviving	killed
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams`	13	79
🧟`tech.picnic.errorprone.utils.MoreASTHelpers`	6	6
🧟`tech.picnic.errorprone.bugpatterns.JUnitMethodSourceGenericParams$1`	1	0
🎉`tech.picnic.errorprone.utils.MoreJUnitMatchers`	0	1

Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie

Lol I confused myself with flushing what I had and not 😬.
Pushing a commit with some changes. Let's discuss some of the mutants to get that more sorted out :).

The check has a nice shape. Let's do these two things:

Run it on a repo of ours internally (I recall you might have done this a while back as you asked me about it 🤔).
I've to continue reviewing :)

rickie · 2025-09-11T07:16:34Z

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/MoreASTHelpers.java

+   * @see ASTHelpers#getUpperBound(Type, Types)
+   */
+  public static Type getLowerBound(Type type, Types types) {
+    if (type.hasTag(TypeTag.WILDCARD)) {


I think we should figure out if we need these statements or not. Otherwise we should find how to test them. If we can kill more mutants, we can be more sure about what we are actually testing / trying to catch here.

mohamedsamehsalah requested review from Stephan202 and rickie July 23, 2025 07:13

rickie reviewed Jul 23, 2025

View reviewed changes

mohamedsamehsalah force-pushed the mohamedsamehsalah/junit-method-source-generic-params branch from 032b7ac to 704b79e Compare August 16, 2025 11:07

mohamedsamehsalah force-pushed the mohamedsamehsalah/junit-method-source-generic-params branch from d273d2a to 81947fd Compare August 16, 2025 12:20

mohamedsamehsalah force-pushed the mohamedsamehsalah/junit-method-source-generic-params branch from 81947fd to 0823854 Compare August 16, 2025 12:27

mohamedsamehsalah commented Aug 16, 2025

View reviewed changes

mohamedsamehsalah requested a review from rickie August 16, 2025 12:37

mohamedsamehsalah added 5 commits September 3, 2025 16:12

Introduce JUnitMethodSourceGenericParams bug checker

c7e39dc

Add test coverage for #getLowerBound

8ad3733

Address test mutation reports

81af079

Add a test for MoreJUniMatchers#findMethodSourceAnnotation

650dbb6

Introduce IrrelevantNonParameterizedType

c0a3c4a

rickie force-pushed the mohamedsamehsalah/junit-method-source-generic-params branch from 0823854 to c0a3c4a Compare September 3, 2025 14:12

rickie reviewed Sep 8, 2025

View reviewed changes

rickie added this to the 0.25.0 milestone Sep 8, 2025

rickie added the new feature label Sep 8, 2025

Suggestions

bf527e0

rickie reviewed Sep 11, 2025

View reviewed changes

rickie modified the milestones: 0.25.0, 0.26.0 Sep 22, 2025

Stephan202 removed this from the 0.26.0 milestone Oct 26, 2025

Stephan202 added this to the 0.28.0 milestone Oct 26, 2025

Introduce JUnitMethodSourceGenericParams bug checker #1786

Are you sure you want to change the base?

Introduce JUnitMethodSourceGenericParams bug checker #1786

Uh oh!

Conversation

mohamedsamehsalah commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Potential Bug ⚠️

Suggested commit message

Uh oh!

github-actions bot commented Jul 23, 2025

Uh oh!

rickie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

mohamedsamehsalah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 3, 2025

Uh oh!

rickie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 11, 2025

Quality Gate passed

Uh oh!

github-actions bot commented Sep 11, 2025

Uh oh!

rickie left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Introduce `JUnitMethodSourceGenericParams` bug checker #1786

Introduce `JUnitMethodSourceGenericParams` bug checker #1786

mohamedsamehsalah commented Jul 23, 2025 •

edited

Loading

rickie left a comment •

edited

Loading