Skip to content
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

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

Merged
merged 3 commits into from Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 0 additions & 31 deletions src/compiler/checker.ts
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
Expand Up @@ -6687,6 +6687,14 @@
"category": "Message",
"code": 90058
},
"Export '{0}' from module '{1}'": {
"category": "Message",
"code": 90059
},
"Export all missing members": {
a-tarasyuk marked this conversation as resolved.
Show resolved Hide resolved
"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
161 changes: 161 additions & 0 deletions src/services/codefixes/fixImportNonExportedMember.ts
@@ -0,0 +1,161 @@
/* @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, info));
return [createCodeFixAction(fixId, changes, [Diagnostics.Export_0_from_module_1, info.exportName.node.text, info.moduleSpecifier], fixId, Diagnostics.Export_all_missing_members)];
},
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, moduleSourceFile, moduleExports.typeOnlyExports, exportDeclaration);
doChanges(changes, moduleSourceFile, moduleExports.exports, tryGetExportDeclaration(moduleSourceFile, /*isTypeOnly*/ false));
}
else {
doChanges(changes, 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, { exportName, node, moduleSourceFile }: Info) {
const exportDeclaration = tryGetExportDeclaration(moduleSourceFile, exportName.isTypeOnly);
if (exportDeclaration) {
updateExport(changes, moduleSourceFile, exportDeclaration, [exportName]);
}
else if (canHaveExportModifier(node)) {
changes.insertExportModifier(moduleSourceFile, node);
}
else {
createExport(changes, moduleSourceFile, [exportName]);
}
}

function doChanges(changes: textChanges.ChangeTracker, sourceFile: SourceFile, moduleExports: ExportName[], node: ExportDeclaration | undefined) {
if (node) {
updateExport(changes, sourceFile, node, moduleExports);
}
else {
createExport(changes, 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, sourceFile: SourceFile, node: ExportDeclaration, names: ExportName[]) {
const namedExports = node.exportClause && isNamedExports(node.exportClause) ? node.exportClause.elements : factory.createNodeArray([]);
const exportNames = map(names, n => ({ ...n, isTypeOnly: node.isTypeOnly ? false : n.isTypeOnly }));
changes.replaceNode(sourceFile, node,
factory.updateExportDeclaration(node, node.modifiers, node.isTypeOnly,
factory.createNamedExports(
factory.createNodeArray([...namedExports, ...createExportSpecifiers(exportNames)], /*hasTrailingComma*/ namedExports.hasTrailingComma)), node.moduleSpecifier, node.assertClause));
}

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

function createExportSpecifiers(names: ExportName[]) {
return factory.createNodeArray(map(names, n => factory.createExportSpecifier(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 @@ -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
@@ -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;`,
}
});
20 changes: 20 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember2.ts
@@ -0,0 +1,20 @@
/// <reference path="fourslash.ts" />

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

// @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":
`export declare function foo(): any;
export declare function bar(): any;`,
}
});
23 changes: 23 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember3.ts
@@ -0,0 +1,23 @@
/// <reference path="fourslash.ts" />

// @module: esnext
// @filename: /a.ts
////let foo = 1, bar = 1;
////export const baz = 1;

// @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":
`let foo = 1, bar = 1;
export const baz = 1;

export { bar };
`,
}
});
12 changes: 12 additions & 0 deletions tests/cases/fourslash/codeFixImportNonExportedMember4.ts
@@ -0,0 +1,12 @@
/// <reference path="fourslash.ts" />

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

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

goTo.file("/b.ts");
verify.not.codeFixAvailable("fixImportNonExportedMember");