Balzanka / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

TypeToken.rejectTypeVariables #1362

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In case something other than TypeToInstanceMap wanted to disallow those 
unresolved type variables, would rejectTypeVariables cause any problems if it 
were public?

There doesn't seem to be any higher-level operation that provides this and it 
would seem to be useful to anyone trying to implement something (heterogeneous 
collections) around TypeToken.

Original issue reported on code.google.com by jysjys1...@gmail.com on 7 Apr 2013 at 9:07

GoogleCodeExporter commented 9 years ago
I'll add that rejecting type variables could be done, ad-hoc:

class Example {
   TypeToInstanceMap<Object> map = new MutableTypeToInstanceMap<>();
   TypeToken<?> token;
   Object value;

   <T> void set(TypeToken<T> token, T value) {
      map.getInstance(token); // rejects type variables
      this.token = token;
      this.value = value;
   }
}

Original comment by jysjys1...@gmail.com on 7 Apr 2013 at 10:21

GoogleCodeExporter commented 9 years ago
Internally we only have this one use case so that's why it's not public.

Can you describe your use case?

My uncertainty has been that different use cases may need slightly different 
functionality. For example, in addition to type variable, maybe wildcards also 
need to be excluded. Or maybe generic array isn't supported either.

Original comment by be...@google.com on 11 Apr 2013 at 5:45

GoogleCodeExporter commented 9 years ago

Original comment by be...@google.com on 11 Apr 2013 at 5:46

GoogleCodeExporter commented 9 years ago
My use case was constructing some set of heterogeneous collections; the backing 
structure of these is a simple entry-like class (immutable) that composites a 
TypeToken and an associated value.

// imports

public interface Typable<B> {

    // is the value null?
    boolean isNulled();

    TypeToken<? extends B> getToken();

    B get();

    <T extends B> T get(TypeToken<T> type);

    <T extends B> T get(Class<T> clas);

    // Optional.fromNullable(get())
    Optional<B> opt();

    <T extends B> Optional<T> opt(TypeToken<T> type);

    <T extends B> Optional<T> opt(Class<T> clas);

    boolean equals(Object o);

    int hashCode();

    Map.Entry<TypeToken<? extends B>, B> toEntry();

}

Reasons for being able to reject type tokens are pretty much the same as 
TypeToInstanceMap. I suppose I'll attach sources of Typable; I may be doing 
some things incorrectly.

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 4:04

Attachments:

GoogleCodeExporter commented 9 years ago
I see. Sounds like a reasonable use case.

Still, two use cases don't make a strong case for a public API, especially one 
that feels a bit weird.

I don't know why I feel it's weird (maybe just that the method name doesn't 
make it clear enough why one wants to do it).

And then, some day another use case of rejecting wildcards might even come up. 
So whether we need a more generic/flexible API is still unclear to me.

So while you have a relatively simple work-around, I'd like to punt on this for 
a while and maybe revisit later when more use cases come up.

Thanks for sharing your use case!

Original comment by be...@google.com on 12 Apr 2013 at 4:36

GoogleCodeExporter commented 9 years ago
I'll add another thought, not of high priority necessarily; being able to, at 
least, assert that a TypeToken type is "concrete."

i.e.
is either a class with no type parameter (String, ExecutorService, and not T, 
?), or is a concrete parameterized type (List<Integer>, Comparable<Complex>, 
and not Collection<?>, Class<? extends Object>, Future<T>)

Quick Reference: 
http://www.angelikalanger.com/GenericsFAQ/FAQSections/Features.html

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 12:49

GoogleCodeExporter commented 9 years ago
Doesn't the FAQ only refers to "types with wildcards" as not concrete? Am I 
missing the piece where it also excludes Future<T>?

Original comment by be...@google.com on 12 Apr 2013 at 1:06

GoogleCodeExporter commented 9 years ago
I used "concrete" loosely; I didn't make that as apparent as possible. Is there 
a term that fits what I described? Namely, raw classes or parameterized types 
with no wildcards or type variables, or, at least, a method (or a few) that 
allow for query of whether wildcards or type variables exist.

i.e.

boolean containsAnyOf(Class<? extends Type>... types)
boolean containsAnyOf(TypeToken<? extends Type>... types)

// This would be able to traverse type parameters to find any of the specified 
types
// perhaps TypeTokens would suffice as arguments...?

containsAnyOf(TypeVariable.class)
containsAnyOf(TypeVariable.class, Wildcard.class)
containsAnyOf(new TypeToken<Class<Object>>(){})

Map<K, Collection<?>>
 // true
 // true
 // false

