Menu

SimpleJavaTypeNode.getType() null for generic

Developers
Nick Radov
2009-11-05
2012-10-07
  • Nick Radov

    Nick Radov - 2009-11-05

    I would like to write a custom rule that will generate a warning for the use
    of java.util.HashSet with an Enum generic type parameter. So for example, this
    line should generate a warning.

    java.util.Set<java.util.formatter.bigdecimallayoutform> set
    = new java.util.HashSet<java.util.formatter.bigdecimallayoutform>(); </java.util.formatter.bigdecimallayoutform></java.util.formatter.bigdecimallayoutform>

    In my custom rule Java class I can get the ASTClassOrInterfaceType object
    representing the HashSet. And then I can get the first child
    ASTClassOrInterfaceType which represents the generic type parameter. The
    problem is that calling ASTClassOrInterfaceType.getType() for the generic type
    parameter returns null instead of the BigDecimalLayoutForm class. The
    documentation for SimpleJavaTypeNode.getType() states that it may return null
    but doesn't say why. How can I find the actual Java class of a generic type
    parameter?

     
  • Ryan Gustafson

    Ryan Gustafson - 2009-11-06

    The reason is because PMD does not implement perfect type resolution. If it
    cannot figure out a type, then it is null.

    Now, tested the example code you provide. The calls in PMD one needs to look
    at is ClassTypeResolver, which is the one that visits the AST and annotates
    TypeNodes. There are two problems.

    1. The visit method for ASTClassOrInterfaceType fails to call super.visit(). The type parameters for the Set/HashSet types happen to be children of that node, and thus fail to get visited. Assuming this is fixed...
    2. The populateType method is not able to figure out that 'java.util.Formatter.BigDecimalLayoutForm' is actually 'java.util.Formatter$BigDecimalLayoutForm'. I'm not sure how to quick fix this in a fashion that doesn't kill type resolution performance. In general resolving an arbitrary name to it's appropriate thing is non-trivial.

    The type resolution stuff needs a fair bit of work to be 100%. It is
    interrelated with the symbol table as well. No clue when (or if) I'll ever get
    around to working on it. I've posted about this before.

    You're welcome to take a crack at an incremental improvement to
    ClassTypeResolver to address the above. We accept patches (assuming they have
    no issues).

    Ryan

     
  • Nick Radov

    Nick Radov - 2009-11-06

    Thanks for the explanation. Regarding problem 1, I see that the
    ClassTypeResolver.visit(ASTClassOrInterfaceType, Object) method does not call
    super.visit. I would be happy to fix that and submit a patch, however there
    seem to be a number of inconsistencies in all of the visit methods. Some call
    super.visit first and later return data, others call super.visit last as the
    return value, and some don't call super.visit at all. Is that inconsistency
    intentional, or should they all be the same? If they should be the same, which
    is the preferred structure?

     
  • Ryan Gustafson

    Ryan Gustafson - 2009-11-06

    A 10 second scan doesn't turn up any more missing super.visit calls.
    ASTImportDeclaration does not because it directly processes it's one possible
    ASTName child (see the Java.jjt grammar).

    The return value of the visitor here isn't really used, it's merely the style
    that one either does 'return super.visit()', or 'return data'. Technically a
    visitor can do anything it wants.

    For some nodes the order of the super.visit() call does matter, and generally
    there should always be one, unless a node never has any children (see the
    Java.jjt) such as ASTBooleanLiteral, even though it does here it's technically
    not needed. You'll notice the 'rollup' methods, which are called after the
    super.visit(). This is because they attempt to place a type on a node based
    upon the types on children node. To do that, one must first visit the
    children. Look at the rollup methods/test cases and you'll get the idea. If
    there is not dependency on children, then it does not matter when
    super.visit() is called.

    If you think you see a specific problem, ask away. Otherwise, things seem to
    be okay.

    Feel free to submit a patch for ClassTypeResolver for the ImportDeclaration
    issue. It's a 1 line fix. Someone should be able to apply it. I'd fix it, but
    I'm not configured at the moment.

    Ryan

     
  • Nick Radov

    Nick Radov - 2009-11-06

    I submitted a patch in the bug linked above.

     

Log in to post a comment.