[GROOVY-4156] [greclipse] Can we make ClassNode a less leaky abstraction?

• Type: Improvement
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: 1.7.2
• Component/s: Compiler
• Assignee: Unassigned
• Reporter: Andy Clement
• Created: 08/Apr/10 19:56
• Updated: 17/Aug/23 20:35

Description

Currently in Groovy I believe a ClassNode can represent two kinds of thing. Either something being compiled or something loaded from a .class file.
Groovy-Eclipse introduces a third kind of thing where the ClassNode represents something loaded by JDT from a project in eclipse.

Unfortunately throughout groovy there are calls to getTypeClass() on a ClassNode which returns the underlying .class loaded Class object, if there is one. This does not work so well when there are three kinds of entity - it can't be assumed if it has no type class then it is an entity from source, and vice versa. And in many cases eclipse would rather represent types via their JDT form instead of loading the class files. (For example, even core types like 'Annotation' in groovy eclipse are represented by a JDT backed ClassNode)

It looks like a reliance on getTypeClass() has been built because the ClassNode doesn't support all the necessary methods to answer all questions. I'd like to think about changing that - remove getTypeClass() (or at least make it less commonly used...) and promote any questions that get asked of the returned value of that call to being real methods on the ClassNode. This would enable the underlying 'thing' behind the ClassNode to answer the question.

As an example, here is some code in ExtendedVerifier.visitAnnotations(AnnotatedNode node, int target):

    for (AnnotationNode unvisited : node.getAnnotations()) {
        AnnotationNode visited = visitAnnotation(unvisited);
        boolean isTargetAnnotation = 
            visited.getClassNode().isResolved() &&
            visited.getClassNode().getTypeClass() == Target.class;

and each time I put a new version of groovy into groovy eclipse I have to change it to

    String jlaTarget = "java.lang.annotation.Target";
...
    for (AnnotationNode unvisited : node.getAnnotations()) {
        AnnotationNode visited = visitAnnotation(unvisited);
        boolean isTargetAnnotation = 
            visited.getClassNode().isResolved() &&
            visited.getClassNode().getName().equals(jlaTarget);

that is a case where I just have to avoid the call to getTypeClass(). Classes like ASTTransformationCollectorCodeVisitor have much more radical changes due to calls like this:

    for (Annotation ann : annotatedType.getTypeClass().getAnnotations()) {

Another example ImmutableASTTransformation is changed from:

private boolean isKnownImmutable(ClassNode fieldType) {
    if (!fieldType.isResolved()) return false;
    Class typeClass = fieldType.getTypeClass();
    return typeClass.isEnum() ||
        typeClass.isPrimitive() ||
        inImmutableList(typeClass);
}

to

private boolean isKnownImmutable(ClassNode fieldType) {
    if (!fieldType.isResolved()) return false;
    String s= fieldType.getName();
    return fieldType.isPrimitive() || fieldType.isEnum() || inImmutableList(fieldType.getName());
}

(so, yes, my ClassNode has an isPrimitive() method added)

If you are amenable to it, I can contribute back a few patches here and there to remove the usages of getTypeClass()

Issue Links

• is related to: NETBEANS-5982 Groovy parser cannot handle annotations from source
• relates to: GROOVY-11159 ClassNode: separate type definition information