Skip to content

Commit fce3d9f

Browse files
weswighamweswigham
authored
Check the ambientness of a symbol name before attempting to trim it (#26312)
* Check the ambientness of a symbol name before attempting to trim it * Use find instead of forEach, remember to also exclude global augmentations
1 parent f6af618 commit fce3d9f

17 files changed

+271
-44
lines changed

‎src/compiler/checker.ts

+24-21
Original file line numberDiff line numberDiff line change
@@ -3880,31 +3880,34 @@ namespace ts {
38803880
}
38813881
}
38823882
}
3883-
return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);
3884-
}
3885-
else {
3886-
if (!context.enclosingDeclaration || !context.tracker.moduleResolverHost) {
3887-
// If there's no context declaration, we can't lookup a non-ambient specifier, so we just use the symbol name
3883+
if (ambientModuleSymbolRegex.test(symbol.escapedName as string)) {
38883884
return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);
38893885
}
3890-
const contextFile = getSourceFileOfNode(getOriginalNode(context.enclosingDeclaration));
3891-
const links = getSymbolLinks(symbol);
3892-
let specifier = links.specifierCache && links.specifierCache.get(contextFile.path);
3893-
if (!specifier) {
3894-
specifier = moduleSpecifiers.getModuleSpecifierForDeclarationFile(
3895-
symbol,
3896-
compilerOptions,
3897-
contextFile,
3898-
context.tracker.moduleResolverHost,
3899-
context.tracker.moduleResolverHost.getSourceFiles!(), // TODO: GH#18217
3900-
{ importModuleSpecifierPreference: "non-relative" },
3901-
host.redirectTargetsMap,
3902-
);
3903-
links.specifierCache = links.specifierCache || createMap();
3904-
links.specifierCache.set(contextFile.path, specifier);
3886+
}
3887+
if (!context.enclosingDeclaration || !context.tracker.moduleResolverHost) {
3888+
// If there's no context declaration, we can't lookup a non-ambient specifier, so we just use the symbol name
3889+
if (ambientModuleSymbolRegex.test(symbol.escapedName as string)) {
3890+
return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);
39053891
}
3906-
return specifier;
3892+
return getSourceFileOfNode(getNonAugmentationDeclaration(symbol)!).fileName; // A resolver may not be provided for baselines and errors - in those cases we use the fileName in full
3893+
}
3894+
const contextFile = getSourceFileOfNode(getOriginalNode(context.enclosingDeclaration));
3895+
const links = getSymbolLinks(symbol);
3896+
let specifier = links.specifierCache && links.specifierCache.get(contextFile.path);
3897+
if (!specifier) {
3898+
specifier = moduleSpecifiers.getModuleSpecifierForDeclarationFile(
3899+
symbol,
3900+
compilerOptions,
3901+
contextFile,
3902+
context.tracker.moduleResolverHost,
3903+
context.tracker.moduleResolverHost.getSourceFiles!(), // TODO: GH#18217
3904+
{ importModuleSpecifierPreference: "non-relative" },
3905+
host.redirectTargetsMap,
3906+
);
3907+
links.specifierCache = links.specifierCache || createMap();
3908+
links.specifierCache.set(contextFile.path, specifier);
39073909
}
3910+
return specifier;
39083911
}
39093912

39103913
function symbolToTypeNode(symbol: Symbol, context: NodeBuilderContext, meaning: SymbolFlags, overrideTypeArguments?: ReadonlyArray<TypeNode>): TypeNode {

‎src/compiler/moduleSpecifiers.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace ts.moduleSpecifiers {
5151
if (!files) {
5252
return Debug.fail("Files list must be present to resolve symlinks in specifier resolution");
5353
}
54-
const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration);
54+
const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration || getNonAugmentationDeclaration(moduleSymbol));
5555
const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap);
5656

5757
const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
@@ -232,8 +232,10 @@ namespace ts.moduleSpecifiers {
232232
}
233233

