-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Improve error messages for annotation methods on synthetic TypeVariables #7974
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
Conversation
Enhance `Types` class with methods to create artificial type variables that preserve annotations and annotated bounds. Update `TypeVariableImpl` to implement `AnnotatedElement` interface, allowing retrieval of annotations. This change improves compatibility and functionality for type variable management. RELNOTES=n/a
@cpovirk I took a shot at implementing the TypeVariable annotation support you identified in |
Unfortunately, a big part of the problem is that I don't even know what behavior would make sense here :( I am a little fuzzy at the moment even on the exact circumstances that we end up using our own We say every so often that we should get together a list of issues for which it's reasonably like that we actually would try to take a patch. The most recent such list that I have covers: |
…date comments to reflect that custom TypeVariables created by Guava do not preserve annotations intentionally due to unclear semantics. Enhance exception messages for unsupported annotation-related methods to guide users towards using original TypeVariables for annotation access. See b/147144588 for more details.
…ficialTypeVariable` method call. This change simplifies the method signature by omitting the original type variable, aligning with the intent to create a new type variable without preserving the original's context.
…streamline type variable creation. This change aligns with recent refactoring efforts to simplify method signatures and clarify the handling of annotations in custom TypeVariables.
TypeVariable
handling.
@cpovirk thanks for the guidance, i have minimised the impact of this pr in light of what you said to focus on the error message. In the TypeResolver note I have used language to be more like "Until there's a clear specification for what annotations should mean on resolved TypeVariables..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. One comment below.
Method typeVariableMethod = typeVariableMethods.get(methodName); | ||
if (typeVariableMethod == null) { | ||
throw new UnsupportedOperationException(methodName); | ||
// Provide helpful error message for annotation-related methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some others at https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/reflect/AnnotatedElement.html. Some are default
methods, whose behavior I'd have to check under Proxy
, but I'd imagine we'd want to cover at least getAnnotations()
and possibly the default
ones, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. While the existing startsWith checks technically already cover getAnnotations() and getDeclaredAnnotations(), I've added them explicitly for clarity. This mainly makes it crystal clear that we're handling these specific methods, and serves as defensive programming if one of the other startsWith approaches ever changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks, clearly I missed the detail of startsWith
entirely. Maybe we should be doing methodName.contains("Annotat")
or something :) I'll see if an internal reviewer has an opinion when I import this now.
…dded support for `getAnnotations` and `getDeclaredAnnotations` to throw `UnsupportedOperationException`, clarifying that these methods are not applicable to synthetic TypeVariables. This change improves user guidance regarding annotation access on resolved types.
…c `TypeVariable` instances. Fixes #7974 RELNOTES=n/a PiperOrigin-RevId: 810587034
Problem
Synthetic TypeVariables created by
TypeResolver
throw unhelpfulUnsupportedOperationException("methodName")
when annotation methods are called.Addresses TODO b/147144588 in
TypeResolver.java:371-374
.Solution
Add helpful error messages explaining why annotations aren't supported and what to do instead.
Changes:
Impact
Testing
All existing tests pass without modification, confirming backward compatibility.
Example Error Message
Before:
After:
Breaking Changes
None. This change only improves error messages. All existing behavior is preserved: