Skip to content

fix inconsistencies in import UMD code fixes adapting to module format #19572

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
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: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ namespace ts {
const languageVersion = getEmitScriptTarget(compilerOptions);
const modulekind = getEmitModuleKind(compilerOptions);
const noUnusedIdentifiers = !!compilerOptions.noUnusedLocals || !!compilerOptions.noUnusedParameters;
const allowSyntheticDefaultImports = typeof compilerOptions.allowSyntheticDefaultImports !== "undefined" ? compilerOptions.allowSyntheticDefaultImports : modulekind === ModuleKind.System;
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
const noImplicitAny = getStrictOptionValue(compilerOptions, "noImplicitAny");
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,13 @@ namespace ts {
return moduleResolution;
}

export function getAllowSyntheticDefaultImports(compilerOptions: CompilerOptions) {
const moduleKind = getEmitModuleKind(compilerOptions);
return compilerOptions.allowSyntheticDefaultImports !== undefined
? compilerOptions.allowSyntheticDefaultImports
: moduleKind === ModuleKind.System;
}

export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "alwaysStrict";

export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3801,5 +3801,13 @@
"Install '{0}'": {
"category": "Message",
"code": 95014
},
"Import '{0}' = require(\"{1}\").": {
"category": "Message",
"code": 95015
},
"Import * as '{0}' from \"{1}\".": {
"category": "Message",
"code": 95016
}
}
39 changes: 28 additions & 11 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ namespace ts.codefix {
export const enum ImportKind {
Named,
Default,
Namespace,
Namespace
}

export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixOptions): ImportCodeAction[] {
Expand Down Expand Up @@ -212,7 +212,7 @@ namespace ts.codefix {

function getNamespaceImportName(declaration: AnyImportSyntax): Identifier {
if (declaration.kind === SyntaxKind.ImportDeclaration) {
const namedBindings = declaration.importClause && declaration.importClause.namedBindings;
const namedBindings = declaration.importClause && isImportClause(declaration.importClause) && declaration.importClause.namedBindings;
return namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport ? namedBindings.name : undefined;
}
else {
Expand All @@ -237,6 +237,8 @@ namespace ts.codefix {
return parent as ImportDeclaration;
case SyntaxKind.ExternalModuleReference:
return (parent as ExternalModuleReference).parent;
case SyntaxKind.ImportEqualsDeclaration:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will never be the parent of a LiteralExpression. Removing in #19667.

return parent as ImportEqualsDeclaration;
default:
Debug.assert(parent.kind === SyntaxKind.ExportDeclaration);
// Ignore these, can't add imports to them.
Expand All @@ -249,11 +251,13 @@ namespace ts.codefix {
const lastImportDeclaration = findLast(sourceFile.statements, isAnyImportSyntax);

const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier);
const quotedModuleSpecifier = createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes);
const importDecl = createImportDeclaration(
/*decorators*/ undefined,
/*modifiers*/ undefined,
/*decorators*/ undefined,
/*modifiers*/ undefined,
createImportClauseOfKind(kind, symbolName),
createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes));
quotedModuleSpecifier);

const changes = ChangeTracker.with(context, changeTracker => {
if (lastImportDeclaration) {
changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter });
Expand All @@ -263,11 +267,15 @@ namespace ts.codefix {
}
});

const actionFormat = kind === ImportKind.Namespace
? Diagnostics.Import_Asterisk_as_0_from_1
: Diagnostics.Import_0_from_1;

// if this file doesn't have any import statements, insert an import statement and then insert a new line
// between the only import statement and user code. Otherwise just insert the statement because chances
// are there are already a new line seperating code and import statements.
return createCodeAction(
Diagnostics.Import_0_from_1,
actionFormat,
[symbolName, moduleSpecifierWithoutQuotes],
changes,
"NewImport",
Expand All @@ -282,7 +290,7 @@ namespace ts.codefix {
return literal;
}

function createImportClauseOfKind(kind: ImportKind, symbolName: string) {
function createImportClauseOfKind(kind: ImportKind.Default | ImportKind.Named | ImportKind.Namespace, symbolName: string) {
const id = createIdentifier(symbolName);
switch (kind) {
case ImportKind.Default:
Expand Down Expand Up @@ -534,7 +542,7 @@ namespace ts.codefix {
declarations: ReadonlyArray<AnyImportSyntax>): ImportCodeAction {
const fromExistingImport = firstDefined(declarations, declaration => {
if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) {
const changes = tryUpdateExistingImport(ctx, declaration.importClause);
const changes = tryUpdateExistingImport(ctx, isImportClause(declaration.importClause) && declaration.importClause || undefined);
if (changes) {
const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText());
return createCodeAction(
Expand Down Expand Up @@ -564,9 +572,10 @@ namespace ts.codefix {
return expression && isStringLiteral(expression) ? expression.text : undefined;
}

function tryUpdateExistingImport(context: SymbolContext & { kind: ImportKind }, importClause: ImportClause): FileTextChanges[] | undefined {
function tryUpdateExistingImport(context: SymbolContext & { kind: ImportKind }, importClause: ImportClause | ImportEqualsDeclaration): FileTextChanges[] | undefined {
const { symbolName, sourceFile, kind } = context;
const { name, namedBindings } = importClause;
const { name } = importClause;
const { namedBindings } = importClause.kind !== SyntaxKind.ImportEqualsDeclaration && importClause;
switch (kind) {
case ImportKind.Default:
return name ? undefined : ChangeTracker.with(context, t =>
Expand Down Expand Up @@ -627,7 +636,7 @@ namespace ts.codefix {
}

function getActionsForUMDImport(context: ImportCodeFixContext): ImportCodeAction[] {
const { checker, symbolToken } = context;
const { checker, symbolToken, compilerOptions } = context;
const umdSymbol = checker.getSymbolAtLocation(symbolToken);
let symbol: ts.Symbol;
let symbolName: string;
Expand All @@ -644,6 +653,14 @@ namespace ts.codefix {
Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here");
}

const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);

// Import a synthetic `default` if enabled.
if (allowSyntheticDefaultImports) {
return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Default });
}

// Fall back to the `import * as ns` style import.
return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Namespace });
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="fourslash.ts" />
// @AllowSyntheticDefaultImports: true

// @Filename: a/f1.ts
//// [|export var x = 0;
//// bar/*0*/();|]

// @Filename: a/foo.d.ts
//// declare function bar(): number;
//// export = bar;
//// export as namespace bar;

verify.importFixAtPosition([
`import bar from "./foo";

export var x = 0;
bar();`
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="fourslash.ts" />
// @Module: system

// @Filename: a/f1.ts
//// [|export var x = 0;
//// bar/*0*/();|]

// @Filename: a/foo.d.ts
//// declare function bar(): number;
//// export = bar;
//// export as namespace bar;

verify.importFixAtPosition([
`import bar from "./foo";

export var x = 0;
bar();`
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="fourslash.ts" />
// @AllowSyntheticDefaultImports: false

// @Filename: a/f1.ts
//// [|export var x = 0;
//// bar/*0*/();|]

// @Filename: a/foo.d.ts
//// declare function bar(): number;
//// export = bar;
//// export as namespace bar;

verify.importFixAtPosition([
`import * as bar from "./foo";

export var x = 0;
bar();`
]);