234234
function tryGetModuleNameFromAmbientModule(moduleSymbol: Symbol): string | undefined {
235-
const decl = moduleSymbol.valueDeclaration;
236-
if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) {
235+
const decl = find(moduleSymbol.declarations,
236+
d => isNonGlobalAmbientModule(d) && (!isExternalModuleAugmentation(d) || !isExternalModuleNameRelative(getTextOfIdentifierOrLiteral(d.name)))
237+
) as (ModuleDeclaration & { name: StringLiteral }) | undefined;
238+
if (decl) {
237239
return decl.name.text;
238240
}
239241
}

‎src/compiler/utilities.ts

+4
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,10 @@ namespace ts {
638638
return false;
639639
}
640640

641+
export function getNonAugmentationDeclaration(symbol: Symbol) {
642+
return find(symbol.declarations, d => !isExternalModuleAugmentation(d) && !(isModuleDeclaration(d) && isGlobalScopeAugmentation(d)));
643+
}
644+
641645
export function isEffectiveExternalModule(node: SourceFile, compilerOptions: CompilerOptions) {
642646
return isExternalModule(node) || compilerOptions.isolatedModules || ((getEmitModuleKind(compilerOptions) === ModuleKind.CommonJS) && !!node.commonJsModuleIndicator);
643647
}

‎tests/baselines/reference/augmentExportEquals3.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
=== tests/cases/compiler/file1.ts ===
22
function foo() {}
3-
>foo : typeof import("o")
3+
>foo : typeof import("tests/cases/compiler/file1.ts")
44

