Skip to content

Commit 7ad72d2

Browse files
weswighamweswigham
authored
Rework symbol visibility checking to allow for multiple potential containers (microsoft#24971)
* Rework symbol visibility checking to allow for multiple potential containers * Express a preference for the local symbol if accessible from the enclosing declaration
1 parent b8b34a3 commit 7ad72d2

8 files changed

+171
-70
lines changed

‎src/compiler/checker.ts

Lines changed: 87 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2463,17 +2463,26 @@ namespace ts {
24632463
* Attempts to find the symbol corresponding to the container a symbol is in - usually this
24642464
* is just its' `.parent`, but for locals, this value is `undefined`
24652465
*/
2466-
function getContainerOfSymbol(symbol: Symbol): Symbol | undefined {
2466+
function getContainersOfSymbol(symbol: Symbol, enclosingDeclaration: Node | undefined): Symbol[] | undefined {
24672467
const container = getParentOfSymbol(symbol);
24682468
if (container) {
2469-
return container;
2469+
const additionalContainers = mapDefined(container.declarations, fileSymbolIfFileSymbolExportEqualsContainer);
2470+
if (enclosingDeclaration && getAccessibleSymbolChain(container, enclosingDeclaration, SymbolFlags.Namespace, /*externalOnly*/ false)) {
2471+
return concatenate([container], additionalContainers); // This order expresses a preference for the real container if it is in scope
2472+
}
2473+
return append(additionalContainers, container);
24702474
}
2471-
const candidate = forEach(symbol.declarations, d => !isAmbientModule(d) && d.parent && hasNonGlobalAugmentationExternalModuleSymbol(d.parent) ? getSymbolOfNode(d.parent) : undefined);
2472-
if (!candidate) {
2475+
const candidates = mapDefined(symbol.declarations, d => !isAmbientModule(d) && d.parent && hasNonGlobalAugmentationExternalModuleSymbol(d.parent) ? getSymbolOfNode(d.parent) : undefined);
2476+
if (!length(candidates)) {
24732477
return undefined;
24742478
}
2475-
const alias = getAliasForSymbolInContainer(candidate, symbol);
2476-
return alias ? candidate : undefined;
2479+
return mapDefined(candidates, candidate => getAliasForSymbolInContainer(candidate, symbol) ? candidate : undefined);
2480+
2481+
function fileSymbolIfFileSymbolExportEqualsContainer(d: Declaration) {
2482+
const fileSymbol = getExternalModuleContainer(d);
2483+
const exported = fileSymbol && fileSymbol.exports && fileSymbol.exports.get(InternalSymbolName.ExportEquals);
2484+
return resolveSymbol(exported) === resolveSymbol(container) ? fileSymbol : undefined;
2485+
}
24772486
}
24782487

24792488
function getAliasForSymbolInContainer(container: Symbol, symbol: Symbol) {
@@ -2759,6 +2768,56 @@ namespace ts {
27592768
return access.accessibility === SymbolAccessibility.Accessible;
27602769
}
27612770

2771+
function isAnySymbolAccessible(symbols: Symbol[] | undefined, enclosingDeclaration: Node, initialSymbol: Symbol, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean): SymbolAccessibilityResult | undefined {
2772+
if (!length(symbols)) return;
2773+
2774+
let hadAccessibleChain: Symbol | undefined;
2775+
for (const symbol of symbols!) {
2776+
// Symbol is accessible if it by itself is accessible
2777+
const accessibleSymbolChain = getAccessibleSymbolChain(symbol, enclosingDeclaration, meaning, /*useOnlyExternalAliasing*/ false);
2778+
if (accessibleSymbolChain) {
2779+
hadAccessibleChain = symbol;
2780+
const hasAccessibleDeclarations = hasVisibleDeclarations(accessibleSymbolChain[0], shouldComputeAliasesToMakeVisible);
2781+
if (hasAccessibleDeclarations) {
2782+
return hasAccessibleDeclarations;
2783+
}
2784+
}
2785+
else {
2786+
if (some(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol)) {
2787+
// Any meaning of a module symbol is always accessible via an `import` type
2788+
return {
2789+
accessibility: SymbolAccessibility.Accessible
2790+
};
2791+
}
2792+
}
2793+
2794+
// If we haven't got the accessible symbol, it doesn't mean the symbol is actually inaccessible.
2795+
// It could be a qualified symbol and hence verify the path
2796+
// e.g.:
2797+
// module m {
2798+
// export class c {
2799+
// }
2800+
// }
2801+
// const x: typeof m.c
2802+
// In the above example when we start with checking if typeof m.c symbol is accessible,
2803+
// we are going to see if c can be accessed in scope directly.
2804+
// But it can't, hence the accessible is going to be undefined, but that doesn't mean m.c is inaccessible
2805+
// It is accessible if the parent m is accessible because then m.c can be accessed through qualification
2806+
const parentResult = isAnySymbolAccessible(getContainersOfSymbol(symbol, enclosingDeclaration), enclosingDeclaration, initialSymbol, initialSymbol === symbol ? getQualifiedLeftMeaning(meaning) : meaning, shouldComputeAliasesToMakeVisible);
2807+
if (parentResult) {
2808+
return parentResult;
2809+
}
2810+
}
2811+
2812+
if (hadAccessibleChain) {
2813+
return {
2814+
accessibility: SymbolAccessibility.NotAccessible,
2815+
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
2816+
errorModuleName: hadAccessibleChain !== initialSymbol ? symbolToString(hadAccessibleChain, enclosingDeclaration, SymbolFlags.Namespace) : undefined,
2817+
};
2818+
}
2819+
}
2820+
27622821
/**
27632822
* Check if the given symbol in given enclosing declaration is accessible and mark all associated alias to be visible if requested
27642823
*
@@ -2769,57 +2828,21 @@ namespace ts {
27692828
*/
27702829
function isSymbolAccessible(symbol: Symbol | undefined, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean): SymbolAccessibilityResult {
27712830
if (symbol && enclosingDeclaration) {
2772-
const initialSymbol = symbol;
2773-
let meaningToLook = meaning;
2774-
while (symbol) {
2775-
// Symbol is accessible if it by itself is accessible
2776-
const accessibleSymbolChain = getAccessibleSymbolChain(symbol, enclosingDeclaration, meaningToLook, /*useOnlyExternalAliasing*/ false);
2777-
if (accessibleSymbolChain) {
2778-
const hasAccessibleDeclarations = hasVisibleDeclarations(accessibleSymbolChain[0], shouldComputeAliasesToMakeVisible);
2779-
if (!hasAccessibleDeclarations) {
2780-
return {
2781-
accessibility: SymbolAccessibility.NotAccessible,
2782-
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
2783-
errorModuleName: symbol !== initialSymbol ? symbolToString(symbol, enclosingDeclaration, SymbolFlags.Namespace) : undefined,
2784-
};
2785-
}
2786-
return hasAccessibleDeclarations;
2787-
}
2788-
else {
2789-
if (some(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol)) {
2790-
// Any meaning of a module symbol is always accessible via an `import` type
2791-
return {
2792-
accessibility: SymbolAccessibility.Accessible
2793-
};
2794-
}
2795-
}
2796-
2797-
// If we haven't got the accessible symbol, it doesn't mean the symbol is actually inaccessible.
2798-
// It could be a qualified symbol and hence verify the path
2799-
// e.g.:
2800-
// module m {
2801-
// export class c {
2802-
// }
2803-
// }
2804-
// const x: typeof m.c
2805-
// In the above example when we start with checking if typeof m.c symbol is accessible,
2806-
// we are going to see if c can be accessed in scope directly.
2807-
// But it can't, hence the accessible is going to be undefined, but that doesn't mean m.c is inaccessible
2808-
// It is accessible if the parent m is accessible because then m.c can be accessed through qualification
2809-
meaningToLook = getQualifiedLeftMeaning(meaning);
2810-
symbol = getContainerOfSymbol(symbol);
2831+
const result = isAnySymbolAccessible([symbol], enclosingDeclaration, symbol, meaning, shouldComputeAliasesToMakeVisible);
2832+
if (result) {
2833+
return result;
28112834
}
28122835

28132836
// This could be a symbol that is not exported in the external module
28142837
// or it could be a symbol from different external module that is not aliased and hence cannot be named
2815-
const symbolExternalModule = forEach(initialSymbol.declarations, getExternalModuleContainer);
2838+
const symbolExternalModule = forEach(symbol.declarations, getExternalModuleContainer);
28162839
if (symbolExternalModule) {
28172840
const enclosingExternalModule = getExternalModuleContainer(enclosingDeclaration);
28182841
if (symbolExternalModule !== enclosingExternalModule) {
28192842
// name from different external module that is not visible
28202843
return {
28212844
accessibility: SymbolAccessibility.CannotBeNamed,
2822-
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
2845+
errorSymbolName: symbolToString(symbol, enclosingDeclaration, meaning),
28232846
errorModuleName: symbolToString(symbolExternalModule)
28242847
};
28252848
}
@@ -2828,16 +2851,16 @@ namespace ts {
28282851
// Just a local name that is not accessible
28292852
return {
28302853
accessibility: SymbolAccessibility.NotAccessible,
2831-
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
2854+
errorSymbolName: symbolToString(symbol, enclosingDeclaration, meaning),
28322855
};
28332856
}
28342857

28352858
return { accessibility: SymbolAccessibility.Accessible };
2859+
}
28362860

2837-
function getExternalModuleContainer(declaration: Node) {
2838-
const node = findAncestor(declaration, hasExternalModuleSymbol);
2839-
return node && getSymbolOfNode(node);
2840-
}
2861+
function getExternalModuleContainer(declaration: Node) {
2862+
const node = findAncestor(declaration, hasExternalModuleSymbol);
2863+
return node && getSymbolOfNode(node);
28412864
}
28422865

28432866
function hasExternalModuleSymbol(declaration: Node) {
@@ -3667,18 +3690,19 @@ namespace ts {
36673690
/** @param endOfChain Set to false for recursive calls; non-recursive calls should always output something. */
36683691
function getSymbolChain(symbol: Symbol, meaning: SymbolFlags, endOfChain: boolean): Symbol[] | undefined {
36693692
let accessibleSymbolChain = getAccessibleSymbolChain(symbol, context.enclosingDeclaration, meaning, !!(context.flags & NodeBuilderFlags.UseOnlyExternalAliasing));
3670-
let parentSymbol: Symbol | undefined;
36713693

36723694
if (!accessibleSymbolChain ||
36733695
needsQualification(accessibleSymbolChain[0], context.enclosingDeclaration, accessibleSymbolChain.length === 1 ? meaning : getQualifiedLeftMeaning(meaning))) {
36743696

36753697
// Go up and add our parent.
3676-
const parent = getContainerOfSymbol(accessibleSymbolChain ? accessibleSymbolChain[0] : symbol);
3677-
if (parent) {
3678-
const parentChain = getSymbolChain(parent, getQualifiedLeftMeaning(meaning), /*endOfChain*/ false);
3679-
if (parentChain) {
3680-
parentSymbol = parent;
3681-
accessibleSymbolChain = parentChain.concat(accessibleSymbolChain || [getAliasForSymbolInContainer(parent, symbol) || symbol]);
3698+
const parents = getContainersOfSymbol(accessibleSymbolChain ? accessibleSymbolChain[0] : symbol, context.enclosingDeclaration);
3699+
if (length(parents)) {
3700+
for (const parent of parents!) {
3701+
const parentChain = getSymbolChain(parent, getQualifiedLeftMeaning(meaning), /*endOfChain*/ false);
3702+
if (parentChain) {
3703+
accessibleSymbolChain = parentChain.concat(accessibleSymbolChain || [getAliasForSymbolInContainer(parent, symbol) || symbol]);
3704+
break;
3705+
}
36823706
}
36833707
}
36843708
}
@@ -3689,11 +3713,12 @@ namespace ts {
36893713
if (
36903714
// If this is the last part of outputting the symbol, always output. The cases apply only to parent symbols.
36913715
endOfChain ||
3692-
// If a parent symbol is an external module, don't write it. (We prefer just `x` vs `"foo/bar".x`.)
3693-
(yieldModuleSymbol || !(!parentSymbol && forEach(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol))) &&
36943716
// If a parent symbol is an anonymous type, don't write it.
36953717
!(symbol.flags & (SymbolFlags.TypeLiteral | SymbolFlags.ObjectLiteral))) {
3696-
3718+
// If a parent symbol is an external module, don't write it. (We prefer just `x` vs `"foo/bar".x`.)
3719+
if (!endOfChain && !yieldModuleSymbol && !!forEach(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol)) {
3720+
return;
3721+
}
36973722
return [symbol];
36983723
}
36993724
}

‎tests/baselines/reference/dynamicNames.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ namespace N {
141141
>T5 : T5
142142
}
143143
export declare type T7 = {
144-
>T7 : { [N.c2]: number; [N.c3]: string; [N.s1]: boolean; }
144+
>T7 : T7
145145

146146
[N.c2]: number;
147147
>[N.c2] : number

‎tests/baselines/reference/exportAssignedNamespaceIsVisibleInDeclarationEmit.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//// [tests/cases/compiler/exportAssignedNamespaceIsVisibleInDeclarationEmit.ts] ////
2+
3+
//// [thing.d.ts]
4+
declare namespace Foo {
5+
export interface Bar {}
6+
export function f(): Bar;
7+
}
8+
export = Foo;
9+
//// [index.ts]
10+
import { f } from "./thing";
11+
export const thing = f();
12+
13+
//// [index.js]
14+
"use strict";
15+
exports.__esModule = true;
16+
var thing_1 = require("./thing");
17+
exports.thing = thing_1.f();
18+
19+
20+
//// [index.d.ts]
21+
export declare const thing: import("./thing").Bar;

‎tests/baselines/reference/exportAssignedNamespaceIsVisibleInDeclarationEmit.symbols

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
=== tests/cases/compiler/thing.d.ts ===
2+
declare namespace Foo {
3+
>Foo : Symbol(Foo, Decl(thing.d.ts, 0, 0))
4+
5+
export interface Bar {}
6+
>Bar : Symbol(Bar, Decl(thing.d.ts, 0, 23))
7+
8+
export function f(): Bar;
9+
>f : Symbol(f, Decl(thing.d.ts, 1, 27))
10+
>Bar : Symbol(Bar, Decl(thing.d.ts, 0, 23))
11+
}
12+
export = Foo;
13+
>Foo : Symbol(Foo, Decl(thing.d.ts, 0, 0))
14+
15+
=== tests/cases/compiler/index.ts ===
16+
import { f } from "./thing";
17+
>f : Symbol(f, Decl(index.ts, 0, 8))
18+
19+
export const thing = f();
20+
>thing : Symbol(thing, Decl(index.ts, 1, 12))
21+
>f : Symbol(f, Decl(index.ts, 0, 8))
22+

‎tests/baselines/reference/exportAssignedNamespaceIsVisibleInDeclarationEmit.types

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
=== tests/cases/compiler/thing.d.ts ===
2+
declare namespace Foo {
3+
>Foo : typeof Foo
4+
5+
export interface Bar {}
6+
>Bar : Bar
7+
8+
export function f(): Bar;
9+
>f : () => Bar
10+
>Bar : Bar
11+
}
12+
export = Foo;
13+
>Foo : typeof Foo
14+
15+
=== tests/cases/compiler/index.ts ===
16+
import { f } from "./thing";
17+
>f : () => import("tests/cases/compiler/thing").Bar
18+
19+
export const thing = f();
20+
>thing : import("tests/cases/compiler/thing").Bar
21+
>f() : import("tests/cases/compiler/thing").Bar
22+
>f : () => import("tests/cases/compiler/thing").Bar
23+

‎tests/baselines/reference/importTypeInJSDoc.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ a = new Foo({doer: Foo.Bar});
5959
>Bar : (x: string, y?: number) => void
6060

6161
const q = /** @type {import("./externs").Bar} */({ doer: q => q });
62-
>q : MyClass.Bar
63-
>({ doer: q => q }) : MyClass.Bar
62+
>q : import("tests/cases/conformance/types/import/externs").Bar
63+
>({ doer: q => q }) : import("tests/cases/conformance/types/import/externs").Bar
6464
>{ doer: q => q } : { doer: (q: string) => string; }
6565
>doer : (q: string) => string
6666
>q => q : (q: string) => string

‎tests/baselines/reference/reactImportDropped.types

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ declare global {
3636

3737
=== tests/cases/compiler/src/components/TabBar.js ===
3838
export default React.createClass({
39-
>React.createClass({ render() { return ( null ); }}) : React.ClassicComponentClass
40-
>React.createClass : (spec: any) => React.ClassicComponentClass
39+
>React.createClass({ render() { return ( null ); }}) : import("tests/cases/compiler/react").ClassicComponentClass
40+
>React.createClass : (spec: any) => import("tests/cases/compiler/react").ClassicComponentClass
4141
>React : typeof React
42-
>createClass : (spec: any) => React.ClassicComponentClass
42+
>createClass : (spec: any) => import("tests/cases/compiler/react").ClassicComponentClass
4343
>{ render() { return ( null ); }} : { render(): any; }
4444

4545
render() {
@@ -57,15 +57,15 @@ export default React.createClass({
5757

5858
=== tests/cases/compiler/src/modules/navigation/NavigationView.js ===
5959
import TabBar from '../../components/TabBar';
60-
>TabBar : React.ClassicComponentClass
60+
>TabBar : import("tests/cases/compiler/react").ClassicComponentClass
6161

6262
import {layout} from '../../utils/theme'; // <- DO NOT DROP this import
6363
>layout : any
6464

6565
const x = <TabBar height={layout.footerHeight} />;
6666
>x : any
6767
><TabBar height={layout.footerHeight} /> : any
68-
>TabBar : React.ClassicComponentClass
68+
>TabBar : import("tests/cases/compiler/react").ClassicComponentClass
6969
>height : any
7070
>layout.footerHeight : any
7171
>layout : any

‎tests/cases/compiler/exportAssignedNamespaceIsVisibleInDeclarationEmit.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// @declaration: true
2+
// @filename: thing.d.ts
3+
declare namespace Foo {
4+
export interface Bar {}
5+
export function f(): Bar;
6+
}
7+
export = Foo;
8+
// @filename: index.ts
9+
import { f } from "./thing";
10+
export const thing = f();

0 commit comments

Comments
 (0)