Skip to content

Conversation

lmcrean
Copy link
Contributor

@lmcrean lmcrean commented Sep 7, 2025

Problem

Synthetic TypeVariables created by TypeResolver throw unhelpful UnsupportedOperationException("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:

  1. Convert TODO to NOTE documenting the intentional limitation
  2. Add descriptive error for annotation methods with bug reference

Impact

  • Better developer experience through clear error messages
  • No functional changes
  • 2 files modified

Testing

# Verify existing tests still pass
./mvnw test -f guava-tests/pom.xml -Dtest="TypeResolverTest,TypesTest"

All existing tests pass without modification, confirming backward compatibility.

Example Error Message

Before:

UnsupportedOperationException: isAnnotationPresent

After:

UnsupportedOperationException: Annotation methods are not supported on synthetic
TypeVariables created during type resolution. The semantics of annotations on
resolved types with modified bounds are undefined. See b/147144588.

Breaking Changes

None. This change only improves error messages. All existing behavior is preserved:

  • Same exceptions thrown
  • Same methods unsupported
  • No new TypeVariable creation
  • No API changes

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
@lmcrean
Copy link
Contributor Author

lmcrean commented Sep 7, 2025

@cpovirk I took a shot at implementing the TypeVariable annotation support you identified in TODO: b/147144588. Does this approach align with what you had in mind? Happy to adjust if needed.

@cpovirk
Copy link
Member

cpovirk commented Sep 10, 2025

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 TypeVariable implementation in the first place (capture conversion? substitution (e.g., resolveType, where)?). Then there's the question of what should happen to annotations during such operations, especially given that the handling in javac itself has changed over time. It's also possible that "proper" handling of annotations would mean returning our own TypeVariable implementation more often instead of the original JDK object (e.g., when substituting in @Nullable E for E?), and that could break existing users.

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.
@lmcrean lmcrean changed the title Add support for annotations in TypeVariable handling. Improve error messages for annotation methods on synthetic TypeVariables Sep 14, 2025
@lmcrean
Copy link
Contributor Author

lmcrean commented Sep 14, 2025

@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..."

Copy link
Member

@cpovirk cpovirk left a 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.

@@ -382,7 +382,17 @@ private static final class TypeVariableInvocationHandler implements InvocationHa
String methodName = method.getName();
Method typeVariableMethod = typeVariableMethods.get(methodName);
if (typeVariableMethod == null) {
throw new UnsupportedOperationException(methodName);
// Provide helpful error message for annotation-related methods
Copy link
Member

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.

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.

3 participants