Skip to content
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

Underlines in baselines should not consider nodes with .original set synthetic #58086

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Apr 5, 2024

This is the churny bit of #58085, but the change itself is small.

This just changes our baselining requirements for the underlines to include any node with a .original pointer as not-synthetic. We already set this on most symbol name positions to the symbol's original declaration, even if we didn't copy exact position information (which we have good reason to not do at times, it turns out, and I'll have a followup related to - if those positions are from another file, we really shouldn't copy them, since it'll cause us to lookup gibberish for literal and identifier text (because we'll be looking in the wrong file for the span!)).

And while I'm here, let me head off the obvious question: yes, there's a scale of "how synthetic a node is", and there's a specific point on that scale we care about for these baselines. Any node not made in the parser gets NodeFlags.Synthesized set. That'll be every node the node builder produces - much too large a set. What we want to underline are nodes which have zero relationship to any input node. A node having a text range set is a strong relationship to an input span (ts.nodeIsSynthesized returns true if a node has no text range set), but isn't the only indicator, and may not always be set - a node having a .original set also indicates that it's related to an input node (and also might not always be set, though I'm struggling to see why you wouldn't set it and would only set the text span of a node, AFAIK the primary use of a .original pointer is to enable using checker APIs with the manufactured node).

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 5, 2024
@@ -35,17 +35,17 @@ class Board {
>this.ships.every(function (val) { return val.isSunk; }) : boolean
> : ^^^^^^^
>this.ships.every : { <S extends Ship>(predicate: (value: Ship, index: number, array: Ship[]) => value is S, thisArg?: any): this is S[]; (predicate: (value: Ship, index: number, array: Ship[]) => unknown, thisArg?: any): boolean; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : ^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^
Copy link
Member Author

Choose a reason for hiding this comment

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

These are great example - these parameters all come from Array.prototype.every, which is declared in the lib (obviously), so while we can copy the parameter name nodes to create this signature node, we can't copy the position on the nodes, since that would try and pull gibberish text from this file, if it didn't crash for violating some invariant somewhere.

@weswigham weswigham closed this Apr 5, 2024
@weswigham weswigham reopened this Apr 5, 2024
@weswigham weswigham merged commit a2d37a5 into microsoft:main Apr 5, 2024
46 checks passed
@weswigham weswigham deleted the underlines-do-not-consider-nodes-with-originals-synthetic branch April 5, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants