Skip to content

Commit 50866e1

Browse files
weswighamweswigham
authored
Fix visibility checking of mutually recursive exports (#19929)
* Do visibility painting from collectLinkedAliases in checker to remove statefullness in declaration emit * Fix #17085 * Add deeply destructured array to test * Add test case for #18634 * Add PR feedback
1 parent 8c6863a commit 50866e1

10 files changed

+376
-10
lines changed

‎src/compiler/checker.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4047,25 +4047,30 @@ namespace ts {
40474047
}
40484048
}
40494049

4050-
function collectLinkedAliases(node: Identifier): Node[] {
4050+
function collectLinkedAliases(node: Identifier, setVisibility?: boolean): Node[] | undefined {
40514051
let exportSymbol: Symbol;
40524052
if (node.parent && node.parent.kind === SyntaxKind.ExportAssignment) {
4053-
exportSymbol = resolveName(node.parent, node.escapedText, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias, Diagnostics.Cannot_find_name_0, node, /*isUse*/ false);
4053+
exportSymbol = resolveName(node, node.escapedText, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias, /*nameNotFoundMessage*/ undefined, node, /*isUse*/ false);
40544054
}
40554055
else if (node.parent.kind === SyntaxKind.ExportSpecifier) {
40564056
exportSymbol = getTargetOfExportSpecifier(<ExportSpecifier>node.parent, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias);
40574057
}
4058-
const result: Node[] = [];
4058+
let result: Node[];
40594059
if (exportSymbol) {
40604060
buildVisibleNodeList(exportSymbol.declarations);
40614061
}
40624062
return result;
40634063

40644064
function buildVisibleNodeList(declarations: Declaration[]) {
40654065
forEach(declarations, declaration => {
4066-
getNodeLinks(declaration).isVisible = true;
40674066
const resultNode = getAnyImportSyntax(declaration) || declaration;
4068-
pushIfUnique(result, resultNode);
4067+
if (setVisibility) {
4068+
getNodeLinks(declaration).isVisible = true;
4069+
}
4070+
else {
4071+
result = result || [];
4072+
pushIfUnique(result, resultNode);
4073+
}
40694074

40704075
if (isInternalModuleImportEqualsDeclaration(declaration)) {
40714076
// Add the referenced top container visible
@@ -23328,6 +23333,9 @@ namespace ts {
2332823333

2332923334
function checkExportSpecifier(node: ExportSpecifier) {
2333023335
checkAliasSymbol(node);
23336+
if (compilerOptions.declaration) {
23337+
collectLinkedAliases(node.propertyName || node.name, /*setVisibility*/ true); // Collect linked aliases to set visibility links
23338+
}
2333123339
if (!(<ExportDeclaration>node.parent.parent).moduleSpecifier) {
2333223340
const exportedName = node.propertyName || node.name;
2333323341
// find immediate value referenced by exported name (SymbolFlags.Alias is set so we don't chase down aliases)
@@ -23365,6 +23373,10 @@ namespace ts {
2336523373
}
2336623374
if (node.expression.kind === SyntaxKind.Identifier) {
2336723375
markExportAsReferenced(node);
23376+
23377+
if (compilerOptions.declaration) {
23378+
collectLinkedAliases(node.expression as Identifier, /*setVisibility*/ true); // Sets visibility flags on refernced nodes
23379+
}
2336823380
}
2336923381
else {
2337023382
checkExpressionCached(node.expression);

‎src/compiler/declarationEmitter.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,10 +1249,18 @@ namespace ts {
12491249
writeLine();
12501250
}
12511251

1252+
function bindingNameContainsVisibleBindingElement(node: BindingName): boolean {
1253+
return !!node && isBindingPattern(node) && some(node.elements, elem => !isOmittedExpression(elem) && isVariableDeclarationVisible(elem));
1254+
}
1255+
1256+
function isVariableDeclarationVisible(node: VariableDeclaration | BindingElement) {
1257+
return resolver.isDeclarationVisible(node) || bindingNameContainsVisibleBindingElement(node.name);
1258+
}
1259+
12521260
function emitVariableDeclaration(node: VariableDeclaration | PropertyDeclaration | PropertySignature | ParameterDeclaration) {
12531261
// If we are emitting property it isn't moduleElement and hence we already know it needs to be emitted
12541262
// so there is no check needed to see if declaration is visible
1255-
if (node.kind !== SyntaxKind.VariableDeclaration || resolver.isDeclarationVisible(node)) {
1263+
if (node.kind !== SyntaxKind.VariableDeclaration || isVariableDeclarationVisible(node)) {
12561264
if (isBindingPattern(node.name)) {
12571265
emitBindingPattern(<BindingPattern>node.name);
12581266
}
@@ -1324,14 +1332,14 @@ namespace ts {
13241332
}
13251333

13261334
function emitBindingPattern(bindingPattern: BindingPattern) {
1327-
// Only select non-omitted expression from the bindingPattern's elements.
1335+
// Only select visible, non-omitted expression from the bindingPattern's elements.
13281336
// We have to do this to avoid emitting trailing commas.
13291337
// For example:
13301338
// original: var [, c,,] = [ 2,3,4]
13311339
// emitted: declare var c: number; // instead of declare var c:number, ;
13321340
const elements: Node[] = [];
13331341
for (const element of bindingPattern.elements) {
1334-
if (element.kind !== SyntaxKind.OmittedExpression) {
1342+
if (element.kind !== SyntaxKind.OmittedExpression && isVariableDeclarationVisible(element)) {
13351343
elements.push(element);
13361344
}
13371345
}
@@ -1371,7 +1379,7 @@ namespace ts {
13711379
}
13721380

13731381
function isVariableStatementVisible(node: VariableStatement) {
1374-
return forEach(node.declarationList.declarations, varDeclaration => resolver.isDeclarationVisible(varDeclaration));
1382+
return forEach(node.declarationList.declarations, varDeclaration => isVariableDeclarationVisible(varDeclaration));
13751383
}
13761384

13771385
function writeVariableStatement(node: VariableStatement) {
@@ -1390,7 +1398,7 @@ namespace ts {
13901398
else {
13911399
write("var ");
13921400
}
1393-
emitCommaList(node.declarationList.declarations, emitVariableDeclaration, resolver.isDeclarationVisible);
1401+
emitCommaList(node.declarationList.declarations, emitVariableDeclaration, isVariableDeclarationVisible);
13941402
write(";");
13951403
writeLine();
13961404
}

‎tests/baselines/reference/destructuredDeclarationEmit.js

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
//// [tests/cases/compiler/destructuredDeclarationEmit.ts] ////
2+
3+
//// [foo.ts]
4+
const foo = { bar: 'hello', bat: 'world', bam: { bork: { bar: 'a', baz: 'b' } } };
5+
const arr: [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]] = [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]];
6+
export { foo, arr };
7+
//// [index.ts]
8+
import { foo, arr } from './foo';
9+
export { foo, arr };
10+
11+
const { bar: baz, bat, bam: { bork: { bar: ibar, baz: ibaz } } } = foo;
12+
export { baz, ibaz };
13+
14+
const [ , one, , [, bee, , [, {sec} ]]] = arr;
15+
export { one, bee, sec };
16+
17+
const getFoo = () => ({
18+
foo: 'foo'
19+
});
20+
21+
const { foo: foo2 } = getFoo();
22+
export { foo2 };
23+
24+
25+
//// [foo.js]
26+
"use strict";
27+
exports.__esModule = true;
28+
var foo = { bar: 'hello', bat: 'world', bam: { bork: { bar: 'a', baz: 'b' } } };
29+
exports.foo = foo;
30+
var arr = [0, 1, 2, ['a', 'b', 'c', [{ def: 'def' }, { sec: 'sec' }]]];
31+
exports.arr = arr;
32+
//// [index.js]
33+
"use strict";
34+
exports.__esModule = true;
35+
var foo_1 = require("./foo");
36+
exports.foo = foo_1.foo;
37+
exports.arr = foo_1.arr;
38+
var baz = foo_1.foo.bar, bat = foo_1.foo.bat, _a = foo_1.foo.bam.bork, ibar = _a.bar, ibaz = _a.baz;
39+
exports.baz = baz;
40+
exports.ibaz = ibaz;
41+
var one = foo_1.arr[1], _b = foo_1.arr[3], bee = _b[1], _c = _b[3], sec = _c[1].sec;
42+
exports.one = one;
43+
exports.bee = bee;
44+
exports.sec = sec;
45+
var getFoo = function () { return ({
46+
foo: 'foo'
47+
}); };
48+
var foo2 = getFoo().foo;
49+
exports.foo2 = foo2;
50+
51+
52+
//// [foo.d.ts]
53+
declare const foo: {
54+
bar: string;
55+
bat: string;
56+
bam: {
57+
bork: {
58+
bar: string;
59+
baz: string;
60+
};
61+
};
62+
};
63+
declare const arr: [0, 1, 2, ['a', 'b', 'c', [{
64+
def: 'def';
65+
}, {
66+
sec: 'sec';
67+
}]]];
68+
export { foo, arr };
69+
//// [index.d.ts]
70+
import { foo, arr } from './foo';
71+
export { foo, arr };
72+
declare const baz: string, ibaz: string;
73+
export { baz, ibaz };
74+
declare const one: 1, bee: "b", sec: "sec";
75+
export { one, bee, sec };
76+
declare const foo2: string;
77+
export { foo2 };

‎tests/baselines/reference/destructuredDeclarationEmit.symbols

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
=== tests/cases/compiler/foo.ts ===
2+
const foo = { bar: 'hello', bat: 'world', bam: { bork: { bar: 'a', baz: 'b' } } };
3+
>foo : Symbol(foo, Decl(foo.ts, 0, 5))
4+
>bar : Symbol(bar, Decl(foo.ts, 0, 13))
5+
>bat : Symbol(bat, Decl(foo.ts, 0, 27))
6+
>bam : Symbol(bam, Decl(foo.ts, 0, 41))
7+
>bork : Symbol(bork, Decl(foo.ts, 0, 48))
8+
>bar : Symbol(bar, Decl(foo.ts, 0, 56))
9+
>baz : Symbol(baz, Decl(foo.ts, 0, 66))
10+
11+
const arr: [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]] = [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]];
12+
>arr : Symbol(arr, Decl(foo.ts, 1, 5))
13+
>def : Symbol(def, Decl(foo.ts, 1, 39))
14+
>sec : Symbol(sec, Decl(foo.ts, 1, 53))
15+
>def : Symbol(def, Decl(foo.ts, 1, 98))
16+
>sec : Symbol(sec, Decl(foo.ts, 1, 112))
17+
18+
export { foo, arr };
19+
>foo : Symbol(foo, Decl(foo.ts, 2, 8))
20+
>arr : Symbol(arr, Decl(foo.ts, 2, 13))
21+
22+
=== tests/cases/compiler/index.ts ===
23+
import { foo, arr } from './foo';
24+
>foo : Symbol(foo, Decl(index.ts, 0, 8))
25+
>arr : Symbol(arr, Decl(index.ts, 0, 13))
26+
27+
export { foo, arr };
28+
>foo : Symbol(foo, Decl(index.ts, 1, 8))
29+
>arr : Symbol(arr, Decl(index.ts, 1, 13))
30+
31+
const { bar: baz, bat, bam: { bork: { bar: ibar, baz: ibaz } } } = foo;
32+
>bar : Symbol(bar, Decl(foo.ts, 0, 13))
33+
>baz : Symbol(baz, Decl(index.ts, 3, 7))
34+
>bat : Symbol(bat, Decl(index.ts, 3, 17))
35+
>bam : Symbol(bam, Decl(foo.ts, 0, 41))
36+
>bork : Symbol(bork, Decl(foo.ts, 0, 48))
37+
>bar : Symbol(bar, Decl(foo.ts, 0, 56))
38+
>ibar : Symbol(ibar, Decl(index.ts, 3, 37))
39+
>baz : Symbol(baz, Decl(foo.ts, 0, 66))
40+
>ibaz : Symbol(ibaz, Decl(index.ts, 3, 48))
41+
>foo : Symbol(foo, Decl(index.ts, 0, 8))
42+
43+
export { baz, ibaz };
44+
>baz : Symbol(baz, Decl(index.ts, 4, 8))
45+
>ibaz : Symbol(ibaz, Decl(index.ts, 4, 13))
46+
47+
const [ , one, , [, bee, , [, {sec} ]]] = arr;
48+
>one : Symbol(one, Decl(index.ts, 6, 9))
49+
>bee : Symbol(bee, Decl(index.ts, 6, 19))
50+
>sec : Symbol(sec, Decl(index.ts, 6, 31))
51+
>arr : Symbol(arr, Decl(index.ts, 0, 13))
52+
53+
export { one, bee, sec };
54+
>one : Symbol(one, Decl(index.ts, 7, 8))
55+
>bee : Symbol(bee, Decl(index.ts, 7, 13))
56+
>sec : Symbol(sec, Decl(index.ts, 7, 18))
57+
58+
const getFoo = () => ({
59+
>getFoo : Symbol(getFoo, Decl(index.ts, 9, 5))
60+
61+
foo: 'foo'
62+
>foo : Symbol(foo, Decl(index.ts, 9, 23))
63+
64+
});
65+
66+
const { foo: foo2 } = getFoo();
67+
>foo : Symbol(foo, Decl(index.ts, 9, 23))
68+
>foo2 : Symbol(foo2, Decl(index.ts, 13, 7))
69+
>getFoo : Symbol(getFoo, Decl(index.ts, 9, 5))
70+
71+
export { foo2 };
72+
>foo2 : Symbol(foo2, Decl(index.ts, 14, 8))
73+

‎tests/baselines/reference/destructuredDeclarationEmit.types

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
=== tests/cases/compiler/foo.ts ===
2+
const foo = { bar: 'hello', bat: 'world', bam: { bork: { bar: 'a', baz: 'b' } } };
3+
>foo : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
4+
>{ bar: 'hello', bat: 'world', bam: { bork: { bar: 'a', baz: 'b' } } } : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
5+
>bar : string
6+
>'hello' : "hello"
7+
>bat : string
8+
>'world' : "world"
9+
>bam : { bork: { bar: string; baz: string; }; }
10+
>{ bork: { bar: 'a', baz: 'b' } } : { bork: { bar: string; baz: string; }; }
11+
>bork : { bar: string; baz: string; }
12+
>{ bar: 'a', baz: 'b' } : { bar: string; baz: string; }
13+
>bar : string
14+
>'a' : "a"
15+
>baz : string
16+
>'b' : "b"
17+
18+
const arr: [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]] = [0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]];
19+
>arr : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
20+
>def : "def"
21+
>sec : "sec"
22+
>[0, 1, 2, ['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]]] : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
23+
>0 : 0
24+
>1 : 1
25+
>2 : 2
26+
>['a', 'b', 'c', [{def: 'def'}, {sec: 'sec'}]] : ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]
27+
>'a' : "a"
28+
>'b' : "b"
29+
>'c' : "c"
30+
>[{def: 'def'}, {sec: 'sec'}] : [{ def: "def"; }, { sec: "sec"; }]
31+
>{def: 'def'} : { def: "def"; }
32+
>def : string
33+
>'def' : "def"
34+
>{sec: 'sec'} : { sec: "sec"; }
35+
>sec : string
36+
>'sec' : "sec"
37+
38+
export { foo, arr };
39+
>foo : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
40+
>arr : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
41+
42+
=== tests/cases/compiler/index.ts ===
43+
import { foo, arr } from './foo';
44+
>foo : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
45+
>arr : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
46+
47+
export { foo, arr };
48+
>foo : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
49+
>arr : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
50+
51+
const { bar: baz, bat, bam: { bork: { bar: ibar, baz: ibaz } } } = foo;
52+
>bar : any
53+
>baz : string
54+
>bat : string
55+
>bam : any
56+
>bork : any
57+
>bar : any
58+
>ibar : string
59+
>baz : any
60+
>ibaz : string
61+
>foo : { bar: string; bat: string; bam: { bork: { bar: string; baz: string; }; }; }
62+
63+
export { baz, ibaz };
64+
>baz : string
65+
>ibaz : string
66+
67+
const [ , one, , [, bee, , [, {sec} ]]] = arr;
68+
> : undefined
69+
>one : 1
70+
> : undefined
71+
> : undefined
72+
>bee : "b"
73+
> : undefined
74+
> : undefined
75+
>sec : "sec"
76+
>arr : [0, 1, 2, ["a", "b", "c", [{ def: "def"; }, { sec: "sec"; }]]]
77+
78+
export { one, bee, sec };
79+
>one : 1
80+
>bee : "b"
81+
>sec : "sec"
82+
83+
const getFoo = () => ({
84+
>getFoo : () => { foo: string; }
85+
>() => ({ foo: 'foo'}) : () => { foo: string; }
86+
>({ foo: 'foo'}) : { foo: string; }
87+
>{ foo: 'foo'} : { foo: string; }
88+
89+
foo: 'foo'
90+
>foo : string
91+
>'foo' : "foo"
92+
93+
});
94+
95+
const { foo: foo2 } = getFoo();
96+
>foo : any
97+
>foo2 : string
98+
>getFoo() : { foo: string; }
99+
>getFoo : () => { foo: string; }
100+
101+
export { foo2 };
102+
>foo2 : string
103+

‎tests/baselines/reference/mutuallyRecursiveInterfaceDeclaration.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [mutuallyRecursiveInterfaceDeclaration.ts]
2+
interface A {
3+
b: B
4+
}
5+
6+
interface B {
7+
a: A
8+
}
9+
export {A, B}
10+
11+
//// [mutuallyRecursiveInterfaceDeclaration.js]
12+
"use strict";
13+
exports.__esModule = true;
14+
15+
16+
//// [mutuallyRecursiveInterfaceDeclaration.d.ts]
17+
interface A {
18+
b: B;
19+
}
20+
interface B {
21+
a: A;
22+
}
23+
export { A, B };

0 commit comments

Comments
 (0)