Map<String, Collection<Future<?>>>
 // false
 // true
 // false

Map<Class<?>, Collection<Object>>
 // false
 // true
 // true

Map<String, Collection<Object>>
 // false
 // false
 // true

That example is contrived, but I hope that it conveyed the point well enough.

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 1:53

GoogleCodeExporter commented 9 years ago
I'm with you.

It's occurred to me a few times that there is a distinction between fully known 
types (Foo.class, List<Foo>) and partially unknown types (List<?>, Map<K, 
String> A[] etc).

I also searched but didn't find this concept being clearly defined anywhere. 
That's one reason a isConcrete() method doesn't exist today in TypeToken.

Another issue is, I find myself needing slight variants of the concept anyway. 
For example in TypeToInstanceMap I need rejectTypeVariable() while wildcards 
work fine for my purpose.

That strengthened the doubt that a isConcrete() method will be truly useful.

The containsAnyOf() is clearly more flexible. I thought about it and it's what 
I referred to as "a more generic/flexible API". 

Though I wasn't sure if it's really the optimal API form. It feels a bit over 
flexible. Is this even meaningful? 
containsAnyOf(Class.class, Wildcard.class) 

Original comment by be...@google.com on 12 Apr 2013 at 2:21

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
From my understanding, Class would have to be parameterized to extend Type:

containsAnyOf(Class.class, Wildcard.class)

T // false
? // true, Wildcard.class.isInstance(?)
Object // true, Class.class.isInstance(Object)
LinkedList // true, Class.class.isInstance(LinkedList)
LinkedList<T> // true, Class.class.isInstance(LinkedList)
LinkedList<Object> // true, Class.class.isInstance(LinkedList)
LinkedList<?> // true, Class.class.isInstance(LinkedList)

The above would likely give a raw type warning. From my understanding, below 
would avoid raw type warning:

containsAnyOf(
   new TypeToken<Class<?>>() {},
   TypeToken.of(Wildcard.class)
)

T // false
? // true, TypeToken.of(Wildcard.class).isInstance(LinkedList)
Object // true, new TypeToken<Class<?>>() {}.isInstance(LinkedList)
LinkedList // true, new TypeToken<Class<?>>() {}.isInstance(LinkedList)
LinkedList<T> // true, new TypeToken<Class<?>>() {}.isInstance(LinkedList)
LinkedList<Object> // true, new TypeToken<Class<?>>(){}.isInstance(LinkedList)
LinkedList<?> // true, new TypeToken<Class<?>>() {}.isInstance(LinkedList)

And it has just occurred to me that handling GenericArrayType would have to be 
well enough defined:

containsAnyOf(
   new TypeToken<Class<?>>() {},
   TypeToken.of(Wildcard.class)
)

LinkedList<?>[] // ?; possibly true?

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 2:51

GoogleCodeExporter commented 9 years ago
ignore the spiel about raw type warning.

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 3:13

GoogleCodeExporter commented 9 years ago
I would assume that adding methods (aside from TypeToken.getComponentType()) 
that allow for walking through specific types clutters API...

It could be done by introspecting the Type returned by TypeToken.getType(), but 
providing a few methods could ease having to check getType().

isParameterized()
getActualTypeArguments()
getOwnerTypes()
 // from ParameterizedType

 //// getActualTypeArguments
 ////// Class, WildcardType, TypeVariable return empty array

 //// getOwnerType
 ////// Class.getEnclosingClass?
 ////// WildcardType and TypeVariable would return the "owner types" of
 ////// their upper bounds (possibly an empty array)

isArray()
getComponentType()
// they exist already. these covers a lot

isVariable()
isWildcard()
getUpperBounds()
getLowerBounds()
 // from WildcardType
 //// [Class.this, GenericArrayType.this, TypeVariable.getBounds]
 //// suffice for upper bound.
 //// [Class.this, GenericArrayType.this, TypeVariable.this] suffice
 //// for lower bound.

containsAnyActualTypeArgumentsOf(TypeToken<? extends Type>...)
containsAny[Upper|Lower]BoundsOf(TypeToken<? extends Type>...)
 // 1-level checks, these wouldn't go deep into anything.

containsAnyOf(
    Iterable<? extends TypeToken<? extends Type>,
    WalkOption...)
enum WalkOption {WALK_COMPONENT_TYPE, WALK_ACTUAL_TYPE_ARGUMENTS, 
WALK_UPPER_BOUNDS};
 // I doubt these would be implemented. These would deeply check
 // a token for any of the specified types.

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 4:24

GoogleCodeExporter commented 9 years ago
add to WalkOption: WALK_RAW_TYPE; this would be the default if no options are 
specified; essentially:

