Skip to content

Support import fix/completions for export = #25708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
4 commits merged into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2269,8 +2269,6 @@ namespace ts {
r && r.packageId && r.packageId.name === getTypesPackageName(packageName)));
}

// An external module with an 'export =' declaration resolves to the target of the 'export =' declaration,
// and an external module with no 'export =' declaration resolves to the module itself.
function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol;
function resolveExternalModuleSymbol(moduleSymbol: Symbol | undefined, dontResolveAlias?: boolean): Symbol | undefined;
function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol {
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3097,6 +3097,10 @@ namespace ts {
*/
/* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] | undefined;
/* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate;
/**
* An external module with an 'export =' declaration resolves to the target of the 'export =' declaration,
* and an external module with no 'export =' declaration resolves to the module itself.
*/
/* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol;
/** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */
/* @internal */ tryGetThisTypeAt(node: Node): Type | undefined;
Expand Down
6 changes: 4 additions & 2 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,8 @@ namespace FourSlash {
}

private verifyCompletionEntry(actual: ts.CompletionEntry, expected: FourSlashInterface.ExpectedCompletionEntry) {
const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, sourceDisplay } = typeof expected === "string"
? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, sourceDisplay: undefined }
const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, source, sourceDisplay } = typeof expected === "string"
? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, source: undefined, sourceDisplay: undefined }
: expected;

if (actual.insertText !== insertText) {
Expand All @@ -927,6 +927,7 @@ namespace FourSlash {

assert.equal(actual.hasAction, hasAction);
assert.equal(actual.isRecommended, isRecommended);
assert.equal(actual.source, source);

if (text) {
const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source)!;
Expand Down Expand Up @@ -4789,6 +4790,7 @@ namespace FourSlashInterface {

export type ExpectedCompletionEntry = string | {
readonly name: string,
readonly source?: string,
readonly insertText?: string,
readonly replacementSpan?: FourSlash.Range,
readonly hasAction?: boolean, // If not specified, will assert that this is false.
Expand Down
49 changes: 33 additions & 16 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ namespace ts.codefix {
position: number,
preferences: UserPreferences,
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getTypeChecker(), program.getSourceFiles());
const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles());
Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol));
// We sort the best codefixes first, so taking `first` is best for completions.
const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences)).moduleSpecifier;
Expand All @@ -175,18 +175,22 @@ namespace ts.codefix {
return { description, changes, commands };
}

function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>): ReadonlyArray<SymbolExportInfo> {
function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>): ReadonlyArray<SymbolExportInfo> {
const result: SymbolExportInfo[] = [];
forEachExternalModule(checker, allSourceFiles, (moduleSymbol, moduleFile) => {
// Don't import from a re-export when looking "up" like to `./index` or `../index`.
if (moduleFile && moduleSymbol !== exportingModuleSymbol && startsWith(sourceFile.fileName, getDirectoryPath(moduleFile.fileName))) {
return;
}

const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
if (defaultInfo && defaultInfo.name === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) {
result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol) });
}

for (const exported of checker.getExportsOfModule(moduleSymbol)) {
if ((exported.escapedName === InternalSymbolName.Default || exported.name === symbolName) && skipAlias(exported, checker) === exportedSymbol) {
const isDefaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol) === exported;
result.push({ moduleSymbol, importKind: isDefaultExport ? ImportKind.Default : ImportKind.Named, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exported) });
if (exported.name === symbolName && skipAlias(exported, checker) === exportedSymbol) {
result.push({ moduleSymbol, importKind: ImportKind.Named, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exported) });
}
}
});
Expand Down Expand Up @@ -400,13 +404,9 @@ namespace ts.codefix {
forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => {
cancellationToken.throwIfCancellationRequested();

// check the default export
const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol);
if (defaultExport) {
const info = getDefaultExportInfo(defaultExport, moduleSymbol, program);
if (info && info.name === symbolName && symbolHasMeaning(info.symbolForMeaning, currentTokenMeaning)) {
addSymbol(moduleSymbol, defaultExport, ImportKind.Default);
}
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions());
if (defaultInfo && defaultInfo.name === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) {
addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind);
}

// check exports with the same name
Expand All @@ -418,19 +418,36 @@ namespace ts.codefix {
return originalSymbolToExportInfos;
}

function getDefaultExportInfo(defaultExport: Symbol, moduleSymbol: Symbol, program: Program): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined {
function getDefaultLikeExportInfo(
moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions,
): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined {
const exported = getDefaultLikeExportWorker(moduleSymbol, checker);
if (!exported) return undefined;
const { symbol, kind } = exported;
const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions);
return info && { symbol, symbolForMeaning: info.symbolForMeaning, name: info.name, kind };
}

function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined {
const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol);
if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default };
const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol);
return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: ImportKind.Equals };
}

