Skip to content

Fixed false positive syntax errors with in inside for #54801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Fixed false positive syntax errors with in inside for #54801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7466,15 +7466,15 @@ namespace Parser {
function parseObjectBindingPattern(): ObjectBindingPattern {
const pos = getNodePos();
parseExpected(SyntaxKind.OpenBraceToken);
const elements = parseDelimitedList(ParsingContext.ObjectBindingElements, parseObjectBindingElement);
const elements = allowInAnd(() => parseDelimitedList(ParsingContext.ObjectBindingElements, parseObjectBindingElement));
parseExpected(SyntaxKind.CloseBraceToken);
return finishNode(factory.createObjectBindingPattern(elements), pos);
}

function parseArrayBindingPattern(): ArrayBindingPattern {
const pos = getNodePos();
parseExpected(SyntaxKind.OpenBracketToken);
const elements = parseDelimitedList(ParsingContext.ArrayBindingElements, parseArrayBindingElement);
const elements = allowInAnd(() => parseDelimitedList(ParsingContext.ArrayBindingElements, parseArrayBindingElement));
parseExpected(SyntaxKind.CloseBracketToken);
return finishNode(factory.createArrayBindingPattern(elements), pos);
}
Expand Down
20 changes: 20 additions & 0 deletions tests/baselines/reference/parserForInStatement8.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
parserForInStatement8.ts(3,10): error TS2461: Type 'string' is not an array type.
parserForInStatement8.ts(3,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
parserForInStatement8.ts(4,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
Comment on lines +2 to +3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those errors are not correct but that's a separate issue

Copy link
Member

Choose a reason for hiding this comment

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

How challenging is that to fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm looking into this but it's the first time that I even opened the files related to downleveling syntax here so it might take a moment (if I don't lose interest in this, that is).

  1. the error has to be likely removed completely (I removed it from one location but there is another one and I didn't even look into it yet)
  2. this undefined shouldn't be emitted (at least in this case)
  3. and the declaration list can't be "replaced" in place, since downleveling binding patterns requires creating temp bindings and for-in only supports a single binding (Uncaught SyntaxError: Invalid left-hand side in for-in loop: Must have a single binding.)
  4. to fix 2 and 3 it's likely the easiest to move the binding "expansion" to the loop's body, this is what Babel does:
for (var _ref3 in {
  '': 0
}) {
  var _ref2 = _toArray(_ref3);
  var _ref2$ = _ref2[0];
  var x = _ref2$ === void 0 ? 'a' in {} : _ref2$;
  var kaka = _ref2[1];
  var rest = _ref2.slice(2);
  console.log(x);
}

Copy link

Choose a reason for hiding this comment

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

It’s legal to use destructuring in for..in? What would that even mean since the value is always a string (or symbol)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strings are iterable so u could read its characters through an array pattern, or some prototype methods/properties through an object pattern.

Is it useful? Not rly. Is it legal? Apparently yes :P

Copy link
Member

Choose a reason for hiding this comment

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

Just close the loop on this one, if you don't think you'll get around to fixing this in this PR, can you add a note to the test case that says "hey, this is wrong" and file an issue, or similar?

Copy link
Member

Choose a reason for hiding this comment

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

Er, actually, is #54853 that PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is.

Gif saying 'You taught me that, Saul. No loose ends'

Copy link
Member

Choose a reason for hiding this comment

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

Alright, I guess we can just leave the PR as-is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i think so.

parserForInStatement8.ts(4,11): error TS2339: Property 'x' does not exist on type 'String'.


==== parserForInStatement8.ts (4 errors) ====
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] in { '': 0 }) console.log(x)
~~~~~~~~~~~~~~~
!!! error TS2461: Type 'string' is not an array type.
~~~~~~~~~~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
for (let {x = 'a' in {}} in { '': 0 }) console.log(x)
~~~~~~~~~~~~~~~
!!! error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern.
~
!!! error TS2339: Property 'x' does not exist on type 'String'.

15 changes: 15 additions & 0 deletions tests/baselines/reference/parserForInStatement8.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForInStatement8.ts] ////

//// [parserForInStatement8.ts]
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] in { '': 0 }) console.log(x)
for (let {x = 'a' in {}} in { '': 0 }) console.log(x)


//// [parserForInStatement8.js]
// repro from https://github.com/microsoft/TypeScript/issues/54769
for (var _a = (void 0)[0], x = _a === void 0 ? 'a' in {} : _a in { '': 0 })
console.log(x);
for (var _b = (void 0).x, x = _b === void 0 ? 'a' in {} : _b in { '': 0 })
console.log(x);
Comment on lines +12 to +15
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this isn't the correct output but since binding patterns on the left side of for-in are not supported by TS today, I think that it's fine to leave it as is for now

21 changes: 21 additions & 0 deletions tests/baselines/reference/parserForInStatement8.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForInStatement8.ts] ////

=== parserForInStatement8.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] in { '': 0 }) console.log(x)
>x : Symbol(x, Decl(parserForInStatement8.ts, 2, 10))
>'' : Symbol('', Decl(parserForInStatement8.ts, 2, 29))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForInStatement8.ts, 2, 10))

for (let {x = 'a' in {}} in { '': 0 }) console.log(x)
>x : Symbol(x, Decl(parserForInStatement8.ts, 3, 10))
>'' : Symbol('', Decl(parserForInStatement8.ts, 3, 29))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForInStatement8.ts, 3, 10))

