Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_parser): Adding syntax error for new A?.b() #3118

Merged
merged 13 commits into from
Sep 21, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,6 @@ if ((this ?? {}).#bar) { foo.bar; }

(undefined && this ?? {}).#bar;
(((typeof this) as string) || {}).#bar;
(new foo || {}).bar;
// (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
(foo() || {}).bar;
((foo || {}).bar() || {}).baz;
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ if ((this ?? {}).#bar) { foo.bar; }

(undefined && this ?? {}).#bar;
(((typeof this) as string) || {}).#bar;
(new foo || {}).bar;
// (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
(foo() || {}).bar;
((foo || {}).bar() || {}).baz;

Expand Down Expand Up @@ -2437,7 +2437,7 @@ Suggested fix: Change to an optional chain.
147 | - (undefined && this ?? {}).#bar;
147 | + (undefined && this)?.#bar;
148 148 | (((typeof this) as string) || {}).#bar;
149 149 | (new foo || {}).bar;
149 149 | // (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
150 150 | (foo() || {}).bar;


Expand All @@ -2457,27 +2457,7 @@ Suggested fix: Change to an optional chain.
147 147 | (undefined && this ?? {}).#bar;
148 | - (((typeof this) as string) || {}).#bar;
148 | + ((typeof this) as string)?.#bar;
149 149 | (new foo || {}).bar;
150 150 | (foo() || {}).bar;
151 151 | ((foo || {}).bar() || {}).baz;


```

```
warning[nursery/useOptionalChain]: Change to an optional chain.
┌─ nullishAndLogicalOr.ts:150:1
150 │ (new foo || {}).bar;
│ -------------------

Suggested fix: Change to an optional chain.
| @@ -147,6 +147,6 @@
146 146 |
147 147 | (undefined && this ?? {}).#bar;
148 148 | (((typeof this) as string) || {}).#bar;
149 | - (new foo || {}).bar;
149 | + new foo?.bar;
149 149 | // (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
150 150 | (foo() || {}).bar;
151 151 | ((foo || {}).bar() || {}).baz;

Expand All @@ -2495,7 +2475,7 @@ Suggested fix: Change to an optional chain.
| @@ -148,5 +148,5 @@
147 147 | (undefined && this ?? {}).#bar;
148 148 | (((typeof this) as string) || {}).#bar;
149 149 | (new foo || {}).bar;
149 149 | // (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
150 | - (foo() || {}).bar;
150 | + foo()?.bar;
151 151 | ((foo || {}).bar() || {}).baz;
Expand All @@ -2513,7 +2493,7 @@ warning[nursery/useOptionalChain]: Change to an optional chain.
Suggested fix: Change to an optional chain.
| @@ -149,4 +149,4 @@
148 148 | (((typeof this) as string) || {}).#bar;
149 149 | (new foo || {}).bar;
149 149 | // (new foo || {}).bar; // tracked here https://github.com/rome/tools/issues/3257
150 150 | (foo() || {}).bar;
151 | - ((foo || {}).bar() || {}).baz;
151 | + foo?.bar()?.baz;
Expand Down
17 changes: 17 additions & 0 deletions crates/rome_js_parser/src/syntax/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,23 @@ fn parse_new_expr(p: &mut Parser, context: ExpressionContext) -> ParsedSyntax {
.or_add_diagnostic(p, expected_expression)
.map(|expr| parse_member_expression_rest(p, expr, context, false, &mut false))
{
// test_err ts invalid_optional_chain_from_new_expressions
// new Test<string>?.test();
// new Test?.test();
// new A.b?.c()
// new (A.b)?.c()
// new (A.b?.()).c()
// new A.b?.()()
if p.at(T![?.]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the error checking here because I don't want to clone the lhs.text(p). If put this branch here, https://github.com/rome/tools/pull/3118/files#diff-1ca6d1ff770e933910c550807255d1bf03cdebbf60bf469579b528c0c28c1537R797, you can't return a &str of lhs.text(p) because lhs is moved via lhs.undo_completion(p)

let error = p
.err_builder("Invalid optional chain from new expression.")
.primary(
p.cur_range(),
&format!("Did you mean to call '{}()'?", lhs.text(p)),
);

p.error(error);
}
if let TS_INSTANTIATION_EXPRESSION = lhs.kind() {
lhs.undo_completion(p).abandon(p)
};
Expand Down
Loading