Skip to content

Commit e85543f

Browse files
weswighamweswigham
committed
Clone node to remove location even when it has been modified if needed
1 parent 0cc62c3 commit e85543f

5 files changed

+202
-13
lines changed

‎src/compiler/checker.ts

+49-13
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ import {
964964
ScriptKind,
965965
ScriptTarget,
966966
SetAccessorDeclaration,
967-
setCommentRange,
967+
setCommentRange as setCommentRangeWorker,
968968
setEmitFlags,
969969
setIdentifierTypeArguments,
970970
setNodeFlags,
@@ -1091,7 +1091,7 @@ import {
10911091
VariableLikeDeclaration,
10921092
VariableStatement,
10931093
VarianceFlags,
1094-
visitEachChild,
1094+
visitEachChild as visitEachChildWorker,
10951095
visitNode,
10961096
visitNodes,
10971097
Visitor,
@@ -2422,7 +2422,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
24222422

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

24282428
function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken, skipDiagnostics?: boolean) {
@@ -6021,13 +6021,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
60216021
* Unlike the utilities `setTextRange`, this checks if the `location` we're trying to set on `range` is within the
60226022
* same file as the active context. If not, the range is not applied. This prevents us from copying ranges across files,
60236023
* which will confuse the node printer (as it assumes all node ranges are within the current file).
6024-
* Additionally, if `range` _isn't synthetic_, and isn't in the current file, it will _copy_ it to _remove_ its' position
6024+
* Additionally, if `range` _isn't synthetic_, or isn't in the current file, it will _copy_ it to _remove_ its' position
60256025
* information.
60266026
*
60276027
* It also calls `setOriginalNode` to setup a `.original` pointer, since you basically *always* want these in the node builder.
60286028
*/
60296029
function setTextRange<T extends Node>(context: NodeBuilderContext, range: T, location: Node | undefined): T {
6030-
if (!nodeIsSynthesized(range) && !(range.flags & NodeFlags.Synthesized) && (!context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(range))) {
6030+
if (!nodeIsSynthesized(range) || !(range.flags & NodeFlags.Synthesized) || !context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(range)) {
60316031
range = factory.cloneNode(range);
60326032
}
60336033
if (range === location) return range;
@@ -6709,7 +6709,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
67096709
if (!nodeIsSynthesized(node) && getParseTreeNode(node) === node) {
67106710
return node;
67116711
}
6712-
return setTextRange(context, factory.cloneNode(visitEachChild(node, deepCloneOrReuseNode, /*context*/ undefined, deepCloneOrReuseNodes, deepCloneOrReuseNode)), node);
6712+
return setTextRange(context, factory.cloneNode(visitEachChildWorker(node, deepCloneOrReuseNode, /*context*/ undefined, deepCloneOrReuseNodes, deepCloneOrReuseNode)), node);
67136713
}
67146714

67156715
function deepCloneOrReuseNodes(
@@ -7056,6 +7056,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
70567056
const getterSignature = getSignatureFromDeclaration(getterDeclaration);
70577057
typeElements.push(
70587058
setCommentRange(
7059+
context,
70597060
signatureToSignatureDeclarationHelper(getterSignature, SyntaxKind.GetAccessor, context, { name: propertyName }) as GetAccessorDeclaration,
70607061
getterDeclaration,
70617062
),
@@ -7064,6 +7065,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
70647065
const setterSignature = getSignatureFromDeclaration(setterDeclaration);
70657066
typeElements.push(
70667067
setCommentRange(
7068+
context,
70677069
signatureToSignatureDeclarationHelper(setterSignature, SyntaxKind.SetAccessor, context, { name: propertyName }) as SetAccessorDeclaration,
70687070
setterDeclaration,
70697071
),
@@ -7121,12 +7123,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
71217123
}
71227124
else if (propertySymbol.valueDeclaration) {
71237125
// Copy comments to node for declaration emit
7124-
setCommentRange(node, propertySymbol.valueDeclaration);
7126+
setCommentRange(context, node, propertySymbol.valueDeclaration);
71257127
}
71267128
return node;
71277129
}
71287130
}
71297131

7132+
function setCommentRange<T extends Node>(context: NodeBuilderContext, node: T, range: Node): T {
7133+
if (context.enclosingFile && context.enclosingFile === getSourceFileOfNode(range)) {
7134+
// Copy comments to node for declaration emit
7135+
return setCommentRangeWorker(node, range);
7136+
}
7137+
return node;
7138+
}
7139+
71307140
function mapToTypeNodes(types: readonly Type[] | undefined, context: NodeBuilderContext, isBareList?: boolean): TypeNode[] | undefined {
71317141
if (some(types)) {
71327142
if (checkTruncationLength(context)) {
@@ -7568,7 +7578,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
75687578
if (context.tracker.canTrackSymbol && isComputedPropertyName(node) && isLateBindableName(node)) {
75697579
trackComputedName(node.expression, context.enclosingDeclaration, context);
75707580
}
7571-
let visited = visitEachChild(node, elideInitializerAndSetEmitFlags, /*context*/ undefined, /*nodesVisitor*/ undefined, elideInitializerAndSetEmitFlags);
7581+
let visited = visitEachChildWorker(node, elideInitializerAndSetEmitFlags, /*context*/ undefined, /*nodesVisitor*/ undefined, elideInitializerAndSetEmitFlags);
75727582
if (isBindingElement(visited)) {
75737583
visited = factory.updateBindingElement(
75747584
visited,
@@ -8382,7 +8392,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
83828392
name.symbol = sym!; // for quickinfo, which uses identifier symbol information
83838393
return setTextRange(context, setEmitFlags(name, EmitFlags.NoAsciiEscaping), node);
83848394
}
8385-
const updated = visitEachChild(node, c => attachSymbolToLeftmostIdentifier(c), /*context*/ undefined);
8395+
const updated = visitEachChildWorker(node, c => attachSymbolToLeftmostIdentifier(c), /*context*/ undefined);
83868396
if (updated !== node) {
83878397
setTextRange(context, updated, node);
83888398
}
@@ -8480,7 +8490,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
84808490
// is set to build for (even though we are reusing the node structure, the position information
84818491
// would make the printer print invalid spans for literals and identifiers, and the formatter would
84828492
// choke on the mismatched positonal spans between a parent and an injected child from another file).
8483-
return result === node ? setTextRange(context, factory.cloneNode(result), node) : result;
8493+
return result ? setTextRange(context, result, node) : undefined;
84848494
}
84858495

84868496
function onEnterNewScope(node: IntroducesNewScopeNode | ConditionalTypeNode) {
@@ -8646,7 +8656,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
86468656
|| (isPropertySignature(node) && !node.type && !node.initializer)
86478657
|| (isParameter(node) && !node.type && !node.initializer)
86488658
) {
8649-
let visited = visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
8659+
let visited = visitEachChild(node, visitExistingNodeTreeSymbols);
86508660
if (visited === node) {
86518661
visited = setTextRange(context, factory.cloneNode(node), node);
86528662
}
@@ -8707,7 +8717,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
87078717
}
87088718

87098719
if (isTupleTypeNode(node) || isTypeLiteralNode(node) || isMappedTypeNode(node)) {
8710-
const visited = visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
8720+
const visited = visitEachChild(node, visitExistingNodeTreeSymbols);
87118721
const clone = setTextRange(context, visited === node ? factory.cloneNode(node) : visited, node);
87128722
const flags = getEmitFlags(clone);
87138723
setEmitFlags(clone, flags | (context.flags & NodeBuilderFlags.MultilineObjectLiterals && isTypeLiteralNode(node) ? 0 : EmitFlags.SingleLine));
@@ -8741,7 +8751,33 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
87418751
}
87428752
}
87438753

8744-
return visitEachChild(node, visitExistingNodeTreeSymbols, /*context*/ undefined);
8754+
return visitEachChild(node, visitExistingNodeTreeSymbols);
8755+
8756+
function visitEachChild<T extends Node>(node: T, visitor: Visitor): T;
8757+
function visitEachChild<T extends Node>(node: T | undefined, visitor: Visitor): T | undefined;
8758+
function visitEachChild<T extends Node>(node: T | undefined, visitor: Visitor): T | undefined {
8759+
const nonlocalNode = !context.enclosingFile || context.enclosingFile !== getSourceFileOfNode(node);
8760+
return visitEachChildWorker(node, visitor, /*context*/ undefined, nonlocalNode ? visitNodesWithoutCopyingPositions : undefined);
8761+
}
8762+
8763+
function visitNodesWithoutCopyingPositions(
8764+
nodes: NodeArray<Node> | undefined,
8765+
visitor: Visitor,
8766+
test?: (node: Node) => boolean,
8767+
start?: number,
8768+
count?: number,
8769+
): NodeArray<Node> | undefined {
8770+
let result = visitNodes(nodes, visitor, test, start, count);
8771+
if (result) {
8772+
if (result.pos !== -1 || result.end !== -1) {
8773+
if (result === nodes) {
8774+
result = factory.createNodeArray(nodes, nodes.hasTrailingComma);
8775+
}
8776+
setTextRangePosEnd(result, -1, -1);
8777+
}
8778+
}
8779+
return result;
8780+
}
87458781

87468782
function getEffectiveDotDotDotForParameter(p: ParameterDeclaration) {
87478783
return p.dotDotDotToken || (p.type && isJSDocVariadicType(p.type) ? factory.createToken(SyntaxKind.DotDotDotToken) : undefined);

‎tests/baselines/reference/declarationEmitTopLevelNodeFromCrossFile.js

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
//// [tests/cases/compiler/declarationEmitTopLevelNodeFromCrossFile.ts] ////
2+
3+
//// [a.ts]
4+
export type X = string;
5+
export const fn = { o: (a?: (X | undefined)[]) => {} };
6+
7+
//// [b.ts]
8+
import {fn} from "./a";
9+
export const m = {
10+
/**
11+
* leading doc for prop
12+
*/
13+
prop: 1
14+
}
15+
16+
17+
export const x = { p: fn };
18+
19+
//// [a.js]
20+
"use strict";
21+
Object.defineProperty(exports, "__esModule", { value: true });
22+
exports.fn = void 0;
23+
exports.fn = { o: function (a) { } };
24+
//// [b.js]
25+
"use strict";
26+
Object.defineProperty(exports, "__esModule", { value: true });
27+
exports.x = exports.m = void 0;
28+
var a_1 = require("./a");
29+
exports.m = {
30+
/**
31+
* leading doc for prop
32+
*/
33+
prop: 1
34+
};
35+
exports.x = { p: a_1.fn };
36+
37+
38+
//// [a.d.ts]
39+
export type X = string;
40+
export declare const fn: {
41+
o: (a?: (X | undefined)[]) => void;
42+
};
43+
//// [b.d.ts]
44+
export declare const m: {
45+
/**
46+
* leading doc for prop
47+
*/
48+
prop: number;
49+
};
50+
export declare const x: {
51+
p: {
52+
o: (a?: (import("./a").X | undefined)[]) => void;
53+
};
54+
};

‎tests/baselines/reference/declarationEmitTopLevelNodeFromCrossFile.symbols

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//// [tests/cases/compiler/declarationEmitTopLevelNodeFromCrossFile.ts] ////
2+
3+
=== a.ts ===
4+
export type X = string;
5+
>X : Symbol(X, Decl(a.ts, 0, 0))
6+
7+
export const fn = { o: (a?: (X | undefined)[]) => {} };
8+
>fn : Symbol(fn, Decl(a.ts, 1, 12))
9+
>o : Symbol(o, Decl(a.ts, 1, 19))
10+
>a : Symbol(a, Decl(a.ts, 1, 24))
11+
>X : Symbol(X, Decl(a.ts, 0, 0))
12+
13+
=== b.ts ===
14+
import {fn} from "./a";
15+
>fn : Symbol(fn, Decl(b.ts, 0, 8))
16+
17+
export const m = {
18+
>m : Symbol(m, Decl(b.ts, 1, 12))
19+
20+
/**
21+
* leading doc for prop
22+
*/
23+
prop: 1
24+
>prop : Symbol(prop, Decl(b.ts, 1, 18))
25+
}
26+
27+
28+
export const x = { p: fn };
29+
>x : Symbol(x, Decl(b.ts, 9, 12))
30+
>p : Symbol(p, Decl(b.ts, 9, 18))
31+
>fn : Symbol(fn, Decl(b.ts, 0, 8))
32+

‎tests/baselines/reference/declarationEmitTopLevelNodeFromCrossFile.types

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
//// [tests/cases/compiler/declarationEmitTopLevelNodeFromCrossFile.ts] ////
2+
3+
=== a.ts ===
4+
export type X = string;
5+
>X : string
6+
> : ^^^^^^
7+
8+
export const fn = { o: (a?: (X | undefined)[]) => {} };
9+
>fn : { o: (a?: (X | undefined)[]) => void; }
10+
> : ^^^^^^ ^^^ ^^^^^^^^^^^^
11+
>{ o: (a?: (X | undefined)[]) => {} } : { o: (a?: (X | undefined)[]) => void; }
12+
> : ^^^^^^ ^^^ ^^^^^^^^^^^^
13+
>o : (a?: (X | undefined)[]) => void
14+
> : ^ ^^^ ^^^^^^^^^
15+
>(a?: (X | undefined)[]) => {} : (a?: (X | undefined)[]) => void
16+
> : ^ ^^^ ^^^^^^^^^
17+
>a : string[]
18+
> : ^^^^^^^^
19+
20+
=== b.ts ===
21+
import {fn} from "./a";
22+
>fn : { o: (a?: (import("a").X | undefined)[]) => void; }
23+
> : ^^^^^^ ^^^ ^^^ ^ ^^^^^^^^^^^^
24+
25+
export const m = {
26+
>m : { prop: number; }
27+
> : ^^^^^^^^^^^^^^^^^
28+
>{ /** * leading doc for prop */ prop: 1} : { prop: number; }
29+
> : ^^^^^^^^^^^^^^^^^
30+
31+
/**
32+
* leading doc for prop
33+
*/
34+
prop: 1
35+
>prop : number
36+
> : ^^^^^^
37+
>1 : 1
38+
> : ^
39+
}
40+
41+
42+
export const x = { p: fn };
43+
>x : { p: { o: (a?: (import("a").X | undefined)[]) => void; }; }
44+
> : ^^^^^^^^^^^ ^^^ ^^^ ^ ^^^^^^^^^^^^^^^
45+
>{ p: fn } : { p: { o: (a?: (import("a").X | undefined)[]) => void; }; }
46+
> : ^^^^^^^^^^^ ^^^ ^^^ ^ ^^^^^^^^^^^^^^^
47+
>p : { o: (a?: (import("a").X | undefined)[]) => void; }
48+
> : ^^^^^^ ^^^ ^^^ ^ ^^^^^^^^^^^^
49+
>fn : { o: (a?: (import("a").X | undefined)[]) => void; }
50+
> : ^^^^^^ ^^^ ^^^ ^ ^^^^^^^^^^^^
51+

‎tests/cases/compiler/declarationEmitTopLevelNodeFromCrossFile.ts

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// @declaration: true
2+
// @filename: a.ts
3+
export type X = string;
4+
export const fn = { o: (a?: (X | undefined)[]) => {} };
5+
6+
// @filename: b.ts
7+
import {fn} from "./a";
8+
export const m = {
9+
/**
10+
* leading doc for prop
11+
*/
12+
prop: 1
13+
}
14+
15+
16+
export const x = { p: fn };

0 commit comments

Comments
 (0)