Skip to content

Add. hint message for missing Annotation Param's value #2874

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rojae
Copy link

@rojae rojae commented Apr 30, 2025

  • Added a helpful hint in the exception message when Annotation Param is missing a value and parameter name can't be resolved.
  • Added a unit test to cover this case.

Files affected:

  • Contract.java
  • DefaultContractTest.java

Fixes #2337

@rojae rojae changed the title Add. hint message for missing @Param value Add. hint message for missing Annotation Param's value Apr 30, 2025
Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

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

Excellent idea

@velo velo enabled auto-merge (squash) May 5, 2025 13:30
- Added a helpful hint in the exception message when @param is missing a value and parameter name can't be resolved.
- Added a unit test to cover this case.
     - The test is skipped if the code was compiled with the `-parameters` compiler flag.

Files affected:
- Contract.java
- DefaultContractTest.java

Fixes OpenFeign#2337
auto-merge was automatically disabled May 7, 2025 18:25

Head branch was pushed to by a user without write access

@rojae rojae force-pushed the fix/2337-improve-param-exception-msg-add-hint branch from 087c942 to cefb8c4 Compare May 7, 2025 18:25
@rojae
Copy link
Author

rojae commented May 7, 2025

Hi @velo

Since the parent POM uses the -parameters compiler flag during tests,
the test I added was failing because parameter names are available at runtime.
➡️ Build failure log

[Parent POM snippet]

<compilerArgs>
  <arg>-parameters</arg>
</compilerArgs>

I've updated the test to skip execution when this flag is present, which should resolve the build failure.
➡️ Comparison with fix

I missed this earlier because I only ran tests in the core module locally.
Let me know if anything else is needed — thank you!

@velo velo enabled auto-merge (squash) May 10, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation/exception for required java compiler flag "parameters"
2 participants