function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined {
const localSymbol = getLocalSymbolForExportDefault(defaultExport);
if (localSymbol) return { symbolForMeaning: localSymbol, name: localSymbol.name };

const name = getNameForExportDefault(defaultExport);
if (name !== undefined) return { symbolForMeaning: defaultExport, name };

if (defaultExport.flags & SymbolFlags.Alias) {
const aliased = program.getTypeChecker().getImmediateAliasedSymbol(defaultExport);
return aliased && getDefaultExportInfo(aliased, Debug.assertDefined(aliased.parent), program);
const aliased = checker.getImmediateAliasedSymbol(defaultExport);
return aliased && getDefaultExportInfoWorker(aliased, Debug.assertDefined(aliased.parent), checker, compilerOptions);
}
else {
return { symbolForMeaning: defaultExport, name: moduleSymbolToValidIdentifier(moduleSymbol, program.getCompilerOptions().target!) };
return { symbolForMeaning: defaultExport, name: moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target!) };
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,14 @@ namespace ts.Completions {
return;
}

if (resolvedModuleSymbol !== moduleSymbol &&
// Don't add another completion for `export =` of a symbol that's already global.
// So in `declare namespace foo {} declare module "foo" { export = foo; }`, there will just be the global completion for `foo`.
resolvedModuleSymbol.declarations.some(d => !!d.getSourceFile().externalModuleIndicator)) {
symbols.push(resolvedModuleSymbol);
symbolToOriginInfoMap[getSymbolId(resolvedModuleSymbol)] = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport: false };
}

for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) {
// Don't add a completion for a re-export, only for the original.
// The actual import fix might end up coming from a re-export -- we don't compute that until getting completion details.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
////fooB/*1*/

goTo.marker("0");
const preferences = { includeCompletionsForModuleExports: true };
const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true };
verify.completions(
{ marker: "0", excludes: { name: "default", source: "/src/foo-bar" }, preferences },
{ marker: "1", includes: { name: "fooBar", source: "/src/foo-bar", sourceDisplay: "./foo-bar", text: "(property) default: 0", kind: "property", hasAction: true }, preferences }
Expand Down
53 changes: 53 additions & 0 deletions tests/cases/fourslash/completionsImport_exportEquals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/// <reference path="fourslash.ts" />

// @module: commonjs

// @Filename: /a.d.ts
////declare function a(): void;
////declare namespace a {
//// export interface b {}
////}
////export = a;

// @Filename: /b.ts
////a/*0*/;
////let x: b/*1*/;

const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true };
verify.completions(
{
marker: "0",
includes: { name: "a", source: "/a", hasAction: true, },
preferences,
},
{
marker: "1",
includes: { name: "b", source: "/a", hasAction: true },
preferences,
}
);

// Import { b } first, or it will just add a qualified name from 'a' (which isn't what we're trying to test)
verify.applyCodeActionFromCompletion("1", {
name: "b",
source: "/a",
description: `Import 'b' from module "./a"`,
newFileContent:
`import { b } from "./a";

a;
let x: b;`,
});

verify.applyCodeActionFromCompletion("0", {
name: "a",
source: "/a",
description: `Import 'a' from module "./a"`,
newFileContent:
`import { b } from "./a";
import a = require("./a");

a;
let x: b;`,
});

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
verify.completions({
marker: "",
// Tester will assert that it is only included once
includes: [{ name: "foo", hasAction: true }],
includes: [{ name: "foo", source: "a", hasAction: true }],
preferences: {
includeCompletionsForModuleExports: true,
}
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/completionsImport_tsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

verify.completions({
marker: "",
includes: { name: "Foo", source: "/a.tsx", hasAction: true },
includes: { name: "Foo", source: "/a", hasAction: true },
excludes: "Bar",
preferences: {
includeCompletionsForModuleExports: true,
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/completionsUniqueSymbol_import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ verify.completions({
marker: "",
exact: [
"n",
{ name: "publicSym", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true },
{ name: "publicSym", source: "/a", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true },
],
preferences: {
includeInsertTextCompletions: true,
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/importNameCodeFix_all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ verify.codeFixAll({
fixId: "fixMissingImport",
fixAllDescription: "Add all missing imports",
newFileContent:
// TODO: GH#25135 (should import 'e')
`import bd, * as b from "./b";
import cd, { c0 } from "./c";
import dd, { d0, d1 } from "./d";
import ad, { a0 } from "./a";
import e = require("./e");

ad; ad; a0; a0;
bd; bd; b.b0; b.b0;
Expand Down
25 changes: 25 additions & 0 deletions tests/cases/fourslash/importNameCodeFix_exportEquals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/// <reference path="fourslash.ts" />

// @Filename: /a.d.ts
////declare function a(): void;
////declare namespace a {
//// export interface b {}
////}
////export = a;

// @Filename: /b.ts
////a;
////let x: b;

goTo.file("/b.ts");
verify.codeFixAll({
fixId: "fixMissingImport",
fixAllDescription: "Add all missing imports",
newFileContent:
`import { b } from "./a";

import a = require("./a");

a;
let x: b;`,
});