Skip to content

Commit

Permalink
feat(37440): Provide a quick-fix for non-exported types (#51038)
Browse files Browse the repository at this point in the history
* feat(37440): add QF to handle missing exports

* change diagnostic message

* add type modifier only if isolatedModules is set or if the export declaration already uses type modifiers
  • Loading branch information
a-tarasyuk committed Oct 13, 2022
1 parent a24201c commit 2bcfed0
Show file tree
Hide file tree
Showing 27 changed files with 725 additions and 35 deletions.
31 changes: 0 additions & 31 deletions src/compiler/checker.ts
Expand Up @@ -7235,16 +7235,6 @@ namespace ts {
return statements;
}

function canHaveExportModifier(node: Statement): node is Extract<HasModifiers, Statement> {
return isEnumDeclaration(node) ||
isVariableStatement(node) ||
isFunctionDeclaration(node) ||
isClassDeclaration(node) ||
(isModuleDeclaration(node) && !isExternalModuleAugmentation(node) && !isGlobalScopeAugmentation(node)) ||
isInterfaceDeclaration(node) ||
isTypeDeclaration(node);
}

function addExportModifier(node: Extract<HasModifiers, Statement>) {
const flags = (getEffectiveModifierFlags(node) | ModifierFlags.Export) & ~ModifierFlags.Ambient;
return factory.updateModifiers(node, flags);
Expand Down Expand Up @@ -42691,27 +42681,6 @@ namespace ts {
getNameOfDeclaration(name.parent) === name;
}

function isTypeDeclaration(node: Node): node is TypeParameterDeclaration | ClassDeclaration | InterfaceDeclaration | TypeAliasDeclaration | JSDocTypedefTag | JSDocCallbackTag | JSDocEnumTag | EnumDeclaration | ImportClause | ImportSpecifier | ExportSpecifier {
switch (node.kind) {
case SyntaxKind.TypeParameter:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.JSDocTypedefTag:
case SyntaxKind.JSDocCallbackTag:
case SyntaxKind.JSDocEnumTag:
return true;
case SyntaxKind.ImportClause:
return (node as ImportClause).isTypeOnly;
case SyntaxKind.ImportSpecifier:
case SyntaxKind.ExportSpecifier:
return (node as ImportSpecifier | ExportSpecifier).parent.parent.isTypeOnly;
default:
return false;
}
}

// True if the given identifier is part of a type reference
function isTypeReferenceIdentifier(node: EntityName): boolean {
while (node.parent.kind === SyntaxKind.QualifiedName) {
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -6687,6 +6687,14 @@
"category": "Message",
"code": 90058
},
"Export '{0}' from module '{1}'": {
"category": "Message",
"code": 90059
},
"Export all referenced locals": {
"category": "Message",
"code": 90060
},

"Convert function to an ES2015 class": {
"category": "Message",
Expand Down
26 changes: 26 additions & 0 deletions src/compiler/utilities.ts
Expand Up @@ -7752,4 +7752,30 @@ namespace ts {
export function getParameterTypeNode(parameter: ParameterDeclaration | JSDocParameterTag) {
return parameter.kind === SyntaxKind.JSDocParameterTag ? parameter.typeExpression?.type : parameter.type;
}

export function isTypeDeclaration(node: Node): node is TypeParameterDeclaration | ClassDeclaration | InterfaceDeclaration | TypeAliasDeclaration | JSDocTypedefTag | JSDocCallbackTag | JSDocEnumTag | EnumDeclaration | ImportClause | ImportSpecifier | ExportSpecifier {
switch (node.kind) {
case SyntaxKind.TypeParameter:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.JSDocTypedefTag:
case SyntaxKind.JSDocCallbackTag:
case SyntaxKind.JSDocEnumTag:
return true;
case SyntaxKind.ImportClause:
return (node as ImportClause).isTypeOnly;
case SyntaxKind.ImportSpecifier:
case SyntaxKind.ExportSpecifier:
return (node as ImportSpecifier | ExportSpecifier).parent.parent.isTypeOnly;
default:
return false;
}
}

export function canHaveExportModifier(node: Node): node is Extract<HasModifiers, Statement> {
return isEnumDeclaration(node) || isVariableStatement(node) || isFunctionDeclaration(node) || isClassDeclaration(node)
|| isInterfaceDeclaration(node) || isTypeDeclaration(node) || (isModuleDeclaration(node) && !isExternalModuleAugmentation(node) && !isGlobalScopeAugmentation(node));
}
}
4 changes: 0 additions & 4 deletions src/services/codefixes/fixAddMissingMember.ts
Expand Up @@ -264,10 +264,6 @@ namespace ts.codefix {
return undefined;
}

function isSourceFileFromLibrary(program: Program, node: SourceFile) {
return program.isSourceFileFromExternalLibrary(node) || program.isSourceFileDefaultLibrary(node);
}

function getActionsForMissingMemberDeclaration(context: CodeFixContext, info: TypeLikeDeclarationInfo): CodeFixAction[] | undefined {
return info.isJSFile ? singleElementArray(createActionForAddMissingMemberInJavascriptFile(context, info)) :
createActionsForAddMissingMemberInTypeScriptFile(context, info);
Expand Down
163 changes: 163 additions & 0 deletions src/services/codefixes/fixImportNonExportedMember.ts
@@ -0,0 +1,163 @@
/* @internal */
namespace ts.codefix {
const fixId = "fixImportNonExportedMember";
const errorCodes = [
Diagnostics.Module_0_declares_1_locally_but_it_is_not_exported.code,
];

registerCodeFix({
errorCodes,
fixIds: [fixId],
getCodeActions(context) {
const { sourceFile, span, program } = context;
const info = getInfo(sourceFile, span.start, program);
if (info === undefined) return undefined;

const changes = textChanges.ChangeTracker.with(context, t => doChange(t, program, info));
return [createCodeFixAction(fixId, changes, [Diagnostics.Export_0_from_module_1, info.exportName.node.text, info.moduleSpecifier], fixId, Diagnostics.Export_all_referenced_locals)];
},
getAllCodeActions(context) {
const { program } = context;
return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => {
const exports = new Map<SourceFile, ModuleExports>();

eachDiagnostic(context, errorCodes, diag => {
const info = getInfo(diag.file, diag.start, program);
if (info === undefined) return undefined;

const { exportName, node, moduleSourceFile } = info;
if (tryGetExportDeclaration(moduleSourceFile, exportName.isTypeOnly) === undefined && canHaveExportModifier(node)) {
changes.insertExportModifier(moduleSourceFile, node);
}
else {
const moduleExports = exports.get(moduleSourceFile) || { typeOnlyExports: [], exports: [] };
if (exportName.isTypeOnly) {
moduleExports.typeOnlyExports.push(exportName);
}
else {
moduleExports.exports.push(exportName);
}
exports.set(moduleSourceFile, moduleExports);
}
});

exports.forEach((moduleExports, moduleSourceFile) => {
const exportDeclaration = tryGetExportDeclaration(moduleSourceFile, /*isTypeOnly*/ true);
if (exportDeclaration && exportDeclaration.isTypeOnly) {
doChanges(changes, program, moduleSourceFile, moduleExports.typeOnlyExports, exportDeclaration);
doChanges(changes, program, moduleSourceFile, moduleExports.exports, tryGetExportDeclaration(moduleSourceFile, /*isTypeOnly*/ false));
}
else {
doChanges(changes, program, moduleSourceFile, [...moduleExports.exports, ...moduleExports.typeOnlyExports], exportDeclaration);
}
});
}));
}
});

interface ModuleExports {
typeOnlyExports: ExportName[];
exports: ExportName[];
}

interface ExportName {
node: Identifier;
isTypeOnly: boolean;
}

interface Info {
exportName: ExportName;
node: Declaration | VariableStatement;
moduleSourceFile: SourceFile;
moduleSpecifier: string;
}

function getInfo(sourceFile: SourceFile, pos: number, program: Program): Info | undefined {
const token = getTokenAtPosition(sourceFile, pos);
if (isIdentifier(token)) {
const importDeclaration = findAncestor(token, isImportDeclaration);
if (importDeclaration === undefined) return undefined;

const moduleSpecifier = isStringLiteral(importDeclaration.moduleSpecifier) ? importDeclaration.moduleSpecifier.text : undefined;
if (moduleSpecifier === undefined) return undefined;

const resolvedModule = getResolvedModule(sourceFile, moduleSpecifier, /*mode*/ undefined);
if (resolvedModule === undefined) return undefined;

const moduleSourceFile = program.getSourceFile(resolvedModule.resolvedFileName);
if (moduleSourceFile === undefined || isSourceFileFromLibrary(program, moduleSourceFile)) return undefined;

const moduleSymbol = moduleSourceFile.symbol;
const locals = moduleSymbol.valueDeclaration?.locals;
if (locals === undefined) return undefined;

const localSymbol = locals.get(token.escapedText);
if (localSymbol === undefined) return undefined;

const node = getNodeOfSymbol(localSymbol);
if (node === undefined) return undefined;

const exportName = { node: token, isTypeOnly: isTypeDeclaration(node) };
return { exportName, node, moduleSourceFile, moduleSpecifier };
}
return undefined;
}

function doChange(changes: textChanges.ChangeTracker, program: Program, { exportName, node, moduleSourceFile }: Info) {
const exportDeclaration = tryGetExportDeclaration(moduleSourceFile, exportName.isTypeOnly);
if (exportDeclaration) {
updateExport(changes, program, moduleSourceFile, exportDeclaration, [exportName]);
}
else if (canHaveExportModifier(node)) {
changes.insertExportModifier(moduleSourceFile, node);
}
else {
createExport(changes, program, moduleSourceFile, [exportName]);
}
}

function doChanges(changes: textChanges.ChangeTracker, program: Program, sourceFile: SourceFile, moduleExports: ExportName[], node: ExportDeclaration | undefined) {
if (length(moduleExports)) {
if (node) {
updateExport(changes, program, sourceFile, node, moduleExports);
}
else {
createExport(changes, program, sourceFile, moduleExports);
}
}
}

function tryGetExportDeclaration(sourceFile: SourceFile, isTypeOnly: boolean) {
const predicate = (node: Node): node is ExportDeclaration =>
isExportDeclaration(node) && (isTypeOnly && node.isTypeOnly || !node.isTypeOnly);
return findLast(sourceFile.statements, predicate);
}

function updateExport(changes: textChanges.ChangeTracker, program: Program, sourceFile: SourceFile, node: ExportDeclaration, names: ExportName[]) {
const namedExports = node.exportClause && isNamedExports(node.exportClause) ? node.exportClause.elements : factory.createNodeArray([]);
const allowTypeModifier = !node.isTypeOnly && !!(program.getCompilerOptions().isolatedModules || find(namedExports, e => e.isTypeOnly));
changes.replaceNode(sourceFile, node,
factory.updateExportDeclaration(node, node.modifiers, node.isTypeOnly,
factory.createNamedExports(
factory.createNodeArray([...namedExports, ...createExportSpecifiers(names, allowTypeModifier)], /*hasTrailingComma*/ namedExports.hasTrailingComma)), node.moduleSpecifier, node.assertClause));
}

function createExport(changes: textChanges.ChangeTracker, program: Program, sourceFile: SourceFile, names: ExportName[]) {
changes.insertNodeAtEndOfScope(sourceFile, sourceFile,
factory.createExportDeclaration(/*modifiers*/ undefined, /*isTypeOnly*/ false,
factory.createNamedExports(createExportSpecifiers(names, /*allowTypeModifier*/ !!program.getCompilerOptions().isolatedModules)), /*moduleSpecifier*/ undefined, /*assertClause*/ undefined));
}

function createExportSpecifiers(names: ExportName[], allowTypeModifier: boolean) {
return factory.createNodeArray(map(names, n => factory.createExportSpecifier(allowTypeModifier && n.isTypeOnly, /*propertyName*/ undefined, n.node)));
}

function getNodeOfSymbol(symbol: Symbol) {
if (symbol.valueDeclaration === undefined) {
return firstOrUndefined(symbol.declarations);
}
const declaration = symbol.valueDeclaration;
const variableStatement = isVariableDeclaration(declaration) ? tryCast(declaration.parent.parent, isVariableStatement) : undefined;
return variableStatement && length(variableStatement.declarationList.declarations) === 1 ? variableStatement : declaration;
}
}
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Expand Up @@ -71,6 +71,7 @@
"codefixes/fixOverrideModifier.ts",
"codefixes/fixNoPropertyAccessFromIndexSignature.ts",
"codefixes/fixImplicitThis.ts",
"codefixes/fixImportNonExportedMember.ts",
"codefixes/fixIncorrectNamedTupleSyntax.ts",
"codefixes/fixSpelling.ts",
"codefixes/returnValueCorrect.ts",
Expand Down
4 changes: 4 additions & 0 deletions src/services/utilities.ts
Expand Up @@ -3423,5 +3423,9 @@ namespace ts {
return jsx === JsxEmit.React || jsx === JsxEmit.ReactNative;
}

export function isSourceFileFromLibrary(program: Program, node: SourceFile) {
return program.isSourceFileFromExternalLibrary(node) || program.isSourceFileDefaultLibrary(node);
}

// #endregion
}
22 changes: 22 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember1.ts
@@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />

// @module: esnext
// @filename: /a.ts
////declare function foo(): any
////declare function bar(): any;
////export { foo };

// @filename: /b.ts
////import { bar } from "./a";

goTo.file("/b.ts");
verify.codeFix({
description: [ts.Diagnostics.Export_0_from_module_1.message, "bar", "./a"],
index: 0,
newFileContent: {
"/a.ts":
`declare function foo(): any
declare function bar(): any;
export { foo, bar };`,
}
});
26 changes: 26 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember10.ts
@@ -0,0 +1,26 @@
/// <reference path="fourslash.ts" />

// @module: esnext
// @filename: /a.ts
/////**
//// * foo
//// */
////function foo() {}
////export const bar = 1;

// @filename: /b.ts
////import { foo } from "./a";

goTo.file("/b.ts");
verify.codeFix({
description: [ts.Diagnostics.Export_0_from_module_1.message, "foo", "./a"],
index: 0,
newFileContent: {
"/a.ts":
`/**
* foo
*/
export function foo() {}
export const bar = 1;`,
}
});
21 changes: 21 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember11.ts
@@ -0,0 +1,21 @@
/// <reference path="fourslash.ts" />

// @module: esnext
// @isolatedModules: true
// @filename: /a.ts
////type T = {};
////export {};

// @filename: /b.ts
////import { T } from "./a";

goTo.file("/b.ts");
verify.codeFix({
description: [ts.Diagnostics.Export_0_from_module_1.message, "T", "./a"],
index: 0,
newFileContent: {
"/a.ts":
`type T = {};
export { type T };`,
}
});
22 changes: 22 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember12.ts
@@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />

// @module: esnext
// @filename: /a.ts
////type T1 = {};
////type T2 = {};
////export { type T1 };

// @filename: /b.ts
////import { T2 } from "./a";

goTo.file("/b.ts");
verify.codeFix({
description: [ts.Diagnostics.Export_0_from_module_1.message, "T2", "./a"],
index: 0,
newFileContent: {
"/a.ts":
`type T1 = {};
type T2 = {};
export { type T1, type T2 };`,
}
});

0 comments on commit 2bcfed0

Please sign in to comment.