Repository navigation
Underlines in baselines should not consider nodes with .original set synthetic - #58086
Merged
Wesley Wigham (weswigham) merged 2 commits intoApr 5, 2024
Conversation
Wesley Wigham (weswigham)
requested review from
Jake Bailey (jakebailey) and
Ron Buckton (rbuckton)
April 5, 2024 00:44
TypeScript Bot (typescript-bot)
added
Author: Team
For Uncommitted Bug
| > : ^^^^^^^ | ||
| >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; } | ||
| > : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| > : ^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^ |
Member
Author
There was a problem hiding this comment.
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.
Jake Bailey (jakebailey)
approved these changes
Apr 5, 2024
Wesley Wigham (weswigham)
deleted the
underlines-do-not-consider-nodes-with-originals-synthetic
branch
April 5, 2024 16:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
.originalpointer 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.Synthesizedset. 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.nodeIsSynthesizedreturns 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.originalset 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.originalpointer is to enable using checker APIs with the manufactured node).