Fixing parsing of method invocations in Javascript #5484
Conversation
|
@greg-at-moderne Did we decide what to do about this? |
|
We have not, forgot about this one. It was several weeks ago. |
rewrite-javascript/src/main/java/org/openrewrite/javascript/tree/JS.java
Outdated
Show resolved
Hide resolved
rewrite-javascript/src/main/java/org/openrewrite/javascript/tree/JS.java
Outdated
Show resolved
Hide resolved
rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java
Outdated
Show resolved
Hide resolved
rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java
Outdated
Show resolved
Hide resolved
| if (functionCall.function) { | ||
| await this.visitRightPadded(functionCall.function, p); | ||
| if (functionCall.function.element.markers.markers.find(m => m.kind === JS.Markers.Optional)) { | ||
| p.append("?."); |
There was a problem hiding this comment.
It would be nice if we could detangle this a bit and have the ? be printed by the presence of the marker and then only here print the .. Would that make sense?
There was a problem hiding this comment.
I had it like you suggest before in my code, but then the Space handling is wrong:
- const result0 = func/*a*/?./*b*/("TS");
+ const result0 = func?/*a*/./*b*/("TS");For that to work, the /*a*/ comment would need to land as Optional.prefix.
That in turn would make it inconsistent with how the /*a*/ comment is placed in other similar syntaxes.
See call.test.ts / with optional chaining operator.
I suggest to keep it like it is. After all the ?. is a single token with its own meaning, thus it feels OK to print it in one go.
Wdyt?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
What's changed?
Fixing parsing of method and other function invocations in Javascript. Currently
console.log("something")gets parsed asMethodInvocation(select=FieldAccess(console.log), name="")which is understandably not desired.The design decision has been to:
J.MethodInvocationJS.FunctionCallWhat's your motivation?
Avoiding wrong parsing. And providing reasonable level of compatibility with Java parsing.