[apex] ExcessiveClassLength multiple warning on the same class #3156
Conversation
Generated by 🚫 Danger |
| @@ -191,8 +228,6 @@ private int visitPosition(Node node, int count) { | |||
| return result; | |||
| } | |||
|
|
|||
| // TEST HELPER | |||
|
|
|||
| private static void assertPosition(Node node, int beginLine, int beginColumn, int endLine, int endColumn) { | |||
There was a problem hiding this comment.
Just something to care about while merging into pmd 7, this method has been moved to pmd-lang-test IIRC
| // this is a synthetic method, only in the AST, not in the source | ||
| // search for the last sibling with real location from the end | ||
| // and place this synthetic method after it. | ||
| for (int i = getParent().getNumChildren() - 1; i >= 0; i--) { |
There was a problem hiding this comment.
So synthetic methods have the coordinates of the last real method of the class. This makes sense, but this may break some assumptions we make on line/column positions. Namely that if a node is-before another in a prefix traversal of the tree, then their positions are also <=. The designer relies on this for instance.
(I think those assumptions are already broken by the apex module anyway, because the designer's node finding methods malfunction)
Maybe we should move those synthetic nodes around when we do the traversal that resolve text coordinates (though since it's already broken we could do that later).
There was a problem hiding this comment.
I'd prefer to do that later. It would be useful to have an explicit test for the condition that the positions of a node is always <= than the position of the next node. Don't we have some test like that in pmd7?
About reordering the nodes: Not sure if we should do that. Currently I think, we use the visit order of the apex jorje parser.
So maybe we should make sure, that the condition is true no matter where the synthetic methods are. That means, we would place them just after the previous (real) sibling and before the next (real) sibling, if there is a next.
Not perfect, but better than before. The classes at least look ok, but the synthetic methods/nodes (like UserClassMethods, BridgeMethodCreator) look still weird.
Ready?
./mvnw clean verifypasses (checked automatically by travis)