Skip to content

Crash on quick info on < in JSX closing tag #47446

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.

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

Closed
DanielRosenwasser opened this issue Jan 14, 2022 · 4 comments · Fixed by #47500
Closed

Crash on quick info on < in JSX closing tag #47446

DanielRosenwasser opened this issue Jan 14, 2022 · 4 comments · Fixed by #47500
Assignees
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Fix Available A PR has been opened for this issue

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 14, 2022

Likely to be #21815
Likely to be #46946

let x = <div>
    /*$*/</div >

Request quick info on the <.

Expected I dunno, probably nothing!
Actual

Failure. False expression: `JsxText` tokens should not be the first child of `JsxElement | JsxSelfClosingElement`
Error: Debug Failure. False expression: `JsxText` tokens should not be the first child of `JsxElement | JsxSelfClosingElement`
    at findRightmostChildNodeWithTokens ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124512:26)
    at find ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124486:29)
    at findPrecedingToken ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124436:22)
    at nodeContainsPosition ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124371:37)
    at [USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124328:21
    at Object.binarySearchKey ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:1187:21)
    at _loop_1 ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124303:24)
    at getTokenAtPositionWorker ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124353:27)
    at getTouchingToken ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124288:16)
    at Object.getTouchingPropertyName ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:124280:16)
    at Object.getQuickInfoAtPosition ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:160763:27)
    at IOSession.Session.getQuickInfoWorker ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:172179:62)
    at Session.handlers.ts.Map.ts.getEntries._a.<computed> ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:171070:61)
    at [USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:172931:88
    at IOSession.Session.executeWithRequestId ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:172922:28)
    at IOSession.Session.executeCommand ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:172931:33)
    at IOSession.Session.onMessage ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:172957:35)
    at Interface.<anonymous> ([USERDIR]\.vscode-insiders\extensions\ms-vscode.vscode-typescript-next-4.6.20220112\node_modules\typescript\lib\tsserver.js:175562:31)
    at Interface.emit (events.js:315:20)
    at Interface._onLine (readline.js:337:10)
    at Interface._normalWrite (readline.js:482:12)
    at Socket.ondata (readline.js:194:10)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
    at Socket.Readable.push (internal/streams/readable.js:223:10)
    at Pipe.onStreamRead (internal/stream_base_commons.js:188:23)

Fourslash test

// @Filename: foo.tsx
////let x = <div>
////    /*$*/</div >

goto.marker("$");
verify.not.quickInfoExists();
@jakebailey
Copy link
Member

This assertion seems to be incorrect; the children being searched in findRightmostChildNodeWithTokens are the children of a SyntaxList, not an element. That SyntaxList is in fact the second child of the element, i.e. the interior, which is where a JsxText is supposed to be (and what the assertion is testing).

#16385 added this check; I don't quite know when SyntaxList was introduced or if this check can just go away. Probably fixable by just verifying that the parent type is one of the two in the message.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Jan 19, 2022

Don't quote me on this, but I believe SyntaxList functions in two ways.

First, a SyntaxList is supposed to normalize how Node.prototype.getChildren() works because NodeArrays aren't proper Nodes. For example, a Block just has a NodeArray of statements.

If you want everything in getChildren() to have a pos, end, etc., you need some other object. So SyntaxList is a synthesized Node. It shares the same prototype as a Node, has a kind field etc. The second function is that a SyntaxList also contains any hypothetical tokens that might have ended up in the tree that don't belong to any children in the original NodeArray, and I assume that in some cases, that can be important to handle.

Maybe @CyrusNajmabadi remembers more.

@jakebailey
Copy link
Member

jakebailey commented Jan 19, 2022

From my point of view, the debug assertion could only be there to double check some invariant of the parse tree.

It could be argued that it should also check that the JsxText node wasn't the last child of a JsxElement, for example, or that a JsxSelfClosingElement doesn't contain a JsxText child (as they shouldn't contain any text).

My PR tries to restore the documented invariant of the check, but I can certainly make the argument that it doesn't have to be there at all, too.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Jan 19, 2022

I think you're right, but that the invariant isn't quite catching what it intends. I think that this all stems from some edge condition trying to make sure trivia wasn't accidentally getting mixed up with JsxText or something, but franky I'm not sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants