Skip to content

Add helpful message when using @Assisted with @Inject#4692

Closed
vadym-shavalda wants to merge 2 commits intogoogle:masterfrom
vadym-shavalda:master
Closed

Add helpful message when using @Assisted with @Inject#4692
vadym-shavalda wants to merge 2 commits intogoogle:masterfrom
vadym-shavalda:master

Conversation

@vadym-shavalda
Copy link
Copy Markdown

@vadym-shavalda vadym-shavalda commented Apr 18, 2025

Been bitten by this (arguably, because of my lack of attention at the moment).

Before:

image

After:

image

UPD:

image

@bcorso
Copy link
Copy Markdown

bcorso commented Apr 18, 2025

Thanks for the pull request!

This isn't quite the right place to add this message. The fix here should go into the InjectValidator instead, which is responsible for ensuring the @Inject/@AssistedInject constructor is valid. Reporting the issue in InjectValidator should prevent reaching this checkState when its invalid.

To clarify, the issue with your current solution is that throwing an exception in an annotation processor to report "user issues" is typically bad practice because it immediately stops the compilation and prevents our processor (and others) from reporting any additional issues. Instead there's a special mechanism for reporting errors in annotation processors (e.g. Messager in javac) which should be use to report user issues.

@vadym-shavalda
Copy link
Copy Markdown
Author

Hey, Brad! Thanks for the swift review and pointers on how to implement this properly!
I'm not that familiar with the architecture yet, so opted for quick 'n dirty fix.

Does this look better?

@bcorso
Copy link
Copy Markdown

bcorso commented Apr 18, 2025

Yes, this is more like what I was thinking.

Unfortunately, there's no way to merge your PR directly from GitHub, so I'll need to patch in your change (I can still credit you with the commit). I'll try to find some time to do that today.

@vadym-shavalda
Copy link
Copy Markdown
Author

vadym-shavalda commented Apr 18, 2025

No worries, and thank you!

I'll close the PR, then.
Please, let me know if anything else is needed from my side.
Cheers.

copybara-service Bot pushed a commit that referenced this pull request Apr 23, 2025
Closes #4692.

RELNOTES=N/A
PiperOrigin-RevId: 749597348
copybara-service Bot pushed a commit that referenced this pull request Apr 23, 2025
Closes #4692.

RELNOTES=N/A
PiperOrigin-RevId: 750608984
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.

2 participants