Skip to content

Conversation

@jihun4452
Copy link
Contributor

Closes #5079

Summary

This PR adds validation for the number of provided arguments when using field injection with @ParameterizedClass.
Previously, insufficient arguments caused an ArrayIndexOutOfBoundsException with an unclear error message.
Now, a ParameterResolutionException is thrown with a clear and descriptive message.

Changes

  • Added validation logic in assertEnoughArgumentsForFieldInjection() to check argument count before field injection
  • Throws ParameterResolutionException when insufficient arguments are provided
  • Error message now includes:
    • The field name, parameter index, and type
    • The number of arguments provided vs. required
  • Added helper methods:
    • requiredArgumentCountForParameterFields() — computes the required argument count
    • firstMissingParameterFieldByIndex() — identifies the first missing field
  • Added integration test failsWithMeaningfulErrorWhenTooFewArgumentsProvidedForFieldInjection() to verify error message clarity

Implementation Notes

  • Added @SuppressWarnings("NullAway") for the ternary expression handling arguments.get(), which can return a @Nullable Object[]. The null case is safely handled, but NullAway's static analysis cannot infer this guarantee
  • Added null checks in helper methods to handle edge cases where fields may not have @Parameter annotations

Example

Before:

ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1

After:

ParameterResolutionException: Not enough arguments for @ParameterizedClass field injection in NotEnoughArgumentsForFieldsTestCase: field 's' (index 1, type java.lang.String) cannot be injected because only 1 argument(s) were provided; at least 2 are required.

I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

  • There are no TODOs left in the code
  • Method preconditions are checked and documented in the method's Javadoc (N/A — private methods)
  • Coding conventions (e.g., for logging) have been followed
  • Change is covered by automated tests including corner cases, errors, and exception handling
  • Public API has Javadoc and @API annotations (N/A — internal implementation only)
  • Change is documented in the User Guide and Release Notes (will add if requested by maintainers)

Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround! I think it would be simpler to implement this in ResolverFacade.Converter#resolve(FieldContext, ExtensionContext, EvaluatedArgumentSet, int). Could you please give that a try?

@jihun4452
Copy link
Contributor Author

Thanks for the review! I’ll try implementing it in ResolverFacade.Converter#resolve as you suggested.

@jihun4452
Copy link
Contributor Author

Hi @marcphilipp,

I’ve made some changes based on your feedback.
The argument validation is now handled inside ResolverFacade.Converter#resolve(FieldContext, …),
so each field performs validation during resolution.
I moved the validation logic there so that it runs naturally as part of field resolution.

I’ve run the related tests, and they pass as expected.
Could you please check if this approach matches what you had in mind?
Please let me know if you’d prefer a different approach.

@marcphilipp marcphilipp merged commit c99fba6 into junit-team:main Oct 27, 2025
16 checks passed
@marcphilipp
Copy link
Member

Thank you for your contribution, @jihun4452! 👍

I took a closer look and decided to move the validation to ArgumentCountValidator for consistency with other related checks.

@jihun4452
Copy link
Contributor Author

@marcphilipp
Thanks for the merge and clear explanation!
Moving the validation to ArgumentCountValidator makes perfect sense for consistency.
I really appreciate the well-organized and thoughtful refactor, and I’ve learned a lot from this contribution. 🙏

@sbrannen sbrannen changed the title fix: Validate argument count for field injection #5079 Validate argument count for field injection #5079 Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using @ParameterizedClass with field injection should validate enough arguments are provided

2 participants