Generalizing MoreTypes.isTypeOf()

[AriaAdibi]

The Generalization

In this pull request, I am proposing a generalization that I found very useful in my work with annotation processors.

Currently, MoreTypes.isTypeOf(Class<?> clazz, TypeMirror type) checks if the type represents the exact same type as the given class or not.

This has the following shortcomings:

1. The hierarchy of types is ignored. For example, if the type represents the type of ArrayList, then isTypeOf(Collection.class, type) will return false.
2. If the type is of TypeVariable type, IllegalArgumentException is thrown.
3. If the type is of Wildcard type, IllegalArgumentException is thrown.

The new generalized implementation addresses these shortcomings.

Now, the first shortcoming is no longer an issue (which I find very helpful). And for the other two shortcomings, I had experiences that I just wanted to check the type arguments passed to a class or method. With the current method, a casting and extracting the type is required, and it also breaks the flow of writing an stream().

The Exact Type (Previous Behavior)

If the previous behavior for checking the exact type is required, MoreTypes.isExactTypeOf() can be used.

Naming, and Backward Compatibility

The yes-instances of the old implementation are included in the yes-instances of the new implementation. For the no-instances, I believe the usage of isTypeOf() to mean the exact same type should be extremely rare. For this reason, I do not think backward compatibility should be an issue. However, if it is, instead of renaming the old implementation as isExactTypeOf() we can leave it as is and give a new name for the generalized method. In this case please feel free to suggest a name. As one suggestion maybe isHierarchicalTypeOf() (?).

Changes

Note: The few Javadoc changes are done by Google's formatting plugging for the Intellij.

1. First commit generalizes isTypeOf() and introduces isExactTypeOf(), and changes the corresponding unit-test present in MoreTypesTest.
2. SuperficialValidation might be one of those rare cases where the use of isExactTypeOf() is more appropriate. The second commit applies these changes.
3. In the third commit, the unit-tests for isTypeOf() are added to include tests for the new capabilities.

[eamonnmcmanus]

I'm not really sure about this.

First of all, I don't think we should change the meaning of isTypeOf. That could break existing code that is depending on the current meaning.

We could introduce a new method, say isAssignableTo. My main hesitation there is that we already have a bunch of methods in the Types interface that allow type queries like these and more, much more reliably than whatever we can cook up with visitors. The supposed advantage of the MoreTypes methods is that they avoid having to thread a Types instance through to where it is needed, but in my experience you end up doing that sooner or later anyway.

A further reason for hesitation is that methods like the existing isTypeOf and this new isAssignableTo (or whatever) blur the distinction between the annotation processor classpath and the compilation classpath. We've generally found that that leads to problems. For example we recommend against using Element.getAnnotation(Class), even though it is very convenient, because it does exactly this sort of blurring. An annotation processor for MyAnnotation should not need to have MyAnnotation on its own classpath.

I'd be interested in more details of your use cases, and also in other people's opinions here.

[AriaAdibi]

I think the name isSubtypeOf() should be a good alternative name.

Addressing your two concerns

Using Types instead

I have created the following test which helps with my demonstration here.

There is an interface TestType implemented as

private interface TestType {
    <RANDOM_ACCESS_LIST extends List<?> & RandomAccess> void method(RANDOM_ACCESS_LIST randomAccessList);
}

and the test is:

// ArrayList<String>
TypeMirror type = typeUtils.getDeclaredType(
    elementUtils.getTypeElement(ArrayList.class.getCanonicalName()),
    elementUtils.getTypeElement(String.class.getCanonicalName()).asType());
// Using MoreTypes.isTypeOf()
System.err.println(MoreTypes.isTypeOf(Collection.class, type));
// Using Types
System.err.println(
    typeUtils.isSubtype(
        type,
        typeUtils.erasure(elementUtils.getTypeElement(Collection.class.getCanonicalName()).asType()))
);

System.err.println("*************");

// ? extends SortedMap
type = typeUtils.getWildcardType(elementUtils.getTypeElement(SortedMap.class.getCanonicalName()).asType(), null);
// Using MoreTypes.isTypeOf()
System.err.println(MoreTypes.isTypeOf(Map.class, type));
// Using Types
System.err.println(
    typeUtils.isSubtype(
        type,
        typeUtils.erasure(elementUtils.getTypeElement(Map.class.getCanonicalName()).asType()))
);

System.err.println("*************");

