Check if module reference requires type reference before path reference (microsoft#25385)

weswigham · web-flow · commit cb4cb7a75ff1 · 2018-07-03T11:41:07.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -3747,7 +3747,7 @@ namespace ts {
                         const ambientDecls = filter(symbol.declarations, isAmbientModule);
                         if (length(ambientDecls)) {
                             for (const decl of ambientDecls) {
-                                context.tracker.trackReferencedAmbientModule(decl);
+                                context.tracker.trackReferencedAmbientModule(decl, symbol);
                             }
                         }
                     }
diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts
@@ -66,7 +66,13 @@ namespace ts {
             }
         }
 
-        function trackReferencedAmbientModule(node: ModuleDeclaration) {
+        function trackReferencedAmbientModule(node: ModuleDeclaration, symbol: Symbol) {
+            // If it is visible via `// <reference types="..."/>`, then we should just use that
+            const directives = resolver.getTypeReferenceDirectivesForSymbol(symbol, SymbolFlags.All);
+            if (length(directives)) {
+                return recordTypeReferenceDirectivesIfNecessary(directives);
+            }
+            // Otherwise we should emit a path-based reference
             const container = getSourceFileOfNode(node);
             refs.set("" + getOriginalNodeId(container), container);
         }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5316,7 +5316,7 @@ namespace ts {
         reportPrivateInBaseOfClassExpression?(propertyName: string): void;
         reportInaccessibleUniqueSymbolError?(): void;
         moduleResolverHost?: ModuleSpecifierResolutionHost;
-        trackReferencedAmbientModule?(decl: ModuleDeclaration): void;
+        trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;
     }
 
     export interface TextSpan {
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -4497,7 +4497,7 @@ declare namespace ts {
         reportPrivateInBaseOfClassExpression?(propertyName: string): void;
         reportInaccessibleUniqueSymbolError?(): void;
         moduleResolverHost?: ModuleSpecifierResolutionHost;
-        trackReferencedAmbientModule?(decl: ModuleDeclaration): void;
+        trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;
     }
     interface TextSpan {
         start: number;
diff --git a/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.js b/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.js
@@ -0,0 +1,22 @@
+//// [tests/cases/compiler/referenceTypesPreferedToPathIfPossible.ts] ////
+
+//// [index.d.ts]
+declare module "url" {
+    export class Url {}
+    export function parse(): Url; 
+}
+//// [usage.ts]
+import { parse } from "url";
+export const thing = () => parse();
+
+
+//// [usage.js]
+"use strict";
+exports.__esModule = true;
+var url_1 = require("url");
+exports.thing = function () { return url_1.parse(); };
+
+
+//// [usage.d.ts]
+/// <reference types="node" />
+export declare const thing: () => import("url").Url;
diff --git a/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.symbols b/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.symbols
@@ -0,0 +1,19 @@
+=== tests/cases/compiler/usage.ts ===
+import { parse } from "url";
+>parse : Symbol(parse, Decl(usage.ts, 0, 8))
+
+export const thing = () => parse();
+>thing : Symbol(thing, Decl(usage.ts, 1, 12))
+>parse : Symbol(parse, Decl(usage.ts, 0, 8))
+
+=== node_modules/@types/node/index.d.ts ===
+declare module "url" {
+>"url" : Symbol("url", Decl(index.d.ts, 0, 0))
+
+    export class Url {}
+>Url : Symbol(Url, Decl(index.d.ts, 0, 22))
+
+    export function parse(): Url; 
+>parse : Symbol(parse, Decl(index.d.ts, 1, 23))
+>Url : Symbol(Url, Decl(index.d.ts, 0, 22))
+}
diff --git a/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.types b/tests/baselines/reference/referenceTypesPreferedToPathIfPossible.types
@@ -0,0 +1,21 @@
+=== tests/cases/compiler/usage.ts ===
+import { parse } from "url";
+>parse : () => import("url").Url
+
+export const thing = () => parse();
+>thing : () => import("url").Url
+>() => parse() : () => import("url").Url
+>parse() : import("url").Url
+>parse : () => import("url").Url
+
+=== node_modules/@types/node/index.d.ts ===
+declare module "url" {
+>"url" : typeof import("url")
+
+    export class Url {}
+>Url : Url
+
+    export function parse(): Url; 
+>parse : () => Url
+>Url : Url
+}
diff --git a/tests/cases/compiler/referenceTypesPreferedToPathIfPossible.ts b/tests/cases/compiler/referenceTypesPreferedToPathIfPossible.ts
@@ -0,0 +1,10 @@
+// @declaration: true
+// @noImplicitReferences: true
+// @filename: /.src/node_modules/@types/node/index.d.ts
+declare module "url" {
+    export class Url {}
+    export function parse(): Url; 
+}
+// @filename: usage.ts
+import { parse } from "url";
+export const thing = () => parse();

Original file line number	Diff line number	Diff line change
`@@ -3747,7 +3747,7 @@ namespace ts {`
`3747`	`3747`	`const ambientDecls = filter(symbol.declarations, isAmbientModule);`
`3748`	`3748`	`if (length(ambientDecls)) {`
`3749`	`3749`	`for (const decl of ambientDecls) {`
`3750`		`- context.tracker.trackReferencedAmbientModule(decl);`
	`3750`	`+ context.tracker.trackReferencedAmbientModule(decl, symbol);`
`3751`	`3751`	`}`
`3752`	`3752`	`}`
`3753`	`3753`	`}`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,13 @@ namespace ts {`
`66`	`66`	`}`
`67`	`67`	`}`
`68`	`68`
`69`		`- function trackReferencedAmbientModule(node: ModuleDeclaration) {`
	`69`	`+ function trackReferencedAmbientModule(node: ModuleDeclaration, symbol: Symbol) {`
	`70`	+ // If it is visible via `// <reference types="..."/>`, then we should just use that
	`71`	`+ const directives = resolver.getTypeReferenceDirectivesForSymbol(symbol, SymbolFlags.All);`
	`72`	`+ if (length(directives)) {`
	`73`	`+ return recordTypeReferenceDirectivesIfNecessary(directives);`
	`74`	`+ }`
	`75`	`+ // Otherwise we should emit a path-based reference`
`70`	`76`	`const container = getSourceFileOfNode(node);`
`71`	`77`	`refs.set("" + getOriginalNodeId(container), container);`
`72`	`78`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5316,7 +5316,7 @@ namespace ts {`
`5316`	`5316`	`reportPrivateInBaseOfClassExpression?(propertyName: string): void;`
`5317`	`5317`	`reportInaccessibleUniqueSymbolError?(): void;`
`5318`	`5318`	`moduleResolverHost?: ModuleSpecifierResolutionHost;`
`5319`		`- trackReferencedAmbientModule?(decl: ModuleDeclaration): void;`
	`5319`	`+ trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;`
`5320`	`5320`	`}`
`5321`	`5321`
`5322`	`5322`	`export interface TextSpan {`
Original file line number	Diff line number	Diff line change
`@@ -4497,7 +4497,7 @@ declare namespace ts {`
`4497`	`4497`	`reportPrivateInBaseOfClassExpression?(propertyName: string): void;`
`4498`	`4498`	`reportInaccessibleUniqueSymbolError?(): void;`
`4499`	`4499`	`moduleResolverHost?: ModuleSpecifierResolutionHost;`
`4500`		`- trackReferencedAmbientModule?(decl: ModuleDeclaration): void;`
	`4500`	`+ trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;`
`4501`	`4501`	`}`
`4502`	`4502`	`interface TextSpan {`
`4503`	`4503`	`start: number;`