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.

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

Clone node to remove location even when it has been modified if needed #58706

Merged
merged 6 commits into from
May 31, 2024
Merged

Clone node to remove location even when it has been modified if needed #58706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e85543f
Clone node to remove location even when it has been modified if needed
weswigham May 29, 2024
1d27384
Merge branch 'main' into textrange-to-original-node
weswigham May 30, 2024
a8f73f8
Accept baselines
weswigham May 30, 2024
9927a3c
Comment & small cleanup
weswigham May 31, 2024
e84e554
Merge branch 'main' into textrange-to-original-node
weswigham May 31, 2024
9400b2e
Tack locations onto copied type parameter names explicitly
weswigham May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
70 changes: 55 additions & 15 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ import {
ScriptKind,
ScriptTarget,
SetAccessorDeclaration,
setCommentRange,
setCommentRange as setCommentRangeWorker,
setEmitFlags,
setIdentifierTypeArguments,
setNodeFlags,
Expand Down Expand Up @@ -1094,7 +1094,7 @@ import {
VariableLikeDeclaration,
VariableStatement,
VarianceFlags,
visitEachChild,
visitEachChild as visitEachChildWorker,
visitNode,
visitNodes,
Visitor,
Expand Down Expand Up @@ -2426,7 +2426,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

function markAsSynthetic<T extends Node>(node: T): VisitResult<T> {
setTextRangePosEnd(node, -1, -1);
return visitEachChild(node, markAsSynthetic, /*context*/ undefined);
return visitEachChildWorker(node, markAsSynthetic, /*context*/ undefined);
}

function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken, skipDiagnostics?: boolean) {
Expand Down Expand Up @@ -6032,21 +6032,21 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
* Unlike the utilities `setTextRange`, this checks if the `location` we're trying to set on `range` is within the
* same file as the active context. If not, the range is not applied. This prevents us from copying ranges across files,
* which will confuse the node printer (as it assumes all node ranges are within the current file).
* Additionally, if `range` _isn't synthetic_, and isn't in the current file, it will _copy_ it to _remove_ its' position
* Additionally, if `range` _isn't synthetic_, or isn't in the current file, it will _copy_ it to _remove_ its' position
* information.
*
* It also calls `setOriginalNode` to setup a `.original` pointer, since you basically *always* want these in the node builder.
*/
function setTextRange<T extends Node>(context: NodeBuilderContext, range: T, location: Node | undefined): T {
if (!nodeIsSynthesized(range) && !(range.flags & NodeFlags.Synthesized) && (!context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(range))) {
range = factory.cloneNode(range);
if (!nodeIsSynthesized(range) || !(range.flags & NodeFlags.Synthesized) || !context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(getOriginalNode(range))) {
range = factory.cloneNode(range); // if `range` is synthesized or originates in another file, copy it so it definitely has synthetic positions
}
if (range === location) return range;
if (!location) {
return range;
}
if (!context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(getOriginalNode(location))) {
return setOriginalNode(range, location);
return setOriginalNode(range, location); // if `location` is from another file, only set/update original pointer, and not positions, since copying text across files isn't supported by the emitter
}
return setTextRangeWorker(setOriginalNode(range, location), location);
}
Expand Down Expand Up @@ -6720,7 +6720,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!nodeIsSynthesized(node) && getParseTreeNode(node) === node) {
return node;
}
return setTextRange(context, factory.cloneNode(visitEachChild(node, deepCloneOrReuseNode, /*context*/ undefined, deepCloneOrReuseNodes, deepCloneOrReuseNode)), node);
return setTextRange(context, factory.cloneNode(visitEachChildWorker(node, deepCloneOrReuseNode, /*context*/ undefined, deepCloneOrReuseNodes, deepCloneOrReuseNode)), node);
}

function deepCloneOrReuseNodes(
Expand Down Expand Up @@ -7067,6 +7067,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const getterSignature = getSignatureFromDeclaration(getterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(getterSignature, SyntaxKind.GetAccessor, context, { name: propertyName }) as GetAccessorDeclaration,
getterDeclaration,
),
Expand All @@ -7075,6 +7076,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const setterSignature = getSignatureFromDeclaration(setterDeclaration);
typeElements.push(
setCommentRange(
context,
signatureToSignatureDeclarationHelper(setterSignature, SyntaxKind.SetAccessor, context, { name: propertyName }) as SetAccessorDeclaration,
setterDeclaration,
),
Expand Down Expand Up @@ -7132,12 +7134,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
else if (propertySymbol.valueDeclaration) {
// Copy comments to node for declaration emit
setCommentRange(node, propertySymbol.valueDeclaration);
setCommentRange(context, node, propertySymbol.valueDeclaration);
}
return node;
}
}

function setCommentRange<T extends Node>(context: NodeBuilderContext, node: T, range: Node): T {
if (context.enclosingFile && context.enclosingFile === getSourceFileOfNode(range)) {
// Copy comments to node for declaration emit
return setCommentRangeWorker(node, range);
}
return node;
}

function mapToTypeNodes(types: readonly Type[] | undefined, context: NodeBuilderContext, isBareList?: boolean): TypeNode[] | undefined {
if (some(types)) {
if (checkTruncationLength(context)) {
Expand Down Expand Up @@ -7579,7 +7589,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (context.tracker.canTrackSymbol && isComputedPropertyName(node) && isLateBindableName(node)) {
trackComputedName(node.expression, context.enclosingDeclaration, context);
}
let visited = visitEachChild(node, elideInitializerAndSetEmitFlags, /*context*/ undefined, /*nodesVisitor*/ undefined, elideInitializerAndSetEmitFlags);
let visited = visitEachChildWorker(node, elideInitializerAndSetEmitFlags, /*context*/ undefined, /*nodesVisitor*/ undefined, elideInitializerAndSetEmitFlags);
if (isBindingElement(visited)) {
visited = factory.updateBindingElement(
visited,
Expand Down Expand Up @@ -7992,6 +8002,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!(result.kind & SyntaxKind.Identifier)) {
return factory.createIdentifier("(Missing type parameter)");
}
const decl = type.symbol?.declarations?.[0];
if (decl && isTypeParameterDeclaration(decl)) {
result = setTextRange(context, result, decl.name);
}
if (context.flags & NodeBuilderFlags.GenerateNamesForShadowedTypeParams) {
const rawtext = result.escapedText as string;
let i = context.typeParameterNamesByTextNextNameCount?.get(rawtext) || 0;
Expand Down Expand Up @@ -8393,7 +8407,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
name.symbol = sym!; // for quickinfo, which uses identifier symbol information
return setTextRange(context, setEmitFlags(name, EmitFlags.NoAsciiEscaping), node);
}
const updated = visitEachChild(node, c => attachSymbolToLeftmostIdentifier(c), /*context*/ undefined);
const updated = visitEachChildWorker(node, c => attachSymbolToLeftmostIdentifier(c), /*context*/ undefined);
if (updated !== node) {
setTextRange(context, updated, node);
}
Expand Down Expand Up @@ -8504,7 +8518,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// is set to build for (even though we are reusing the node structure, the position information
// would make the printer print invalid spans for literals and identifiers, and the formatter would
// choke on the mismatched positonal spans between a parent and an injected child from another file).
return result === node ? setTextRange(context, factory.cloneNode(result), node) : result;
return result ? setTextRange(context, result, node) : undefined;
}

function createRecoveryBoundary() {
Expand Down Expand Up @@ -8769,7 +8783,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
|| (isPropertySignature(node) && !node.type && !node.initializer)
|| (isParameter(node) && !node.type && !node.initializer)
) {
let visited = visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
let visited = visitEachChild(node, visitExistingNodeTreeSymbols);
if (visited === node) {
visited = setTextRange(context, factory.cloneNode(node), node);
}
Expand Down Expand Up @@ -8831,7 +8845,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

if (isTupleTypeNode(node) || isTypeLiteralNode(node) || isMappedTypeNode(node)) {
const visited = visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
const visited = visitEachChild(node, visitExistingNodeTreeSymbols);
const clone = setTextRange(context, visited === node ? factory.cloneNode(node) : visited, node);
const flags = getEmitFlags(clone);
setEmitFlags(clone, flags | (context.flags & NodeBuilderFlags.MultilineObjectLiterals && isTypeLiteralNode(node) ? 0 : EmitFlags.SingleLine));
Expand Down Expand Up @@ -8878,7 +8892,33 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

return visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
return visitEachChild(node, visitExistingNodeTreeSymbols);

function visitEachChild<T extends Node>(node: T, visitor: Visitor): T;
function visitEachChild<T extends Node>(node: T | undefined, visitor: Visitor): T | undefined;
function visitEachChild<T extends Node>(node: T | undefined, visitor: Visitor): T | undefined {
const nonlocalNode = !context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(node);
return visitEachChildWorker(node, visitor, /*context*/ undefined, nonlocalNode ? visitNodesWithoutCopyingPositions : undefined);
}

function visitNodesWithoutCopyingPositions(
nodes: NodeArray<Node> | undefined,
visitor: Visitor,
test?: (node: Node) => boolean,
start?: number,
count?: number,
): NodeArray<Node> | undefined {
let result = visitNodes(nodes, visitor, test, start, count);
if (result) {
if (result.pos !== -1 || result.end !== -1) {
if (result === nodes) {
result = factory.createNodeArray(nodes, nodes.hasTrailingComma);
}
setTextRangePosEnd(result, -1, -1);
}
}
return result;
}

function getEffectiveDotDotDotForParameter(p: ParameterDeclaration) {
return p.dotDotDotToken || (p.type && isJSDocVariadicType(p.type) ? factory.createToken(SyntaxKind.DotDotDotToken) : undefined);
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/2dArrays.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ 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; }
> : ^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^
> : ^^^ ^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^
>this.ships : Ship[]
> : ^^^^^^
>this : this
> : ^^^^
>ships : Ship[]
> : ^^^^^^
>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; }
> : ^^^^^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^
> : ^^^ ^^^^^^^^^^^^^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^ ^^^ ^ ^^^ ^^^ ^^^^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^ ^^^ ^^^ ^^^
>function (val) { return val.isSunk; } : (val: Ship) => boolean
> : ^ ^^^^^^^^^^^^^^^^^^
>val : Ship
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/FunctionPropertyAssignments6_es6.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
=== FunctionPropertyAssignments6_es6.ts ===
var v = { *<T>() { } }
>v : { ""<T>(): Generator<never, void, unknown>; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : ^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>{ *<T>() { } } : { ""<T>(): Generator<never, void, unknown>; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : ^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : <T>() => Generator<never, void, unknown>
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2 changes: 1 addition & 1 deletion tests/baselines/reference/MemberFunctionDeclaration7_es6.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ class C {

*foo<T>() { }
>foo : <T>() => Generator<never, void, unknown>
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
}
Loading