final Class<?> raw = getRawType();
for (TypeToken<? extends Type> token : tokens)
   if (token.getType().isAssignableFrom(raw))
      return true;
return false;

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 4:43

GoogleCodeExporter commented 9 years ago
isParameterized() - equivalent to getType() instanceof ParameterizedType
isVariable() - getType() instanceof TypeVariable
isWildcard() - getType() instanceof WildcardType

So I don't feel their addition brings a lot of value.

getActualTypeArguments() - I'm afraid some users might expect different 
behavior for <? extends ImmutableLIst<String>>. In other words, we are trying 
to extend the "actual type parameters" concept in a potentially ambiguous way.

getOwnerTypes()
getUpperBounds()
getLowerBounds()
---- these don't feel needed often enough to justify the API addition

containsAnyActualTypeArgumentsOf(TypeToken<? extends Type>...)
containsAny[Upper|Lower]BoundsOf(TypeToken<? extends Type>...)

---- From time to time, I needed to do some kind of type traversal. 
containsAny() may be one of them, but not necessarily the only one or the most 
common one. I haven't been able to think of a common API form that's simple and 
flexible enough to cover all of them, sadly.

Original comment by be...@google.com on 12 Apr 2013 at 6:31

GoogleCodeExporter commented 9 years ago
Your getActualTypeArguments() <em>should</em> actually get String from <? 
extends ImmutableList<String>> then; I typed everything above rather quickly.

getOwnerTypes was added trivially to show that it could be implemented over all 
types.

getUpperBounds would allow walking through WildcardType and TypeVariable bounds 
without introspection from the user.

getLowerBounds is there for completeness, I suppose.

Do you feel anything towards:

containsAnyOf(
    Iterable<? extends TypeToken<? extends Type>>,
    WalkOption...)

and would:

containsAnyOf(
    Predicate<? super TypeToken<?>>,
    WalkOption...)

generalize it? Although, I hope I'm not contriving too much; adding WalkOptions 
may be a bit of a commitment.

Original comment by jysjys1...@gmail.com on 12 Apr 2013 at 7:01

GoogleCodeExporter commented 9 years ago
What do you think of a TypeVisitor class that you can use as:

new TypeVisitor() {
  @Override void visitTypeVariable(TypeVariable<?> var) {
    throw new IllegalArgumentException(...);
  }
}).visit(type);

I think I might also be able to use it to get rid of some instanceof calls 
inside TypeToken.

Original comment by be...@google.com on 16 Apr 2013 at 2:52

GoogleCodeExporter commented 9 years ago
Okay.

I assume that TypeVisitor.visit(TypeToken) exhaustively traverses through the 
specified token. If so, would there be some method of limiting how deep the 
search goes? Or do you think that it would be needed?

Is this the gist of the Visitor?

class TypeVisitor {

   visit(TypeToken<?> type) { ... }

   visitTypeVariable(TypeVariable<?> type) {}
   visitGenericArrayType(GenericArrayType type) {}
   visitWildcardType(WildcardType type) {}
   visitParameterizedType(ParameterizedType type) {}
   visitClass(Class<?> type) {}

}

Original comment by jysjys1...@gmail.com on 17 Apr 2013 at 11:15

GoogleCodeExporter commented 9 years ago
Yeah. I realized that while implementing an internal one too.

Basically, different subclasses need different traversal strategies:

* Some visits bounds, owner types and type arguments.
* Some cares about about upper bounds, not lower bounds.
* Some visits generic super types of Class.
* Some visits component type of GenericArray.

The crude version can allow subclass control what it wants to traverse, such as:

new TypeVisitor() {
  visitTypeVariable(type) {
    visit(type.getUpperBounds());
    visit(type.getLowerBounds()):
  }

  visitWildcardType(type) {
    visit(type.getBounds());
  }

  visitParameterizedType(type) {
    visit(type.getActualTypeArguments());
    visit(type.getOwnerType());
  }

  visitGenericArrayType(type) {
    visit(type.getComponentType());
  }
}

The catch is, user would need to know what he's doing and drive the traversal 
in the correct direction. For example, missing to visit getOwnerType() isn't 
sufficient for rejectTypeVariable() purpose.

There could be other ways to offer a higher level abstraction. But the right 
API hasn't revealed itself to me yet.

Original comment by be...@google.com on 18 Apr 2013 at 12:06

GoogleCodeExporter commented 9 years ago
Okay.

