Skip to content

feat(37440): Provide a quick-fix for non-exported types #51038

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 3 commits into from
Oct 13, 2022
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
31 changes: 0 additions & 31 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7237,16 +7237,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 @@ -42665,27 +42655,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -3421,5 +3421,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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 };`,
}
});
Loading