55
namespace foo {
6-
>foo : typeof import("o")
6+
>foo : typeof import("tests/cases/compiler/file1.ts")
77

88
export var v = 1;
99
>v : number

‎tests/baselines/reference/augmentExportEquals3_1.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ declare module "file1" {
33
>"file1" : typeof import("file1")
44

55
function foo(): void;
6-
>foo : typeof import("o")
6+
>foo : typeof import("tests/cases/compiler/file1.d.ts")
77

88
namespace foo {
9-
>foo : typeof import("o")
9+
>foo : typeof import("tests/cases/compiler/file1.d.ts")
1010

1111
export var v: number;
1212
>v : number

‎tests/baselines/reference/augmentExportEquals4.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
=== tests/cases/compiler/file1.ts ===
22
class foo {}
3-
>foo : import("o")
3+
>foo : import("tests/cases/compiler/file1.ts")
44

55
namespace foo {
6-
>foo : typeof import("o")
6+
>foo : typeof import("tests/cases/compiler/file1.ts")
77

88
export var v = 1;
99
>v : number

‎tests/baselines/reference/augmentExportEquals4_1.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ declare module "file1" {
33
>"file1" : typeof import("file1")
44

55
class foo {}
6-
>foo : import("o")
6+
>foo : import("tests/cases/compiler/file1.d.ts")
77

88
namespace foo {
9-
>foo : typeof import("o")
9+
>foo : typeof import("tests/cases/compiler/file1.d.ts")
1010

1111
export var v: number;
1212
>v : number

‎tests/baselines/reference/augmentExportEquals5.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ declare module "express" {
99
>"express" : typeof import("express")
1010

1111
function e(): e.Express;
12-
>e : typeof import("e")
12+
>e : typeof import("tests/cases/compiler/express.d.ts")
1313
>e : any
1414

1515
namespace e {
16-
>e : typeof import("e")
16+
>e : typeof import("tests/cases/compiler/express.d.ts")
1717

1818
interface IRoute {
1919
all(...handler: RequestHandler[]): IRoute;

‎tests/baselines/reference/augmentExportEquals6.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
=== tests/cases/compiler/file1.ts ===
22
class foo {}
3-
>foo : import("o")
3+
>foo : import("tests/cases/compiler/file1.ts")
44

55
namespace foo {
6-
>foo : typeof import("o")
6+
>foo : typeof import("tests/cases/compiler/file1.ts")
77

88
export class A {}
99
>A : A

‎tests/baselines/reference/augmentExportEquals6_1.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ declare module "file1" {
33
>"file1" : typeof import("file1")
44

55
class foo {}
6-
>foo : import("o")
6+
>foo : import("tests/cases/compiler/file1.d.ts")
77

88
namespace foo {
9-
>foo : typeof import("o")
9+
>foo : typeof import("tests/cases/compiler/file1.d.ts")
1010

1111
class A {}
1212
>A : A

‎tests/baselines/reference/moduleAugmentationDuringSyntheticDefaultCheck.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import moment = require("moment-timezone");
66

77
=== tests/cases/compiler/node_modules/moment/index.d.ts ===
88
declare function moment(): moment.Moment;
9-
>moment : () => import("omen").Moment
9+
>moment : () => import("tests/cases/compiler/node_modules/moment/index.d.ts").Moment
1010
>moment : any
1111

1212
declare namespace moment {
@@ -16,7 +16,7 @@ declare namespace moment {
1616
}
1717
}
1818
export = moment;
19-
>moment : () => import("omen").Moment
19+
>moment : () => import("tests/cases/compiler/node_modules/moment/index.d.ts").Moment
2020

2121
=== tests/cases/compiler/node_modules/moment-timezone/index.d.ts ===
2222
import * as moment from 'moment';

‎tests/baselines/reference/reactTransitiveImportHasValidDeclaration.js

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//// [tests/cases/compiler/reactTransitiveImportHasValidDeclaration.ts] ////
2+
3+
//// [index.d.ts]
4+
declare namespace React {
5+
export interface DetailedHTMLProps<T, U> {}
6+
export interface HTMLAttributes<T> {}
7+
}
8+
export = React;
9+
export as namespace React;
10+
//// [index.d.ts]
11+
/// <reference types="react" />
12+
declare module 'react' { // augment
13+
interface HTMLAttributes<T> {
14+
css?: unknown;
15+
}
16+
}
17+
export interface StyledOtherComponentList {
18+
"div": React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement>
19+
}
20+
export interface StyledOtherComponent<A, B, C> {}
21+
22+
//// [index.d.ts]
23+
export * from "./types/react";
24+
25+
//// [index.d.ts]
26+
import {StyledOtherComponent, StyledOtherComponentList} from "create-emotion-styled";
27+
export default function styled(tag: string): (o: object) => StyledOtherComponent<{}, StyledOtherComponentList["div"], any>;
28+
29+
//// [index.ts]
30+
import styled from "react-emotion"
31+
32+
const Form = styled('div')({ color: "red" })
33+
34+
export default Form
35+
36+
37+
//// [index.js]
38+
"use strict";
39+
exports.__esModule = true;
40+
var react_emotion_1 = require("react-emotion");
41+
var Form = react_emotion_1["default"]('div')({ color: "red" });
42+
exports["default"] = Form;
43+
44+
45+
//// [index.d.ts]
46+
/// <reference path="node_modules/create-emotion-styled/types/react/index.d.ts" />
47+
/// <reference types="react" />
48+
declare const Form: import("create-emotion-styled/types/react").StyledOtherComponent<{}, import("react").DetailedHTMLProps<import("react").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>;
49+
export default Form;

‎tests/baselines/reference/reactTransitiveImportHasValidDeclaration.symbols

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
=== tests/cases/compiler/node_modules/react/index.d.ts ===
2+
declare namespace React {
3+
>React : Symbol(React, Decl(index.d.ts, 0, 0), Decl(index.d.ts, 0, 0))
4+
5+
export interface DetailedHTMLProps<T, U> {}
6+
>DetailedHTMLProps : Symbol(DetailedHTMLProps, Decl(index.d.ts, 0, 25))
7+
>T : Symbol(T, Decl(index.d.ts, 1, 39))
8+
>U : Symbol(U, Decl(index.d.ts, 1, 41))
9+
10+
export interface HTMLAttributes<T> {}
11+
>HTMLAttributes : Symbol(HTMLAttributes, Decl(index.d.ts, 1, 47), Decl(index.d.ts, 1, 24))
12+
>T : Symbol(T, Decl(index.d.ts, 2, 36), Decl(index.d.ts, 2, 29))
13+
}
14+
export = React;
15+
>React : Symbol(React, Decl(index.d.ts, 0, 0))
16+
17+
export as namespace React;
18+
>React : Symbol(React, Decl(index.d.ts, 4, 15))
19+
20+
=== tests/cases/compiler/node_modules/create-emotion-styled/types/react/index.d.ts ===
21+
/// <reference types="react" />
22+
declare module 'react' { // augment
23+
>'react' : Symbol(React, Decl(index.d.ts, 0, 0), Decl(index.d.ts, 0, 0))
24+
25+
interface HTMLAttributes<T> {
26+
>HTMLAttributes : Symbol(HTMLAttributes, Decl(index.d.ts, 1, 47), Decl(index.d.ts, 1, 24))
27+
>T : Symbol(T, Decl(index.d.ts, 2, 36), Decl(index.d.ts, 2, 29))
28+
29+
css?: unknown;
30+
>css : Symbol(HTMLAttributes.css, Decl(index.d.ts, 2, 33))
31+
}
32+
}
33+
export interface StyledOtherComponentList {
34+
>StyledOtherComponentList : Symbol(StyledOtherComponentList, Decl(index.d.ts, 5, 1))
35+
36+
"div": React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement>
37+
>"div" : Symbol(StyledOtherComponentList["div"], Decl(index.d.ts, 6, 43))
38+
>React : Symbol(React, Decl(index.d.ts, 4, 15))
39+
>DetailedHTMLProps : Symbol(DetailedHTMLProps, Decl(index.d.ts, 0, 25))
40+
>React : Symbol(React, Decl(index.d.ts, 4, 15))
41+
>HTMLAttributes : Symbol(HTMLAttributes, Decl(index.d.ts, 1, 47), Decl(index.d.ts, 1, 24))
42+
>HTMLDivElement : Symbol(HTMLDivElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
43+
>HTMLDivElement : Symbol(HTMLDivElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
44+
}
45+
export interface StyledOtherComponent<A, B, C> {}
46+
>StyledOtherComponent : Symbol(StyledOtherComponent, Decl(index.d.ts, 8, 1))
47+
>A : Symbol(A, Decl(index.d.ts, 9, 38))
48+
>B : Symbol(B, Decl(index.d.ts, 9, 40))
49+
>C : Symbol(C, Decl(index.d.ts, 9, 43))
50+
51+
=== tests/cases/compiler/node_modules/create-emotion-styled/index.d.ts ===
52+
export * from "./types/react";
53+
No type information for this code.
54+
No type information for this code.=== tests/cases/compiler/node_modules/react-emotion/index.d.ts ===
55+
import {StyledOtherComponent, StyledOtherComponentList} from "create-emotion-styled";
56+
>StyledOtherComponent : Symbol(StyledOtherComponent, Decl(index.d.ts, 0, 8))
57+
>StyledOtherComponentList : Symbol(StyledOtherComponentList, Decl(index.d.ts, 0, 29))
58+
59+
export default function styled(tag: string): (o: object) => StyledOtherComponent<{}, StyledOtherComponentList["div"], any>;
60+
>styled : Symbol(styled, Decl(index.d.ts, 0, 85))
61+
>tag : Symbol(tag, Decl(index.d.ts, 1, 31))
62+
>o : Symbol(o, Decl(index.d.ts, 1, 46))
63+
>StyledOtherComponent : Symbol(StyledOtherComponent, Decl(index.d.ts, 0, 8))
64+
>StyledOtherComponentList : Symbol(StyledOtherComponentList, Decl(index.d.ts, 0, 29))
65+
66+
=== tests/cases/compiler/index.ts ===
67+
import styled from "react-emotion"
68+
>styled : Symbol(styled, Decl(index.ts, 0, 6))
69+
70+
const Form = styled('div')({ color: "red" })
71+
>Form : Symbol(Form, Decl(index.ts, 2, 5))
72+
>styled : Symbol(styled, Decl(index.ts, 0, 6))
73+
>color : Symbol(color, Decl(index.ts, 2, 28))
74+
75+
export default Form
76+
>Form : Symbol(Form, Decl(index.ts, 2, 5))
77+

‎tests/baselines/reference/reactTransitiveImportHasValidDeclaration.types

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
=== tests/cases/compiler/node_modules/react/index.d.ts ===
2+
declare namespace React {
3+
export interface DetailedHTMLProps<T, U> {}
4+
export interface HTMLAttributes<T> {}
5+
}
6+
export = React;
7+
>React : any
8+
9+
export as namespace React;
10+
>React : any
11+
12+
=== tests/cases/compiler/node_modules/create-emotion-styled/types/react/index.d.ts ===
13+
/// <reference types="react" />
14+
declare module 'react' { // augment
15+
>'react' : any
16+
17+
interface HTMLAttributes<T> {
18+
css?: unknown;
19+
>css : unknown
20+
}
21+
}
22+
export interface StyledOtherComponentList {
23+
"div": React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement>
24+
>"div" : import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>
25+
>React : any
26+
>React : any
27+
}
28+
export interface StyledOtherComponent<A, B, C> {}
29+
30+
=== tests/cases/compiler/node_modules/create-emotion-styled/index.d.ts ===
31+
export * from "./types/react";
32+
No type information for this code.
33+
No type information for this code.=== tests/cases/compiler/node_modules/react-emotion/index.d.ts ===
34+
import {StyledOtherComponent, StyledOtherComponentList} from "create-emotion-styled";
35+
>StyledOtherComponent : any
36+
>StyledOtherComponentList : any
37+
38+
export default function styled(tag: string): (o: object) => StyledOtherComponent<{}, StyledOtherComponentList["div"], any>;
39+
>styled : (tag: string) => (o: object) => StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
40+
>tag : string
41+
>o : object
42+
43+
=== tests/cases/compiler/index.ts ===
44+
import styled from "react-emotion"
45+
>styled : (tag: string) => (o: object) => import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
46+
47+
const Form = styled('div')({ color: "red" })
48+
>Form : import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
49+
>styled('div')({ color: "red" }) : import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
50+
>styled('div') : (o: object) => import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
51+
>styled : (tag: string) => (o: object) => import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
52+
>'div' : "div"
53+
>{ color: "red" } : { color: string; }
54+
>color : string
55+
>"red" : "red"
56+
57+
export default Form
58+
>Form : import("tests/cases/compiler/node_modules/create-emotion-styled/types/react/index").StyledOtherComponent<{}, import("tests/cases/compiler/node_modules/react/index.d.ts").DetailedHTMLProps<import("tests/cases/compiler/node_modules/react/index.d.ts").HTMLAttributes<HTMLDivElement>, HTMLDivElement>, any>
59+

‎tests/baselines/reference/umd-augmentation-3.types

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ var t = p.x;
4646

4747
=== tests/cases/conformance/externalModules/node_modules/math2d/index.d.ts ===
4848
export as namespace Math2d;
49-
>Math2d : typeof import("2")
49+
>Math2d : typeof import("tests/cases/conformance/externalModules/node_modules/math2d/index.d.ts")
5050

5151
export = M2D;
5252
>M2D : typeof M2D
5353

5454
declare namespace M2D {
55-
>M2D : typeof import("2")
55+
>M2D : typeof import("tests/cases/conformance/externalModules/node_modules/math2d/index.d.ts")
5656

5757
interface Point {
5858
x: number;

0 commit comments

Comments
 (0)