Covering traversing of a type hierarchy (i.e. Class.getInterfaces()) vs 
traversal of the components of a type (i.e. upper/lower bounds or component 
types), and any other forms of traversal seem to cover a broad basis.  
TypeVisitor seems to allow for that at a low level; as if to say, "once I know 
that type is a class, I could possibly traverse its hierarchy without much need 
for any other methods."

{
  visitClass(type) {
    // traverse hierarchy
    classVisited(type);
    visitClass(type.getSuperclass());
  }

  classVisited(type) {
    // act on visited class
  }

  visitWildcardType(type) {
    visit(type.getBounds());
  }

  ...

}

I feel as if finding an API would entail finding some things specific that 
you'd want a subclass of TypeVisitor to do (such as merely examining components 
of a type as mentioned early, or traversing a type hierarchy).

// This examines the type as mentioned in above comments
/// End points are classes

class TypeDeclarationVisitor() { (?)

  visit(types) {
    for (type : types)
      visit(type);
  }

  visit(type) {
    match (type, instanceof) {
      case Class<?>: visitClass(type)
      case TypeVariable<?>: visitTypeVariable(type)
      case ParameterizedType: visitParameterizedType(type)
      ...
    }
  }

  visitClass(type) {
    classVisited(type)
  }

  visitTypeVariable(type) {
    typeVariableVisited(type);
    visit(type.getBounds());
  }

  visitWildcardType(type) {
    wildcardTypeVisited(type);
    visit(type.getUpperBounds());
  }

  visitParameterizedType(type) {
    parameterizedTypeVisited(type);
    visit(type.getActualTypeArguments());
    visit(type.getOwnerType());
  }

  visitGenericArrayType(type) {
    genericArrayTypeVisited(type);
    visit(type.getComponentType());
  }

  classVisited(type) {}
  typeVariableVisited(type) {}
  ...

}

Subclasses use the [type]Visited(type) methods to do anything relevant (add a 
visited class to some referenced collection or to assert that a wildcard type 
has upper bounds, etc.).

At this point, I'm just jotting, but, it seem that, as a root to specialized 
subclasses, TypeVisitor may ease a good deal of traversal through Types.

Original comment by jysjys1...@gmail.com on 18 Apr 2013 at 1:24

GoogleCodeExporter commented 9 years ago
ClassVisited() doesn't feel necessary. Subclasses can just override 
visitClass(), and call super.visitClass() if it wants to follow through the 
traversal.

This is useful when subclass wants to stop the traversal if something is 
already found, or that the default traversal works mostly but not for a certain 
type.

For example, I may want to follow the default traversal except that the lower 
bounds of type variables are of no use. I could just do:

@Override visitTypeVariable(t) {
  // do my thing
  visit(t.getUpperBounds());
}

And keep everything else the same.

Regarding traversing over type components vs. type hierarchy. The distinction 
is still vague. You seem to suggest to use the TypeDeclarationVisitor to mean 
that we traverse through type bounds, not generic super types of Class.

But it may still be counter-intuitive to some: When I see Map<K, V>, I may 
think that K and V are the atomic leaf nodes because syntactically their bounds 
don't appear in the parameterized type.

From another perspective, both "<K extends List> and "class ArrayList extends 
List" are type *definitions* with super types defined. Traversing in the case 
of type variable definition but not Class definition is a bit hard to 
rationalize without concrete use cases.

So how about having the default implementation only traverse syntactical type 
components?

visitWildcardType(t) {
  visit(t.getBounds());
}
visitGenericArrayType(t) {
  visit(t.getComponentType());
}
visitParameterizedType(t) {
  visit(t.getOwnerType());
  visit(t.getActualTypeArguments());
}
visitClass(t) {}
visitTypeVariable(t) {}

Original comment by be...@google.com on 18 Apr 2013 at 1:55

GoogleCodeExporter commented 9 years ago
Well. Still, for a TypeVisitor or TypeDeclarationVisitor, it's not the most 
intuitive that the default implementation visits T of T[], but not String of 
String[].

And then the default implementation is quite trivial. For the few forwarding it 
does and the potential confusion it may cause, I can't help doubting whether we 
want the default implementation, as opposed to leaving every visit*Type() 
method empty and let subclasses drive.

Original comment by be...@google.com on 18 Apr 2013 at 2:21

GoogleCodeExporter commented 9 years ago
The name was likely poorly conceived.

I want to make certain that I follow you.

Would wildcard types in both Map<?, ?> and <? extends List> invoke the same 
arguments that you proposed of atomic leaves for Map<K, V> and type definition 
<K extends List> respectively?  Would we (or not) check the wildcard upper 
bounds as well if we aren't checking TypeVariable bounds?