ExecutableElement executableElement = MoreElements.asExecutable(
    Iterables.getOnlyElement(
        elementUtils.getTypeElement(TestType.class.getCanonicalName()).getEnclosedElements()));
// RANDOM_ACCESS_LIST
type = Iterables.getOnlyElement(executableElement.getTypeParameters()).asType();
// Using MoreTypes.isTypeOf()
System.err.println(MoreTypes.isTypeOf(List.class, type));
// Using Types
System.err.println(
    typeUtils.isSubtype(
        type,
        typeUtils.erasure(elementUtils.getTypeElement(List.class.getCanonicalName()).asType()))
);

The output is:

true
true
*************
true
false
*************
true
true

Therefore, I believe using Types has two disadvantages:

1. The code seems cumbersome, not very readable and clear (at least at the first glance), and needs vigilance on the presence of generic types. Moreover, I personally feel when filtering in a stream the conciseness of the new implementation makes a significant difference in the code. Lastly, the new implementation throws IllegalArgumentException if the TypeMirror does not represent a type that can be referenced by a Class which is helpful information.
2. The checking of sub-type for wildcards does not work as intended in the new implementation (which I think is the more natural behavior).

Blurring the class-path and processor-path

This is an interesting point that I have never thought about. Do you have a ready example of the kind of problems that this may cause? I am interested to know.

My use cases

As requested few of the use cases that I have used so far are as follows.

Makes certain validations easier

For example, I wanted to make sure that every method in interface of type XInterfaceType has one identifying parameter.

/* Validate that every method has a unique identifier of type XIdentifier */
for (ExecutableElement interfaceMethod :
      ElementFilter.methodsIn(MoreTypes.asElement(XInterfaceType).getEnclosedElements())) {
    long nIdentifiers =
        interfaceMethod.getParameters().stream()
            .filter(param -> MoreTypes.isTypeOf(XIdentifier.class, param.asType()))
            .count();

    if (nIdentifiers != 1) {
        //Report error
    }
}

Different kinds of filtering

For example, I have the following @converter annotation:

@Retention(RetentionPolicy.SOURCE)
@Target(ElementType.FIELD)
public @interface Converter {
  Class<? extends IParameterConverter<?, ?>> value();
}

and I want to get the target type at some points. For this I have:

private TypeMirror getConversionTargetType(AnnotationMirror converterAnnotationMirror, DeclaredType converterClassType) {
    DeclaredType iParameterConverterType = MoreTypes.asDeclared(
        MoreElements.asType(converterClassType.asElement()).getInterfaces().stream()
            // isTypeOf() (not exact) to support nested interfaces
            .filter(type -> MoreTypes.isTypeOf(IParameterConverter.class, type))
            .findAny()
            .orElseThrow() // By static check of Javac (Class<? extends IParameterConverter<?, ?>>) it should exist.);

    return iParameterConverterType.getTypeArguments().get(1);
}

Or, as another example, for certain fields, I only wanted to output the Json related annotations, but excluding @JsonIgnore. I did:

valueField.addAnnotations(
    mappedElement.getAnnotationMirrors().stream()
        .filter(annotationMirror -> {
            DeclaredType annotType = annotationMirror.getAnnotationType();
            return annotType.toString().startsWith("com.fasterxml.jackson") &&
                !MoreTypes.isTypeOf(JsonIgnore.class, annotType);
            })
        .map(AnnotationSpec::get).collect(Collectors.toList()));

Checking if it is of type Collection

I have used this on many occasions. One example is that for some tasks I wanted the "actual" type as opposed to the collection-wrapped one. For this I have implemented this:

public TypeMirror removeCollectionWrappers(TypeMirror collectionWrappedType) {
    while (MoreTypes.isTypeOf(Collection.class, collectionWrappedType)) {
        List<? extends TypeMirror> typeArgs = Collections.unmodifiableList(
            MoreTypes.asDeclared(collectionWrappedType).getTypeArguments());
    
        if (typeArgs.isEmpty()) {
            // Error raw collection is not allowed
        }
    
        collectionWrappedType = Iterables.getOnlyElement(typeArgs);
    }
    
    return collectionWrappedType;
}

Checking type parameters

The last example is that I have the following service class:

public  class XService<D extends XDto, F extends XDtoFilter> extends XBaseService<D, F> implements IService<D, F> {
}

and for doing some checks on the XDto I have written a stream and used MoreType.isTypeOf() in it.