33 changes: 33 additions & 0 deletions tests/baselines/reference/parserForInStatement8.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForInStatement8.ts] ////

=== parserForInStatement8.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] in { '': 0 }) console.log(x)
>x : any
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>{ '': 0 } : { '': number; }
>'' : number
>0 : 0
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : any

for (let {x = 'a' in {}} in { '': 0 }) console.log(x)
>x : any
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>{ '': 0 } : { '': number; }
>'' : number
>0 : 0
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : any

11 changes: 11 additions & 0 deletions tests/baselines/reference/parserForOfStatement25.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
parserForOfStatement25.ts(4,11): error TS2339: Property 'x' does not exist on type '{}'.


==== parserForOfStatement25.ts (1 errors) ====
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] of [[]]) console.log(x)
for (let {x = 'a' in {}} of [{}]) console.log(x)
~
!!! error TS2339: Property 'x' does not exist on type '{}'.

15 changes: 15 additions & 0 deletions tests/baselines/reference/parserForOfStatement25.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/conformance/parser/ecmascript6/Iterators/parserForOfStatement25.ts] ////

//// [parserForOfStatement25.ts]
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] of [[]]) console.log(x)
for (let {x = 'a' in {}} of [{}]) console.log(x)


//// [parserForOfStatement25.js]
// repro from https://github.com/microsoft/TypeScript/issues/54769
for (let [x = 'a' in {}] of [[]])
console.log(x);
for (let { x = 'a' in {} } of [{}])
console.log(x);
19 changes: 19 additions & 0 deletions tests/baselines/reference/parserForOfStatement25.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/parser/ecmascript6/Iterators/parserForOfStatement25.ts] ////

=== parserForOfStatement25.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] of [[]]) console.log(x)
>x : Symbol(x, Decl(parserForOfStatement25.ts, 2, 10))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForOfStatement25.ts, 2, 10))

for (let {x = 'a' in {}} of [{}]) console.log(x)
>x : Symbol(x, Decl(parserForOfStatement25.ts, 3, 10))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForOfStatement25.ts, 3, 10))

31 changes: 31 additions & 0 deletions tests/baselines/reference/parserForOfStatement25.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/conformance/parser/ecmascript6/Iterators/parserForOfStatement25.ts] ////

=== parserForOfStatement25.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] of [[]]) console.log(x)
>x : boolean
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>[[]] : undefined[][]
>[] : undefined[]
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : boolean

for (let {x = 'a' in {}} of [{}]) console.log(x)
>x : any
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>[{}] : {}[]
>{} : {}
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : any

15 changes: 15 additions & 0 deletions tests/baselines/reference/parserForStatement9.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForStatement9.ts] ////

//// [parserForStatement9.ts]
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] = []; !x; x = !x) console.log(x)
for (let {x = 'a' in {}} = {}; !x; x = !x) console.log(x)


//// [parserForStatement9.js]
// repro from https://github.com/microsoft/TypeScript/issues/54769
for (var _a = [][0], x = _a === void 0 ? 'a' in {} : _a; !x; x = !x)
console.log(x);
for (var _b = {}.x, x = _b === void 0 ? 'a' in {} : _b; !x; x = !x)
console.log(x);
25 changes: 25 additions & 0 deletions tests/baselines/reference/parserForStatement9.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForStatement9.ts] ////

=== parserForStatement9.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] = []; !x; x = !x) console.log(x)
>x : Symbol(x, Decl(parserForStatement9.ts, 2, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 2, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 2, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 2, 10))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForStatement9.ts, 2, 10))

for (let {x = 'a' in {}} = {}; !x; x = !x) console.log(x)
>x : Symbol(x, Decl(parserForStatement9.ts, 3, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 3, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 3, 10))
>x : Symbol(x, Decl(parserForStatement9.ts, 3, 10))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>x : Symbol(x, Decl(parserForStatement9.ts, 3, 10))

41 changes: 41 additions & 0 deletions tests/baselines/reference/parserForStatement9.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//// [tests/cases/conformance/parser/ecmascript5/Statements/parserForStatement9.ts] ////

=== parserForStatement9.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] = []; !x; x = !x) console.log(x)
>x : boolean
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>[] : []
>!x : boolean
>x : boolean
>x = !x : boolean
>x : boolean
>!x : boolean
>x : boolean
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : boolean

for (let {x = 'a' in {}} = {}; !x; x = !x) console.log(x)
>x : boolean
>'a' in {} : boolean
>'a' : "a"
>{} : {}
>{} : { x?: boolean; }
>!x : boolean
>x : boolean
>x = !x : boolean
>x : boolean
>!x : boolean
>x : boolean
>console.log(x) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>x : boolean

4 changes: 4 additions & 0 deletions tests/cases/conformance/parser/ecmascript5/Statements/parserForInStatement8.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] in { '': 0 }) console.log(x)
for (let {x = 'a' in {}} in { '': 0 }) console.log(x)
4 changes: 4 additions & 0 deletions tests/cases/conformance/parser/ecmascript5/Statements/parserForStatement9.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] = []; !x; x = !x) console.log(x)
for (let {x = 'a' in {}} = {}; !x; x = !x) console.log(x)
6 changes: 6 additions & 0 deletions tests/cases/conformance/parser/ecmascript6/Iterators/parserForOfStatement25.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @target: esnext

// repro from https://github.com/microsoft/TypeScript/issues/54769

for (let [x = 'a' in {}] of [[]]) console.log(x)
for (let {x = 'a' in {}} of [{}]) console.log(x)