fix #3859 UnsupportedOperationException when trying to resolve a type… #3873
Conversation
…lve a type in an argument javaparser#3859
| * first Node that returns make {@code isAcceptedParentNode} evaluate to | ||
| * {@code true}. | ||
| */ | ||
| public static Node demandParentNode(Node node, Predicate<Node> isAcceptedParentNode) { |
There was a problem hiding this comment.
This can be done by recursion instead of using a loop.
There was a problem hiding this comment.
Hi @jlerbsc, you are right, it can be done by recursion.
However I would prefer to keep it in the loop form, as it is easy to understand and straight forward. Using recursion in a functional language is typically a good idea (and sometimes even necessary). However I would use recursion in Java only when it provides a better solution than the imperative solution, especially as the Java runtime does not provide recursion-related optimisations (like tail call elimination) to make recursion comparable to loops performance-wise. Also taking Java's stack architecture into account using recursion typically requires additional thinking if this recursion may not potentially lead to a stack overflow (not in our case, but think of a loop with 100000 iterations replaced with an equivalent recursive implementation).
If you think this method must get a recursive implementation please explain where you see the benefits. Otherwise I would leave the method as it is
| * Returns {@code true} when the Node to be tested is not an | ||
| * {@link EnclosedExpr}, {@code false} otherwise. | ||
| */ | ||
| private static final Predicate<Node> IS_NOT_ENCLOSED_EXPR = n -> !(n instanceof EnclosedExpr); |
There was a problem hiding this comment.
if n is an expression you can write n.isEnclosedExpr() instead of using instanceof.
There was a problem hiding this comment.
This won't work.
Assume I change the implementation to
private static final Predicate<Expression> IS_NOT_ENCLOSED_EXPR = n -> !(n.isEnclosedExpr());
As the Predicate is used as a parameter to the new demandParentNode(Node, Predicate<Node>) it would require me to change that method to demandParentNode(Node, Predicate<Expression>). However in the implementation of demandParentNode we assume the result of a demandParentNode call is tested against the predicate , and that result is a Node.
Or did I missed something.
| @@ -255,6 +263,9 @@ protected final Optional<Value> solveWithAsValue(SymbolDeclarator symbolDeclarat | |||
| private int pos(MethodCallExpr callExpr, Expression param) { | |||
| int i = 0; | |||
| for (Expression p : callExpr.getArguments()) { | |||
| while (p instanceof EnclosedExpr) { | |||
| p = ((EnclosedExpr) p).getInner(); | |||
There was a problem hiding this comment.
Here p is an expression so you can write n.isEnclosedExpr() and use p.asEnclosedExpr() instead of casting the expression.
There was a problem hiding this comment.
Good idea. Will change this to
while (p.isEnclosedExpr()) {
p = p.asEnclosedExpr().getInner();
}
| if (call.getArguments().get(i) == node) return i; | ||
| Expression expression = call.getArguments().get(i); | ||
| while (expression instanceof EnclosedExpr) { | ||
| expression = ((EnclosedExpr) expression).getInner(); |
There was a problem hiding this comment.
Here argument "expression" is an expression so you can write n.isEnclosedExpr() and use p.asEnclosedExpr() instead of casting the expression.
| " foo((s->print(s)));\n" + | ||
| " }\n" + | ||
| "}\n"; | ||
| ParserConfiguration configuration = new ParserConfiguration() |
There was a problem hiding this comment.
For some time this can be written more simply
CompilationUnit cu = JavaParserAdapter.of(createParserWithResolver(defaultTypeSolver())).parse(code);
There was a problem hiding this comment.
Nice, I didn't know that.
Will change accordingly
This fixes the issue #3859
This commit contains just the fix. Additional refactorings are planned for a later commit/PR.