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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
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...
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
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
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.
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
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?
https://sourceforge.net/tracker/?func=detail&aid=2892980&group_id=56262&ati
d=479921
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
I submitted a patch in the bug linked above.