Otherwise, and besides that, generally, TypeVisitor could do anything in terms 
of type traversal; having a few typical subclasses that lean towards 
specialized traversal would be useful.

Original comment by jysjys1...@gmail.com on 18 Apr 2013 at 2:54

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I don't think a default implementation for a root, TypeVisitor, is needed per 
se. It would only be help for subclasses that would assert how the 
implementation generally visits, i.e. a visitor that visits super-types to the 
root. That would be common, and other users could subclass that easily enough.

Original comment by jysjys1...@gmail.com on 18 Apr 2013 at 3:00

GoogleCodeExporter commented 9 years ago
Yeah. I was thinking that we follow bounds of wildcards because they 
syntactically appear in the type reference, while type variable bounds don't 
appear in the type reference. But then I realized that traversing T for T[] but 
not String for String[] creates another confusion.

I agree it's mostly about finding a good class name that makes the semantics 
easy to understand.

Another thought is to call it GenericTypeVisitor:

/*
 * Given a Type, calls corresponding visit*() method according to what kind of Type it is.
 * By default, all generic type components of the Type is visited, until either a raw class is reached,
 * or a type bound recursion is detected. This includes: <ul>
 * <li> T of T[] is visited.
 * <li> List and T of List<T> are visited.
 * <li> T of <? extends T> is visited.
 * <li> Enum<E> of <E extends Enum<E>> is visited.
 * </ul>
 * If a subclass needs to traverse through Class's generic super classes, it can override
 * visitClass() and call visit(clazz.getGenericSuperClass()) and visit(clazz.getGenericInterfaces()).
 */
public abstract GenericTypeVisitor {
  ...
}

It seems overkill to expose more than one public classes for the type visitor 
concept. If we are to add it to Guava, I feel we want at most one type and no 
more.

Original comment by be...@google.com on 18 Apr 2013 at 3:06

GoogleCodeExporter commented 9 years ago
That's reasonable. Most importantly, just so long as the doc explains what's 
going on by default, there should not be as many problems concerning overriding 
methods.

Original comment by jysjys1...@gmail.com on 18 Apr 2013 at 3:45

GoogleCodeExporter commented 9 years ago
That might work.

Now we have a clearer picture of the API. It's still an open question whether 
such API is worth it. And we need use cases.

Use case 1: rejectTypeVariable().

Is there any other use cases for walking the type components that'd help us 
make the decision?

Original comment by be...@google.com on 18 Apr 2013 at 3:58

GoogleCodeExporter commented 9 years ago
I'm terrible at finding specific use cases. A Visitor seems versatile; it could 
practically do anything just so long as its "visiting," and that does not even 
really even define how a subclass would go about "visiting..."

I suppose, vaguely and obviously, its uses would likely lean towards carrying 
out common reflection, or maybe, more specifically, traversing type 
hierarchies, perhaps in mass--selecting classes of a particular match, 
detecting attributes of classes, and/or collecting specific members of certain, 
or all, classes.

//----------

collectAllPublicAbstractMethodsInHierarchyVisitor.visit(AbstractSequentialList.c
lass);
Set<Method> methodSet = 
collectAllPublicAbstractMethodsInHierarchyVisitor.copySet();

// first in hierarchy such that visitor is assignable from the specified type
findTheFirstMatchVisitor.setRelation(assignableFrom(ArrayList.class));
findTheFirstMatchVisitor.visit(AbstractSequentialList.class, Collection.class, 
String.class);
Set<Type> selection = 
collectAllPublicAbstractMethodsInHierarchyVisitor.selection();

// selection.isEmpty() == false
// selection -> { AbstractList.class, Collection.class, Object.class }

findTheFirstMatchVisitor.setRelation(hasMethod("setColor(Color?)"));
findTheFirstMatchVisitor.visit(listOfTypes);

// Note, you probably either just collect each setColor method, or even just 
invoke them during the visit.

//----------

I'd hope that those cases are relevant.

Original comment by jysjys1...@gmail.com on 20 Apr 2013 at 5:24

GoogleCodeExporter commented 9 years ago
Agreed that visitor is versatile. And it's the reason I'd prefer releasing it 
over the too-specific rejectTypeVariable() method.

It could be used to traverse super types, but we do have a higher-level 
abstraction for it: TypeToken.getTypes().

Still, I'm sure I won't convince the team to release it when I can only name 
one clear use case. Reflection utilities are niche utilities so we don't need a 
ton of use cases to justify, but ideally we'd want 3, or the single use case 
must be very commonly needed.

Original comment by be...@google.com on 20 Apr 2013 at 2:26

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08