From f91c87006df378fe3b1ebe4658ce52a849d4c30a Mon Sep 17 00:00:00 2001 From: navya9singh Date: Fri, 19 Apr 2024 13:38:28 -0700 Subject: [PATCH 1/6] updating moveToFile to use latest importAdder changes --- src/services/codefixes/importFixes.ts | 9 + src/services/refactors/helpers.ts | 40 +++ src/services/refactors/moveToFile.ts | 340 +++++++----------- src/services/refactors/moveToNewFile.ts | 140 +------- src/services/utilities.ts | 5 - .../moveToFile_emptyTargetFile.js | 20 +- .../fourslash/moveToFile_blankExistingFile.ts | 2 - .../moveToFile_consistentQuoteStyle.ts | 1 - .../moveToFile_differentDirectories2.ts | 1 - .../moveToFile_expandSelectionRange4.ts | 2 +- .../fourslash/moveToFile_namedImports.ts | 13 +- .../fourslash/moveToFile_unresolvedImport.ts | 2 +- tests/cases/fourslash/moveToNewFile.ts | 4 +- .../fourslash/moveToNewFile_decorators.ts | 8 +- .../fourslash/moveToNewFile_decorators1.ts | 8 +- .../fourslash/moveToNewFile_decorators2.ts | 6 +- .../fourslash/moveToNewFile_decorators3.ts | 8 +- tests/cases/fourslash/moveToNewFile_global.ts | 8 +- .../fourslash/moveToNewFile_importEquals.ts | 3 +- tests/cases/fourslash/moveToNewFile_js.ts | 6 +- .../fourslash/moveToNewFile_moveJsxImport1.ts | 3 +- .../moveToNewFile_prologueDirectives1.ts | 5 +- .../moveToNewFile_prologueDirectives7.ts | 5 +- .../fourslash/moveToNewFile_requireImport1.ts | 2 +- .../fourslash/moveToNewFile_requireImport2.ts | 2 +- .../fourslash/moveToNewFile_requireImport3.ts | 33 -- .../fourslash/moveToNewFile_typeImport2.ts | 2 +- .../fourslash/moveToNewFile_updateUses.ts | 2 - 28 files changed, 251 insertions(+), 429 deletions(-) delete mode 100644 tests/cases/fourslash/moveToNewFile_requireImport3.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9dc8ebefe7db9..efdf2e457d7e9 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -402,6 +402,15 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly)); break; case ImportKind.CommonJS: + if (compilerOptions.verbatimModuleSyntax) { + const prevValue = (entry.namedImports ||= new Map()).get(symbolName); + entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly)); + } + else { + Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName"); + entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly }; + } + break; case ImportKind.Namespace: Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName"); entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly }; diff --git a/src/services/refactors/helpers.ts b/src/services/refactors/helpers.ts index ce50f672d23b8..47c774896d631 100644 --- a/src/services/refactors/helpers.ts +++ b/src/services/refactors/helpers.ts @@ -1,3 +1,16 @@ +import { + codefix, + Debug, + findAncestor, + isAnyImportOrRequireStatement, + Program, + skipAlias, + SourceFile, + Symbol, + TypeChecker, +} from "../_namespaces/ts"; +import { ImportOrRequireAliasDeclaration } from "../_namespaces/ts.codefix"; +import { addImportsForMovedSymbols } from "./moveToFile"; /** * Returned by refactor functions when some error message needs to be surfaced to users. * @@ -26,3 +39,30 @@ export function refactorKindBeginsWith(known: string, requested: string | undefi if (!requested) return true; return known.substr(0, requested.length) === requested; } + +/** @internal */ +export function addTargetFileImports( + oldFile: SourceFile, + importsToCopy: Map, + targetFileImportsFromOldFile: Map, + checker: TypeChecker, + program: Program, + importAdder: codefix.ImportAdder, +) { + /** + * Recomputing the imports is preferred with importAdder because it manages multiple import additions for a file and writes then to a ChangeTracker, + * but sometimes it fails because of unresolved imports from files, or when a source file is not available for the target file (in this case when creating a new file). + * So in that case, fall back to copying the import verbatim. + */ + importsToCopy.forEach(([isValidTypeOnlyUseSite, declaration], symbol) => { + const targetSymbol = skipAlias(symbol, checker); + if (checker.isUnknownSymbol(targetSymbol)) { + importAdder.addVerbatimImport(Debug.checkDefined(declaration ?? findAncestor(symbol.declarations?.[0], isAnyImportOrRequireStatement))); + } + else { + importAdder.addImportFromExportedSymbol(targetSymbol, isValidTypeOnlyUseSite, declaration); + } + }); + + addImportsForMovedSymbols(targetFileImportsFromOldFile, oldFile.fileName, importAdder, program); +} diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index b2a04354e1ad3..64a4b6ca951d2 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -1,7 +1,5 @@ -import { getModuleSpecifier } from "../../compiler/moduleSpecifiers"; +import { getModuleSpecifier } from "../../compiler/_namespaces/ts.moduleSpecifiers"; import { - AnyImportOrRequireStatement, - append, ApplicableRefactorInfo, arrayFrom, AssignmentDeclarationKind, @@ -19,6 +17,7 @@ import { Comparison, concatenate, contains, + createFutureSourceFile, createModuleSpecifierResolutionHost, createTextRangeFromSpan, Debug, @@ -29,6 +28,7 @@ import { EnumDeclaration, escapeLeadingUnderscores, ExportDeclaration, + ExportKind, Expression, ExpressionStatement, extensionFromPath, @@ -45,13 +45,16 @@ import { flatMap, forEachKey, FunctionDeclaration, + FutureSourceFile, getAssignmentDeclarationKind, GetCanonicalFileName, getDecorators, getDirectoryPath, + getEmitScriptTarget, getLineAndCharacterOfPosition, getLocaleSpecificMessage, getModifiers, + getNameForExportedSymbol, getNormalizedAbsolutePath, getPropertySymbolFromBindingElement, getQuotePreference, @@ -71,9 +74,7 @@ import { ImportDeclaration, ImportEqualsDeclaration, importFromModuleSpecifier, - insertImports, InterfaceDeclaration, - InternalSymbolName, isArrayLiteralExpression, isBinaryExpression, isBindingElement, @@ -83,12 +84,16 @@ import { isExportSpecifier, isExpressionStatement, isExternalModuleReference, + isFullSourceFile, isFunctionLikeDeclaration, isIdentifier, + isImportClause, isImportDeclaration, isImportEqualsDeclaration, + isImportSpecifier, isNamedExports, isNamedImports, + isNamespaceImport, isObjectBindingPattern, isObjectLiteralExpression, isOmittedExpression, @@ -108,12 +113,12 @@ import { LanguageServiceHost, last, length, - makeImportIfNecessary, makeStringLiteral, mapDefined, ModifierFlags, ModifierLike, ModuleDeclaration, + ModuleKind, NamedImportBindings, Node, NodeFlags, @@ -127,7 +132,6 @@ import { RefactorContext, RefactorEditInfo, RequireOrImportCall, - RequireVariableStatement, resolvePath, ScriptTarget, skipAlias, @@ -151,6 +155,11 @@ import { VariableDeclarationList, VariableStatement, } from "../_namespaces/ts"; +import { + ImportAdder, + ImportOrRequireAliasDeclaration, +} from "../_namespaces/ts.codefix"; +import { addTargetFileImports } from "../_namespaces/ts.refactor"; import { registerRefactor } from "../refactorProvider"; const refactorNameForMoveToFile = "Move to file"; @@ -213,169 +222,58 @@ function error(notApplicableReason: string) { function doChange(context: RefactorContext, oldFile: SourceFile, targetFile: string, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void { const checker = program.getTypeChecker(); - // For a new file - if (!host.fileExists(targetFile)) { - changes.createNewFile(oldFile, targetFile, getNewStatementsAndRemoveFromOldFile(oldFile, targetFile, getUsageInfo(oldFile, toMove.all, checker), changes, toMove, program, host, preferences)); + const isForNewFile = !host.fileExists(targetFile); + const targetSourceFile = isForNewFile + ? createFutureSourceFile(targetFile, oldFile.externalModuleIndicator ? ModuleKind.ESNext : oldFile.commonJsModuleIndicator ? ModuleKind.CommonJS : undefined, program, host) + : Debug.checkDefined(program.getSourceFile(targetFile)); + const importAdderForOldFile = codefix.createImportAdder(oldFile, context.program, context.preferences, context.host); + const importAdderForNewFile = codefix.createImportAdder(targetSourceFile, context.program, context.preferences, context.host); + getNewStatementsAndRemoveFromOldFile(oldFile, targetSourceFile, getUsageInfo(oldFile, toMove.all, checker, isForNewFile ? undefined : getExistingLocals(targetSourceFile as SourceFile, toMove.all, checker)), changes, toMove, program, host, preferences, importAdderForNewFile, importAdderForOldFile); + if (isForNewFile) { addNewFileToTsconfig(program, changes, oldFile.fileName, targetFile, hostGetCanonicalFileName(host)); } - else { - const targetSourceFile = Debug.checkDefined(program.getSourceFile(targetFile)); - const importAdder = codefix.createImportAdder(targetSourceFile, context.program, context.preferences, context.host); - getNewStatementsAndRemoveFromOldFile(oldFile, targetSourceFile, getUsageInfo(oldFile, toMove.all, checker, getExistingLocals(targetSourceFile, toMove.all, checker)), changes, toMove, program, host, preferences, importAdder); - } } -function getNewStatementsAndRemoveFromOldFile( +/** @internal */ +export function getNewStatementsAndRemoveFromOldFile( oldFile: SourceFile, - targetFile: string | SourceFile, + targetFile: SourceFile | FutureSourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, host: LanguageServiceHost, preferences: UserPreferences, - importAdder?: codefix.ImportAdder, + importAdderForNewFile: codefix.ImportAdder, + importAdderForOldFile: codefix.ImportAdder, ) { const checker = program.getTypeChecker(); const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective); - if (oldFile.externalModuleIndicator === undefined && oldFile.commonJsModuleIndicator === undefined && usage.oldImportsNeededByTargetFile.size === 0 && usage.targetFileImportsFromOldFile.size === 0 && typeof targetFile === "string") { - deleteMovedStatements(oldFile, toMove.ranges, changes); - return [...prologueDirectives, ...toMove.all]; - } - - // If the targetFile is a string, it’s the file name for a new file, if it’s a SourceFile, it’s the existing target file. - const targetFileName = typeof targetFile === "string" ? targetFile : targetFile.fileName; - const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(targetFileName, program, host, !!oldFile.commonJsModuleIndicator); + const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(targetFile.fileName, program, host, !!oldFile.commonJsModuleIndicator); const quotePreference = getQuotePreference(oldFile, preferences); - const importsFromTargetFile = createOldFileImportsFromTargetFile(oldFile, usage.oldFileImportsFromTargetFile, targetFileName, program, host, useEsModuleSyntax, quotePreference); - if (importsFromTargetFile) { - insertImports(changes, oldFile, importsFromTargetFile, /*blankLineBetween*/ true, preferences); - } - - deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker); + addImportsForMovedSymbols(usage.oldFileImportsFromTargetFile, targetFile.fileName, importAdderForOldFile, program); // imports for moved symbols in the target file + deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker, importAdderForOldFile); + importAdderForOldFile.writeFixes(changes, quotePreference); deleteMovedStatements(oldFile, toMove.ranges, changes); - updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, targetFileName, quotePreference); - - const imports = getTargetFileImportsAndAddExportInOldFile(oldFile, targetFileName, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, changes, checker, program, host, useEsModuleSyntax, quotePreference, importAdder); - const body = addExports(oldFile, toMove.all, usage.oldFileImportsFromTargetFile, useEsModuleSyntax); - if (typeof targetFile !== "string") { - if (targetFile.statements.length > 0) { - moveStatementsToTargetFile(changes, program, body, targetFile, toMove); - } - else { - changes.insertNodesAtEndOfFile(targetFile, body, /*blankLineBetween*/ false); - } - if (imports.length > 0) { - insertImports(changes, targetFile, imports, /*blankLineBetween*/ true, preferences); - } + updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, targetFile.fileName, quotePreference); + addExportsInOldFIle(oldFile, usage.targetFileImportsFromOldFile, changes, useEsModuleSyntax); + addTargetFileImports(oldFile, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, checker, program, importAdderForNewFile); + if (!isFullSourceFile(targetFile) && prologueDirectives.length) { + // Ensure prologue directives come before imports + changes.insertStatementsInNewFile(targetFile.fileName, prologueDirectives, oldFile); } - if (importAdder) { - importAdder.writeFixes(changes, quotePreference); + importAdderForNewFile.writeFixes(changes, quotePreference); + const body = addExports(oldFile, toMove.all, arrayFrom(usage.oldFileImportsFromTargetFile.keys()), useEsModuleSyntax); + if (isFullSourceFile(targetFile) && targetFile.statements.length > 0) { + moveStatementsToTargetFile(changes, program, body, targetFile, toMove); } - if (imports.length && body.length) { - return [ - ...prologueDirectives, - ...imports, - SyntaxKind.NewLineTrivia as const, - ...body, - ]; - } - - return [ - ...prologueDirectives, - ...imports, - ...body, - ]; -} - -function getTargetFileImportsAndAddExportInOldFile( - oldFile: SourceFile, - targetFile: string, - importsToCopy: Map, - targetFileImportsFromOldFile: Set, - changes: textChanges.ChangeTracker, - checker: TypeChecker, - program: Program, - host: LanguageServiceHost, - useEsModuleSyntax: boolean, - quotePreference: QuotePreference, - importAdder?: codefix.ImportAdder, -): readonly AnyImportOrRequireStatement[] { - const copiedOldImports: AnyImportOrRequireStatement[] = []; - /** - * Recomputing the imports is preferred with importAdder because it manages multiple import additions for a file and writes then to a ChangeTracker, - * but sometimes it fails because of unresolved imports from files, or when a source file is not available for the target file (in this case when creating a new file). - * So in that case, fall back to copying the import verbatim. - */ - if (importAdder) { - importsToCopy.forEach((isValidTypeOnlyUseSite, symbol) => { - try { - importAdder.addImportFromExportedSymbol(skipAlias(symbol, checker), isValidTypeOnlyUseSite); - } - catch { - for (const oldStatement of oldFile.statements) { - forEachImportInStatement(oldStatement, i => { - append(copiedOldImports, filterImport(i, factory.createStringLiteral(moduleSpecifierFromImport(i).text), name => importsToCopy.has(checker.getSymbolAtLocation(name)!))); - }); - } - } - }); + else if (isFullSourceFile(targetFile)) { + changes.insertNodesAtEndOfFile(targetFile, body, /*blankLineBetween*/ false); } else { - const targetSourceFile = program.getSourceFile(targetFile); // Would be undefined for a new file - for (const oldStatement of oldFile.statements) { - forEachImportInStatement(oldStatement, i => { - // Recomputing module specifier - const moduleSpecifier = moduleSpecifierFromImport(i); - const compilerOptions = program.getCompilerOptions(); - const resolved = program.getResolvedModuleFromModuleSpecifier(moduleSpecifier); - const fileName = resolved?.resolvedModule?.resolvedFileName; - if (fileName && targetSourceFile) { - const newModuleSpecifier = getModuleSpecifier(compilerOptions, targetSourceFile, targetSourceFile.fileName, fileName, createModuleSpecifierResolutionHost(program, host)); - append(copiedOldImports, filterImport(i, makeStringLiteral(newModuleSpecifier, quotePreference), name => importsToCopy.has(checker.getSymbolAtLocation(name)!))); - } - else { - append(copiedOldImports, filterImport(i, factory.createStringLiteral(moduleSpecifierFromImport(i).text), name => importsToCopy.has(checker.getSymbolAtLocation(name)!))); - } - }); - } + changes.insertStatementsInNewFile(targetFile.fileName, importAdderForNewFile.hasFixes() ? [SyntaxKind.NewLineTrivia, ...body] : body, oldFile); } - - // Also, import things used from the old file, and insert 'export' modifiers as necessary in the old file. - const targetFileSourceFile = program.getSourceFile(targetFile); - let oldFileDefault: Identifier | undefined; - const oldFileNamedImports: string[] = []; - const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`. - targetFileImportsFromOldFile.forEach(symbol => { - if (!symbol.declarations) { - return; - } - for (const decl of symbol.declarations) { - if (!isTopLevelDeclaration(decl)) continue; - const name = nameOfTopLevelDeclaration(decl); - if (!name) continue; - - const top = getTopLevelDeclarationStatement(decl); - if (markSeenTop(top)) { - addExportToChanges(oldFile, top, name, changes, useEsModuleSyntax); - } - if (importAdder && checker.isUnknownSymbol(symbol)) { - importAdder.addImportFromExportedSymbol(skipAlias(symbol, checker)); - } - else { - if (hasSyntacticModifier(decl, ModifierFlags.Default)) { - oldFileDefault = name; - } - else { - oldFileNamedImports.push(name.text); - } - } - } - }); - return targetFileSourceFile - ? append(copiedOldImports, makeImportOrRequire(targetFileSourceFile, oldFileDefault, oldFileNamedImports, oldFile.fileName, program, host, useEsModuleSyntax, quotePreference)) - : append(copiedOldImports, makeImportOrRequire(oldFile, oldFileDefault, oldFileNamedImports, oldFile.fileName, program, host, useEsModuleSyntax, quotePreference)); } /** @internal */ @@ -401,13 +299,58 @@ export function deleteMovedStatements(sourceFile: SourceFile, moved: readonly St } /** @internal */ -export function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Statement[], changes: textChanges.ChangeTracker, toDelete: Set, checker: TypeChecker) { +export function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Statement[], changes: textChanges.ChangeTracker, toDelete: Set, checker: TypeChecker, importAdder: ImportAdder) { for (const statement of oldFile.statements) { if (contains(toMove, statement)) continue; - forEachImportInStatement(statement, i => deleteUnusedImports(oldFile, i, changes, name => toDelete.has(checker.getSymbolAtLocation(name)!))); + forEachImportInStatement(statement, i => { + if (i.kind === SyntaxKind.ImportDeclaration) { + if (i.importClause?.name && toDelete.has(Debug.checkDefined(i.importClause.symbol))) { + importAdder.removeExistingImport(i.importClause); + } + if (i.importClause?.namedBindings?.kind === SyntaxKind.NamespaceImport && toDelete.has(Debug.checkDefined(i.importClause.namedBindings.symbol))) { + importAdder.removeExistingImport(i.importClause.namedBindings); + } + if (i.importClause?.namedBindings?.kind === SyntaxKind.NamedImports) { + for (const element of i.importClause.namedBindings.elements) { + if (toDelete.has(Debug.checkDefined(element.symbol))) { + importAdder.removeExistingImport(element); + } + } + } + } + else if (i.kind === SyntaxKind.VariableDeclaration && i.name.kind === SyntaxKind.Identifier && toDelete.has(Debug.checkDefined(i.symbol))) { + importAdder.removeExistingImport(i); + } + else if (i.name.kind === SyntaxKind.ObjectBindingPattern) { + for (const element of i.name.elements) { + if (toDelete.has(Debug.checkDefined(element.symbol))) { + importAdder.removeExistingImport(element); + } + } + } + }); } } +function addExportsInOldFIle(oldFile: SourceFile, targetFileImportsFromOldFile: Map, changes: textChanges.ChangeTracker, useEsModuleSyntax: boolean) { + const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`. + targetFileImportsFromOldFile.forEach((_, symbol) => { + if (!symbol.declarations) { + return; + } + for (const decl of symbol.declarations) { + if (!isTopLevelDeclaration(decl)) continue; + const name = nameOfTopLevelDeclaration(decl); + if (!name) continue; + + const top = getTopLevelDeclarationStatement(decl); + if (markSeenTop(top)) { + addExportToChanges(oldFile, top, name, changes, useEsModuleSyntax); + } + } + }); +} + /** @internal */ export function updateImportsInOtherFiles( changes: textChanges.ChangeTracker, @@ -557,52 +500,16 @@ export type SupportedImportStatement = | VariableStatement; /** @internal */ -export function createOldFileImportsFromTargetFile( - sourceFile: SourceFile, - targetFileNeedExport: Set, - targetFileNameWithExtension: string, - program: Program, - host: LanguageServiceHost, - useEs6Imports: boolean, - quotePreference: QuotePreference, -): AnyImportOrRequireStatement | undefined { - let defaultImport: Identifier | undefined; - const imports: string[] = []; - targetFileNeedExport.forEach(symbol => { - if (symbol.escapedName === InternalSymbolName.Default) { - defaultImport = factory.createIdentifier(symbolNameNoDefault(symbol)!); - } - else { - imports.push(symbol.name); - } - }); - return makeImportOrRequire(sourceFile, defaultImport, imports, targetFileNameWithExtension, program, host, useEs6Imports, quotePreference); -} - -/** @internal */ -export function makeImportOrRequire( - sourceFile: SourceFile, - defaultImport: Identifier | undefined, - imports: readonly string[], - targetFileNameWithExtension: string, +export function addImportsForMovedSymbols( + symbols: Map, + targetFileName: string, + importAdder: codefix.ImportAdder, program: Program, - host: LanguageServiceHost, - useEs6Imports: boolean, - quotePreference: QuotePreference, -): AnyImportOrRequireStatement | undefined { - const pathToTargetFile = resolvePath(getDirectoryPath(getNormalizedAbsolutePath(sourceFile.fileName, program.getCurrentDirectory())), targetFileNameWithExtension); - const pathToTargetFileWithCorrectExtension = getModuleSpecifier(program.getCompilerOptions(), sourceFile, sourceFile.fileName, pathToTargetFile, createModuleSpecifierResolutionHost(program, host)); - - if (useEs6Imports) { - const specifiers = imports.map(i => factory.createImportSpecifier(/*isTypeOnly*/ false, /*propertyName*/ undefined, factory.createIdentifier(i))); - return makeImportIfNecessary(defaultImport, specifiers, pathToTargetFileWithCorrectExtension, quotePreference); - } - else { - Debug.assert(!defaultImport, "No default import should exist"); // If there's a default export, it should have been an es6 module. - const bindingElements = imports.map(i => factory.createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ undefined, i)); - return bindingElements.length - ? makeVariableStatement(factory.createObjectBindingPattern(bindingElements), /*type*/ undefined, createRequireCall(makeStringLiteral(pathToTargetFileWithCorrectExtension, quotePreference))) as RequireVariableStatement - : undefined; +) { + for (const [symbol, isValidTypeOnlyUseSite] of symbols) { + const symbolName = getNameForExportedSymbol(symbol, getEmitScriptTarget(program.getCompilerOptions())); + const exportKind = symbol.name === "default" && symbol.parent ? ExportKind.Default : ExportKind.Named; + importAdder.addImportForNonExistentExport(symbolName, targetFileName, exportKind, symbol.flags, isValidTypeOnlyUseSite); } } @@ -610,13 +517,12 @@ function makeVariableStatement(name: BindingName, type: TypeNode | undefined, in return factory.createVariableStatement(/*modifiers*/ undefined, factory.createVariableDeclarationList([factory.createVariableDeclaration(name, /*exclamationToken*/ undefined, type, initializer)], flags)); } -/** @internal */ -export function addExports(sourceFile: SourceFile, toMove: readonly Statement[], needExport: Set, useEs6Exports: boolean): readonly Statement[] { +function addExports(sourceFile: SourceFile, toMove: readonly Statement[], needExport: Symbol[], useEs6Exports: boolean): readonly Statement[] { return flatMap(toMove, statement => { if ( isTopLevelDeclarationStatement(statement) && !isExported(sourceFile, statement, useEs6Exports) && - forEachTopLevelDeclaration(statement, d => needExport.has(Debug.checkDefined(tryCast(d, canHaveSymbol)?.symbol))) + forEachTopLevelDeclaration(statement, d => needExport.includes(Debug.checkDefined(tryCast(d, canHaveSymbol)?.symbol))) ) { const exports = addExport(getSynthesizedDeepClone(statement), useEs6Exports); if (exports) return exports; @@ -690,9 +596,6 @@ function deleteUnusedImportsInVariableDeclaration(sourceFile: SourceFile, varDec if (varDecl.initializer && isRequireCall(varDecl.initializer, /*requireStringLiteralLikeArgument*/ true)) { changes.delete(sourceFile, isVariableDeclarationList(varDecl.parent) && length(varDecl.parent.declarations) === 1 ? varDecl.parent.parent : varDecl); } - else { - changes.delete(sourceFile, name); - } } break; case SyntaxKind.ArrayBindingPattern: @@ -879,16 +782,16 @@ export interface StatementRange { /** @internal */ export interface UsageInfo { - // Symbols whose declarations are moved from the old file to the new file. + /** Symbols whose declarations are moved from the old file to the new file. */ readonly movedSymbols: Set; - // Symbols declared in the old file that must be imported by the new file. (May not already be exported.) - readonly targetFileImportsFromOldFile: Set; - // Subset of movedSymbols that are still used elsewhere in the old file and must be imported back. - readonly oldFileImportsFromTargetFile: Set; + /** Symbols declared in the old file that must be imported by the new file. (May not already be exported.) */ + readonly targetFileImportsFromOldFile: Map; + /** Subset of movedSymbols that are still used elsewhere in the old file and must be imported back. */ + readonly oldFileImportsFromTargetFile: Map; - readonly oldImportsNeededByTargetFile: Map; - // Subset of oldImportsNeededByTargetFile that are will no longer be used in the old file. + readonly oldImportsNeededByTargetFile: Map; + /** Subset of oldImportsNeededByTargetFile that are will no longer be used in the old file. */ readonly unusedImportsFromOldFile: Set; } @@ -1022,13 +925,13 @@ function isPureImport(node: Node): boolean { /** @internal */ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], checker: TypeChecker, existingTargetLocals: ReadonlySet = new Set()): UsageInfo { const movedSymbols = new Set(); - const oldImportsNeededByTargetFile = new Map(); - const targetFileImportsFromOldFile = new Set(); + const oldImportsNeededByTargetFile = new Map(); + const targetFileImportsFromOldFile = new Map(); const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx(toMove)); if (jsxNamespaceSymbol) { // Might not exist (e.g. in non-compiling code) - oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, false); + oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, [false, tryCast(jsxNamespaceSymbol.declarations?.[0], (d): d is ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d))]); } for (const statement of toMove) { @@ -1050,10 +953,13 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], for (const decl of symbol.declarations) { if (isInImport(decl)) { const prevIsTypeOnly = oldImportsNeededByTargetFile.get(symbol); - oldImportsNeededByTargetFile.set(symbol, prevIsTypeOnly === undefined ? isValidTypeOnlyUseSite : prevIsTypeOnly && isValidTypeOnlyUseSite); + oldImportsNeededByTargetFile.set(symbol, [ + prevIsTypeOnly === undefined ? isValidTypeOnlyUseSite : prevIsTypeOnly && isValidTypeOnlyUseSite, + tryCast(decl, (d): d is ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d)), + ]); } else if (isTopLevelDeclaration(decl) && sourceFileOfTopLevelDeclaration(decl) === oldFile && !movedSymbols.has(symbol)) { - targetFileImportsFromOldFile.add(symbol); + targetFileImportsFromOldFile.set(symbol, isValidTypeOnlyUseSite); } } }); @@ -1063,7 +969,7 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], unusedImportsFromOldFile.add(unusedImport); } - const oldFileImportsFromTargetFile = new Set(); + const oldFileImportsFromTargetFile = new Map(); for (const statement of oldFile.statements) { if (contains(toMove, statement)) continue; @@ -1072,8 +978,8 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], unusedImportsFromOldFile.delete(jsxNamespaceSymbol); } - forEachReference(statement, checker, symbol => { - if (movedSymbols.has(symbol)) oldFileImportsFromTargetFile.add(symbol); + forEachReference(statement, checker, (symbol, isValidTypeOnlyUseSite) => { + if (movedSymbols.has(symbol)) oldFileImportsFromTargetFile.set(symbol, isValidTypeOnlyUseSite); unusedImportsFromOldFile.delete(symbol); }); } @@ -1107,7 +1013,7 @@ function makeUniqueFilename(proposedFilename: string, extension: string, inDirec } } -function inferNewFileName(importsFromNewFile: Set, movedSymbols: Set): string { +function inferNewFileName(importsFromNewFile: Map, movedSymbols: Set): string { return forEachKey(importsFromNewFile, symbolNameNoDefault) || forEachKey(movedSymbols, symbolNameNoDefault) || "newFile"; } diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index 99b6612fecf85..2665239e10483 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -1,56 +1,31 @@ import { - append, ApplicableRefactorInfo, + codefix, + createFutureSourceFile, Debug, Diagnostics, emptyArray, - fileShouldUseJavaScriptRequire, - getBaseFileName, getLineAndCharacterOfPosition, getLocaleSpecificMessage, - getQuotePreference, - hasSyntacticModifier, hostGetCanonicalFileName, - Identifier, - insertImports, - isPrologueDirective, LanguageServiceHost, last, - ModifierFlags, - nodeSeenTracker, + ModuleKind, Program, - QuotePreference, + RefactorContext, RefactorEditInfo, SourceFile, - Symbol, - SyntaxKind, - takeWhile, textChanges, - TypeChecker, UserPreferences, } from "../_namespaces/ts"; import { - addExports, - addExportToChanges, addNewFileToTsconfig, createNewFileName, - createOldFileImportsFromTargetFile, - deleteMovedStatements, - deleteUnusedOldImports, - filterImport, - forEachImportInStatement, + getNewStatementsAndRemoveFromOldFile, getStatementsToMove, - getTopLevelDeclarationStatement, getUsageInfo, - isTopLevelDeclaration, - makeImportOrRequire, - moduleSpecifierFromImport, - nameOfTopLevelDeclaration, registerRefactor, - SupportedImportStatement, ToMove, - updateImportsInOtherFiles, - UsageInfo, } from "../_namespaces/ts.refactor"; const refactorName = "Move to a new file"; @@ -81,112 +56,19 @@ registerRefactor(refactorName, { getEditsForAction: function getRefactorEditsToMoveToNewFile(context, actionName): RefactorEditInfo { Debug.assert(actionName === refactorName, "Wrong refactor invoked"); const statements = Debug.checkDefined(getStatementsToMove(context)); - const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences)); + const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context, context.preferences)); return { edits, renameFilename: undefined, renameLocation: undefined }; }, }); -function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void { +function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, context: RefactorContext, preferences: UserPreferences): void { const checker = program.getTypeChecker(); const usage = getUsageInfo(oldFile, toMove.all, checker); const newFilename = createNewFileName(oldFile, program, host, toMove); + const newSourceFile = createFutureSourceFile(newFilename, oldFile.externalModuleIndicator ? ModuleKind.ESNext : oldFile.commonJsModuleIndicator ? ModuleKind.CommonJS : undefined, program, host); - // If previous file was global, this is easy. - changes.createNewFile(oldFile, newFilename, getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFilename, preferences)); - + const importAdderForOldFile = codefix.createImportAdder(oldFile, context.program, context.preferences, context.host); + const importAdderForNewFile = codefix.createImportAdder(newSourceFile, context.program, context.preferences, context.host); + getNewStatementsAndRemoveFromOldFile(oldFile, newSourceFile, usage, changes, toMove, program, host, preferences, importAdderForNewFile, importAdderForOldFile); addNewFileToTsconfig(program, changes, oldFile.fileName, newFilename, hostGetCanonicalFileName(host)); } - -function getNewStatementsAndRemoveFromOldFile( - oldFile: SourceFile, - usage: UsageInfo, - changes: textChanges.ChangeTracker, - toMove: ToMove, - program: Program, - host: LanguageServiceHost, - newFilename: string, - preferences: UserPreferences, -) { - const checker = program.getTypeChecker(); - const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective); - if (oldFile.externalModuleIndicator === undefined && oldFile.commonJsModuleIndicator === undefined && usage.oldImportsNeededByTargetFile.size === 0) { - deleteMovedStatements(oldFile, toMove.ranges, changes); - return [...prologueDirectives, ...toMove.all]; - } - - const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(newFilename, program, host, !!oldFile.commonJsModuleIndicator); - const quotePreference = getQuotePreference(oldFile, preferences); - const importsFromNewFile = createOldFileImportsFromTargetFile(oldFile, usage.oldFileImportsFromTargetFile, newFilename, program, host, useEsModuleSyntax, quotePreference); - if (importsFromNewFile) { - insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true, preferences); - } - - deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker); - deleteMovedStatements(oldFile, toMove.ranges, changes); - updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, newFilename, quotePreference); - - const imports = getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, changes, checker, program, host, useEsModuleSyntax, quotePreference); - const body = addExports(oldFile, toMove.all, usage.oldFileImportsFromTargetFile, useEsModuleSyntax); - if (imports.length && body.length) { - return [ - ...prologueDirectives, - ...imports, - SyntaxKind.NewLineTrivia as const, - ...body, - ]; - } - - return [ - ...prologueDirectives, - ...imports, - ...body, - ]; -} - -function getNewFileImportsAndAddExportInOldFile( - oldFile: SourceFile, - importsToCopy: Map, - newFileImportsFromOldFile: Set, - changes: textChanges.ChangeTracker, - checker: TypeChecker, - program: Program, - host: LanguageServiceHost, - useEsModuleSyntax: boolean, - quotePreference: QuotePreference, -): readonly SupportedImportStatement[] { - const copiedOldImports: SupportedImportStatement[] = []; - for (const oldStatement of oldFile.statements) { - forEachImportInStatement(oldStatement, i => { - append(copiedOldImports, filterImport(i, moduleSpecifierFromImport(i), name => importsToCopy.has(checker.getSymbolAtLocation(name)!))); - }); - } - - // Also, import things used from the old file, and insert 'export' modifiers as necessary in the old file. - let oldFileDefault: Identifier | undefined; - const oldFileNamedImports: string[] = []; - const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`. - newFileImportsFromOldFile.forEach(symbol => { - if (!symbol.declarations) { - return; - } - for (const decl of symbol.declarations) { - if (!isTopLevelDeclaration(decl)) continue; - const name = nameOfTopLevelDeclaration(decl); - if (!name) continue; - - const top = getTopLevelDeclarationStatement(decl); - if (markSeenTop(top)) { - addExportToChanges(oldFile, top, name, changes, useEsModuleSyntax); - } - if (hasSyntacticModifier(decl, ModifierFlags.Default)) { - oldFileDefault = name; - } - else { - oldFileNamedImports.push(name.text); - } - } - }); - - append(copiedOldImports, makeImportOrRequire(oldFile, oldFileDefault, oldFileNamedImports, getBaseFileName(oldFile.fileName), program, host, useEsModuleSyntax, quotePreference)); - return copiedOldImports; -} diff --git a/src/services/utilities.ts b/src/services/utilities.ts index d8d05869a0bed..179ca20e4c46c 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2504,11 +2504,6 @@ export function moduleResolutionUsesNodeModules(moduleResolution: ModuleResoluti || moduleResolution === ModuleResolutionKind.Bundler; } -/** @internal */ -export function makeImportIfNecessary(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string, quotePreference: QuotePreference): ImportDeclaration | undefined { - return defaultImport || namedImports && namedImports.length ? makeImport(defaultImport, namedImports, moduleSpecifier, quotePreference) : undefined; -} - /** @internal */ export function makeImport(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string | Expression, quotePreference: QuotePreference, isTypeOnly?: boolean): ImportDeclaration { return factory.createImportDeclaration( diff --git a/tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js b/tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js index 5414c78b40e24..73f4d8a932124 100644 --- a/tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js +++ b/tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js @@ -242,13 +242,13 @@ Info seq [hh:mm:ss:mss] response: ] }, { - "name": "Move to a new file", - "description": "Move to a new file", + "name": "Move to file", + "description": "Move to file", "actions": [ { - "name": "Move to a new file", - "description": "Move to a new file", - "kind": "refactor.move.newFile", + "name": "Move to file", + "description": "Move to file", + "kind": "refactor.move.file", "range": { "start": { "line": 1, @@ -263,13 +263,13 @@ Info seq [hh:mm:ss:mss] response: ] }, { - "name": "Move to file", - "description": "Move to file", + "name": "Move to a new file", + "description": "Move to a new file", "actions": [ { - "name": "Move to file", - "description": "Move to file", - "kind": "refactor.move.file", + "name": "Move to a new file", + "description": "Move to a new file", + "kind": "refactor.move.newFile", "range": { "start": { "line": 1, diff --git a/tests/cases/fourslash/moveToFile_blankExistingFile.ts b/tests/cases/fourslash/moveToFile_blankExistingFile.ts index 370fa30b7edab..085ff33a1f391 100644 --- a/tests/cases/fourslash/moveToFile_blankExistingFile.ts +++ b/tests/cases/fourslash/moveToFile_blankExistingFile.ts @@ -20,8 +20,6 @@ verify.moveToFile({ "/bar.ts": `// import { p } from './a'; - - import { b } from './other'; diff --git a/tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts b/tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts index 969060b24f2c4..1dd41d7ceeb5f 100644 --- a/tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts +++ b/tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts @@ -21,7 +21,6 @@ verify.moveToFile({ "/bar.ts": `import { a } from './a'; - import { b } from './other'; export const tt = 2; diff --git a/tests/cases/fourslash/moveToFile_differentDirectories2.ts b/tests/cases/fourslash/moveToFile_differentDirectories2.ts index 43aea215d6117..173b64a4cff99 100644 --- a/tests/cases/fourslash/moveToFile_differentDirectories2.ts +++ b/tests/cases/fourslash/moveToFile_differentDirectories2.ts @@ -24,7 +24,6 @@ y;`, "/src/dir2/bar.ts": `import { a } from '../dir1/a'; - import { b } from '../dir1/other'; diff --git a/tests/cases/fourslash/moveToFile_expandSelectionRange4.ts b/tests/cases/fourslash/moveToFile_expandSelectionRange4.ts index 7a0861405a9e0..b3ccd1be8fe5d 100644 --- a/tests/cases/fourslash/moveToFile_expandSelectionRange4.ts +++ b/tests/cases/fourslash/moveToFile_expandSelectionRange4.ts @@ -28,7 +28,7 @@ export const x: T={ `, "/bar.ts": -`import { x } from "./a"; +`import { x } from './a'; import { b } from './other'; const t = b; const b = x.a; diff --git a/tests/cases/fourslash/moveToFile_namedImports.ts b/tests/cases/fourslash/moveToFile_namedImports.ts index 916831d7699fd..7d9adb9f1b9fd 100644 --- a/tests/cases/fourslash/moveToFile_namedImports.ts +++ b/tests/cases/fourslash/moveToFile_namedImports.ts @@ -12,10 +12,13 @@ //// ////export const p = 0; ////export const b = 1; -////[|const y = p + b;|] +////[|export const y = p + b; +////export const z = 1;|] // @Filename: /other.ts +////import { z, y } from './a'; ////export const t = 2; +////const u = z + t + y; verify.moveToFile({ @@ -34,8 +37,14 @@ export const b = 1; `import { p, b } from './a'; const q = 0; -const y = p + b; +export const y = p + b; +export const z = 1; `, + +"/other.ts": +`import { z, y } from './bar'; +export const t = 2; +const u = z + t + y;` }, interactiveRefactorArguments: { targetFile: "/bar.ts" }, diff --git a/tests/cases/fourslash/moveToFile_unresolvedImport.ts b/tests/cases/fourslash/moveToFile_unresolvedImport.ts index 8cdd873361f22..a595c425f696c 100644 --- a/tests/cases/fourslash/moveToFile_unresolvedImport.ts +++ b/tests/cases/fourslash/moveToFile_unresolvedImport.ts @@ -13,7 +13,7 @@ verify.moveToFile({ ``, "/bar.ts": -`import { a } from "doesnt-exist"; +`import { a } from 'doesnt-exist'; const a = 1; a(); diff --git a/tests/cases/fourslash/moveToNewFile.ts b/tests/cases/fourslash/moveToNewFile.ts index ef81e13f9595a..5e16b05228de6 100644 --- a/tests/cases/fourslash/moveToNewFile.ts +++ b/tests/cases/fourslash/moveToNewFile.ts @@ -21,8 +21,8 @@ export const p = 0; a; y;`, "/y.ts": -`import { b } from './other'; -import { p } from './a'; +`import { p } from './a'; +import { b } from './other'; export const y: Date = p + b; `, diff --git a/tests/cases/fourslash/moveToNewFile_decorators.ts b/tests/cases/fourslash/moveToNewFile_decorators.ts index a44ff9cb5cd73..9273346386f9f 100644 --- a/tests/cases/fourslash/moveToNewFile_decorators.ts +++ b/tests/cases/fourslash/moveToNewFile_decorators.ts @@ -18,11 +18,13 @@ verify.noErrors(); verify.moveToNewFile({ newFileContents: { "/a.ts": -`const decorator1: any = () => {}; -const decorator2: any = () => {}; +`export const decorator1: any = () => {}; +export const decorator2: any = () => {}; `, "/Foo.ts": -`class Foo { +`import { decorator1, decorator2 } from "./a"; + +class Foo { constructor(@decorator1 private readonly x: number, @decorator1 @decorator2 private readonly y: number) { } diff --git a/tests/cases/fourslash/moveToNewFile_decorators1.ts b/tests/cases/fourslash/moveToNewFile_decorators1.ts index 3f2ca9a30f495..07dabd3899d15 100644 --- a/tests/cases/fourslash/moveToNewFile_decorators1.ts +++ b/tests/cases/fourslash/moveToNewFile_decorators1.ts @@ -15,11 +15,13 @@ verify.noErrors(); verify.moveToNewFile({ newFileContents: { "/a.ts": -`const decorator1: any = () => {}; -const decorator2: any = () => {}; +`export const decorator1: any = () => {}; +export const decorator2: any = () => {}; `, "/Foo.ts": -`class Foo { +`import { decorator1, decorator2 } from "./a"; + +class Foo { @decorator1 method1() { } @decorator1 @decorator2 method2() { } } diff --git a/tests/cases/fourslash/moveToNewFile_decorators2.ts b/tests/cases/fourslash/moveToNewFile_decorators2.ts index 41daa532eeac0..bda98148a2b9a 100644 --- a/tests/cases/fourslash/moveToNewFile_decorators2.ts +++ b/tests/cases/fourslash/moveToNewFile_decorators2.ts @@ -12,10 +12,12 @@ verify.noErrors(); verify.moveToNewFile({ newFileContents: { "/a.ts": -`const decorator1: any = () => {}; +`export const decorator1: any = () => {}; `, "/Foo.ts": -`@decorator1 class Foo { +`import { decorator1 } from "./a"; + +@decorator1 class Foo { } ` } diff --git a/tests/cases/fourslash/moveToNewFile_decorators3.ts b/tests/cases/fourslash/moveToNewFile_decorators3.ts index dc75ce489699f..d256fda521a7c 100644 --- a/tests/cases/fourslash/moveToNewFile_decorators3.ts +++ b/tests/cases/fourslash/moveToNewFile_decorators3.ts @@ -13,11 +13,13 @@ verify.noErrors(); verify.moveToNewFile({ newFileContents: { "/a.ts": -`const decorator1: any = () => {}; -const decorator2: any = () => {}; +`export const decorator1: any = () => {}; +export const decorator2: any = () => {}; `, "/Foo.ts": -`@decorator1 @decorator2 class Foo { +`import { decorator1, decorator2 } from "./a"; + +@decorator1 @decorator2 class Foo { } ` } diff --git a/tests/cases/fourslash/moveToNewFile_global.ts b/tests/cases/fourslash/moveToNewFile_global.ts index 940b4df8a528f..a98a43a4376a9 100644 --- a/tests/cases/fourslash/moveToNewFile_global.ts +++ b/tests/cases/fourslash/moveToNewFile_global.ts @@ -7,11 +7,15 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": -`const x = y; +`import { y } from "./y"; + +export const x = y; `, "/y.ts": -`const y = x; +`import { x } from "./a"; + +export const y = x; `, }, }); diff --git a/tests/cases/fourslash/moveToNewFile_importEquals.ts b/tests/cases/fourslash/moveToNewFile_importEquals.ts index 87199e15437a6..589c4b32249fc 100644 --- a/tests/cases/fourslash/moveToNewFile_importEquals.ts +++ b/tests/cases/fourslash/moveToNewFile_importEquals.ts @@ -9,7 +9,8 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": -`import j = require("./j"); +`import i = require("./i"); +import j = require("./j"); j;`, "/y.ts": `import i = require("./i"); diff --git a/tests/cases/fourslash/moveToNewFile_js.ts b/tests/cases/fourslash/moveToNewFile_js.ts index fd8db38e3d32f..72faec5af1e81 100644 --- a/tests/cases/fourslash/moveToNewFile_js.ts +++ b/tests/cases/fourslash/moveToNewFile_js.ts @@ -17,15 +17,15 @@ verify.moveToNewFile({ newFileContents: { "/a.js": -`const { a, } = require("./other"); +`const { a } = require("./other"); const { y, z } = require("./y"); const p = 0; exports.p = p; a; y; z;`, "/y.js": -`const { b } = require("./other"); -const { p } = require("./a"); +`const { p } = require("./a"); +const { b } = require("./other"); const y = p + b; exports.y = y; diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts index f1c86b53a51cc..fa0a5042be259 100644 --- a/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts +++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts @@ -12,7 +12,8 @@ verify.moveToNewFile({ newFileContents: { "/tests/cases/fourslash/file.tsx": -`1;`, +`import React = require('react'); +1;`, "/tests/cases/fourslash/newFile.tsx": `import React = require('react'); diff --git a/tests/cases/fourslash/moveToNewFile_prologueDirectives1.ts b/tests/cases/fourslash/moveToNewFile_prologueDirectives1.ts index 5153f52c6f30d..563eff8e65308 100644 --- a/tests/cases/fourslash/moveToNewFile_prologueDirectives1.ts +++ b/tests/cases/fourslash/moveToNewFile_prologueDirectives1.ts @@ -11,11 +11,14 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": `"use strict"; + +import { b } from "./b"; + b();`, "/b.ts": `"use strict"; -function b() { +export function b() { return this; } `, diff --git a/tests/cases/fourslash/moveToNewFile_prologueDirectives7.ts b/tests/cases/fourslash/moveToNewFile_prologueDirectives7.ts index dbf5b10b912de..37be07dd4acac 100644 --- a/tests/cases/fourslash/moveToNewFile_prologueDirectives7.ts +++ b/tests/cases/fourslash/moveToNewFile_prologueDirectives7.ts @@ -12,12 +12,15 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": `"use strict"; + +import { b } from "./b"; + "prologue directive like statement"; b();`, "/b.ts": `"use strict"; -function b() { +export function b() { return this; } `, diff --git a/tests/cases/fourslash/moveToNewFile_requireImport1.ts b/tests/cases/fourslash/moveToNewFile_requireImport1.ts index 37fb2b7e1a44a..b3a42808ffe30 100644 --- a/tests/cases/fourslash/moveToNewFile_requireImport1.ts +++ b/tests/cases/fourslash/moveToNewFile_requireImport1.ts @@ -18,7 +18,7 @@ verify.moveToNewFile({ "/b.js": "", "/f.js": -`var a = require("./a"); +`const a = require("./a"); function f() { a; diff --git a/tests/cases/fourslash/moveToNewFile_requireImport2.ts b/tests/cases/fourslash/moveToNewFile_requireImport2.ts index ebd39990ffab9..1c8a464ce4eff 100644 --- a/tests/cases/fourslash/moveToNewFile_requireImport2.ts +++ b/tests/cases/fourslash/moveToNewFile_requireImport2.ts @@ -23,7 +23,7 @@ verify.moveToNewFile({ `, "/f.js": -`var b = require("./a"); +`const b = require("./a"); function f() { b; diff --git a/tests/cases/fourslash/moveToNewFile_requireImport3.ts b/tests/cases/fourslash/moveToNewFile_requireImport3.ts deleted file mode 100644 index 88eba6b77d461..0000000000000 --- a/tests/cases/fourslash/moveToNewFile_requireImport3.ts +++ /dev/null @@ -1,33 +0,0 @@ -/// - -// @module: commonjs -// @moduleResolution: node - -// @filename: /node.d.ts -////declare var module: any; -////declare var require: any; - -// @filename: /a.ts -////module.exports = 1; - -// @filename: /b.ts -////var a = require("./a"); -////[|function f() { -//// a; -////}|] - -verify.noErrors(); - -verify.moveToNewFile({ - newFileContents: { - "/b.ts": "", - - "/f.ts": -`var a = require("./a"); - -function f() { - a; -} -`, - }, -}); diff --git a/tests/cases/fourslash/moveToNewFile_typeImport2.ts b/tests/cases/fourslash/moveToNewFile_typeImport2.ts index f916330c12322..4a902dba69795 100644 --- a/tests/cases/fourslash/moveToNewFile_typeImport2.ts +++ b/tests/cases/fourslash/moveToNewFile_typeImport2.ts @@ -13,7 +13,7 @@ verify.moveToNewFile({ newFileContents: { "/b.ts": "", "/f.ts": -`import { type A } from "./a"; +`import type { A } from "./a"; function f(a: A) { } `, diff --git a/tests/cases/fourslash/moveToNewFile_updateUses.ts b/tests/cases/fourslash/moveToNewFile_updateUses.ts index 7b29c07acdd8a..62dba22b0a508 100644 --- a/tests/cases/fourslash/moveToNewFile_updateUses.ts +++ b/tests/cases/fourslash/moveToNewFile_updateUses.ts @@ -7,7 +7,6 @@ // @Filename: /user.ts ////import { x, y } from "./a"; ////import { x as x2 } from "./a"; -////import { y as y2 } from "./a"; ////import {} from "./a"; verify.moveToNewFile({ @@ -24,7 +23,6 @@ verify.moveToNewFile({ `import { x } from "./a"; import { y } from "./y"; import { x as x2 } from "./a"; -import { y as y2 } from "./y"; import {} from "./a";`, }, }); From eb76c13fb5d950b993c826e42d985edabc2584c3 Mon Sep 17 00:00:00 2001 From: navya9singh Date: Fri, 19 Apr 2024 15:35:18 -0700 Subject: [PATCH 2/6] fixing errors --- src/compiler/commandLineParser.ts | 2 +- src/services/refactors/helpers.ts | 3 +- src/services/refactors/moveToFile.ts | 82 +++++++++++-------- ...-file'-and-'move-to-new-file'-refactors.js | 20 ++--- .../fourslash/moveToNewFile_importEquals.ts | 3 +- .../fourslash/moveToNewFile_moveJsxImport1.ts | 3 +- 6 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 4aff5cdc2648e..d39aa9562c1b0 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -3639,7 +3639,7 @@ export function convertJsonOption( convertJsonOption(opt.element, value, basePath, errors, propertyAssignment, valueExpression, sourceFile); } else if (!isString(opt.type)) { - return convertJsonOptionOfCustomType(opt as CommandLineOptionOfCustomType, value as string, errors, valueExpression, sourceFile); + return convertJsonOptionOfCustomType(opt, value as string, errors, valueExpression, sourceFile); } const validatedValue = validateJsonOptionValue(opt, value, errors, valueExpression, sourceFile); return isNullOrUndefined(validatedValue) ? validatedValue : normalizeNonListOptionValue(opt, basePath, validatedValue); diff --git a/src/services/refactors/helpers.ts b/src/services/refactors/helpers.ts index 47c774896d631..51ef3895c7816 100644 --- a/src/services/refactors/helpers.ts +++ b/src/services/refactors/helpers.ts @@ -9,7 +9,6 @@ import { Symbol, TypeChecker, } from "../_namespaces/ts"; -import { ImportOrRequireAliasDeclaration } from "../_namespaces/ts.codefix"; import { addImportsForMovedSymbols } from "./moveToFile"; /** * Returned by refactor functions when some error message needs to be surfaced to users. @@ -43,7 +42,7 @@ export function refactorKindBeginsWith(known: string, requested: string | undefi /** @internal */ export function addTargetFileImports( oldFile: SourceFile, - importsToCopy: Map, + importsToCopy: Map, targetFileImportsFromOldFile: Map, checker: TypeChecker, program: Program, diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index 64a4b6ca951d2..cd8c086e7dd03 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -155,10 +155,6 @@ import { VariableDeclarationList, VariableStatement, } from "../_namespaces/ts"; -import { - ImportAdder, - ImportOrRequireAliasDeclaration, -} from "../_namespaces/ts.codefix"; import { addTargetFileImports } from "../_namespaces/ts.refactor"; import { registerRefactor } from "../refactorProvider"; @@ -252,8 +248,8 @@ export function getNewStatementsAndRemoveFromOldFile( const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(targetFile.fileName, program, host, !!oldFile.commonJsModuleIndicator); const quotePreference = getQuotePreference(oldFile, preferences); - addImportsForMovedSymbols(usage.oldFileImportsFromTargetFile, targetFile.fileName, importAdderForOldFile, program); // imports for moved symbols in the target file - deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker, importAdderForOldFile); + addImportsForMovedSymbols(usage.oldFileImportsFromTargetFile, targetFile.fileName, importAdderForOldFile, program); + deleteUnusedOldImports(oldFile, toMove.all, usage.unusedImportsFromOldFile, importAdderForOldFile); importAdderForOldFile.writeFixes(changes, quotePreference); deleteMovedStatements(oldFile, toMove.ranges, changes); updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, targetFile.fileName, quotePreference); @@ -299,35 +295,15 @@ export function deleteMovedStatements(sourceFile: SourceFile, moved: readonly St } /** @internal */ -export function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Statement[], changes: textChanges.ChangeTracker, toDelete: Set, checker: TypeChecker, importAdder: ImportAdder) { +export function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Statement[], toDelete: Set, importAdder: codefix.ImportAdder) { for (const statement of oldFile.statements) { if (contains(toMove, statement)) continue; forEachImportInStatement(statement, i => { - if (i.kind === SyntaxKind.ImportDeclaration) { - if (i.importClause?.name && toDelete.has(Debug.checkDefined(i.importClause.symbol))) { - importAdder.removeExistingImport(i.importClause); - } - if (i.importClause?.namedBindings?.kind === SyntaxKind.NamespaceImport && toDelete.has(Debug.checkDefined(i.importClause.namedBindings.symbol))) { - importAdder.removeExistingImport(i.importClause.namedBindings); - } - if (i.importClause?.namedBindings?.kind === SyntaxKind.NamedImports) { - for (const element of i.importClause.namedBindings.elements) { - if (toDelete.has(Debug.checkDefined(element.symbol))) { - importAdder.removeExistingImport(element); - } - } - } - } - else if (i.kind === SyntaxKind.VariableDeclaration && i.name.kind === SyntaxKind.Identifier && toDelete.has(Debug.checkDefined(i.symbol))) { - importAdder.removeExistingImport(i); - } - else if (i.name.kind === SyntaxKind.ObjectBindingPattern) { - for (const element of i.name.elements) { - if (toDelete.has(Debug.checkDefined(element.symbol))) { - importAdder.removeExistingImport(element); - } + forEachAliasDeclarationInImportOrRequire(i, decl => { + if (toDelete.has(decl.symbol)) { + importAdder.removeExistingImport(decl); } - } + }); }); } } @@ -487,6 +463,37 @@ export function forEachImportInStatement(statement: Statement, cb: (importNode: } } +function forEachAliasDeclarationInImportOrRequire(importOrRequire: SupportedImport, cb: (declaration: codefix.ImportOrRequireAliasDeclaration) => void): void { + if (importOrRequire.kind === SyntaxKind.ImportDeclaration) { + if (importOrRequire.importClause?.name) { + cb(importOrRequire.importClause); + } + if (importOrRequire.importClause?.namedBindings?.kind === SyntaxKind.NamespaceImport) { + cb(importOrRequire.importClause.namedBindings); + } + if (importOrRequire.importClause?.namedBindings?.kind === SyntaxKind.NamedImports) { + for (const element of importOrRequire.importClause.namedBindings.elements) { + cb(element); + } + } + } + else if (importOrRequire.kind === SyntaxKind.ImportEqualsDeclaration) { + cb(importOrRequire); + } + else if (importOrRequire.kind === SyntaxKind.VariableDeclaration) { + if (importOrRequire.name.kind === SyntaxKind.Identifier) { + cb(importOrRequire); + } + else if (importOrRequire.name.kind === SyntaxKind.ObjectBindingPattern) { + for (const element of importOrRequire.name.elements) { + if (isIdentifier(element.name)) { + cb(element); + } + } + } + } +} + /** @internal */ export type SupportedImport = | ImportDeclaration & { moduleSpecifier: StringLiteralLike; } @@ -541,6 +548,11 @@ function isExported(sourceFile: SourceFile, decl: TopLevelDeclarationStatement, /** @internal */ export function deleteUnusedImports(sourceFile: SourceFile, importDecl: SupportedImport, changes: textChanges.ChangeTracker, isUnused: (name: Identifier) => boolean): void { + forEachAliasDeclarationInImportOrRequire(importDecl, i => { + if (i.kind === SyntaxKind.ImportEqualsDeclaration) { + changes.delete(sourceFile, i); + } + }) switch (importDecl.kind) { case SyntaxKind.ImportDeclaration: deleteUnusedImportsInDeclaration(sourceFile, importDecl, changes, isUnused); @@ -790,7 +802,7 @@ export interface UsageInfo { /** Subset of movedSymbols that are still used elsewhere in the old file and must be imported back. */ readonly oldFileImportsFromTargetFile: Map; - readonly oldImportsNeededByTargetFile: Map; + readonly oldImportsNeededByTargetFile: Map; /** Subset of oldImportsNeededByTargetFile that are will no longer be used in the old file. */ readonly unusedImportsFromOldFile: Set; } @@ -925,13 +937,13 @@ function isPureImport(node: Node): boolean { /** @internal */ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], checker: TypeChecker, existingTargetLocals: ReadonlySet = new Set()): UsageInfo { const movedSymbols = new Set(); - const oldImportsNeededByTargetFile = new Map(); + const oldImportsNeededByTargetFile = new Map(); const targetFileImportsFromOldFile = new Map(); const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx(toMove)); if (jsxNamespaceSymbol) { // Might not exist (e.g. in non-compiling code) - oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, [false, tryCast(jsxNamespaceSymbol.declarations?.[0], (d): d is ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d))]); + oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, [false, tryCast(jsxNamespaceSymbol.declarations?.[0], (d): d is codefix.ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d))]); } for (const statement of toMove) { @@ -955,7 +967,7 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[], const prevIsTypeOnly = oldImportsNeededByTargetFile.get(symbol); oldImportsNeededByTargetFile.set(symbol, [ prevIsTypeOnly === undefined ? isValidTypeOnlyUseSite : prevIsTypeOnly && isValidTypeOnlyUseSite, - tryCast(decl, (d): d is ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d)), + tryCast(decl, (d): d is codefix.ImportOrRequireAliasDeclaration => isImportSpecifier(d) || isImportClause(d) || isNamespaceImport(d) || isImportEqualsDeclaration(d) || isBindingElement(d) || isVariableDeclaration(d)), ]); } else if (isTopLevelDeclaration(decl) && sourceFileOfTopLevelDeclaration(decl) === oldFile && !movedSymbols.has(symbol)) { diff --git a/tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js b/tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js index fdc2c35dd50be..216d84e744946 100644 --- a/tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js +++ b/tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js @@ -107,13 +107,13 @@ Info seq [hh:mm:ss:mss] response: { "response": [ { - "name": "Move to a new file", - "description": "Move to a new file", + "name": "Move to file", + "description": "Move to file", "actions": [ { - "name": "Move to a new file", - "description": "Move to a new file", - "kind": "refactor.move.newFile", + "name": "Move to file", + "description": "Move to file", + "kind": "refactor.move.file", "range": { "start": { "line": 1, @@ -128,13 +128,13 @@ Info seq [hh:mm:ss:mss] response: ] }, { - "name": "Move to file", - "description": "Move to file", + "name": "Move to a new file", + "description": "Move to a new file", "actions": [ { - "name": "Move to file", - "description": "Move to file", - "kind": "refactor.move.file", + "name": "Move to a new file", + "description": "Move to a new file", + "kind": "refactor.move.newFile", "range": { "start": { "line": 1, diff --git a/tests/cases/fourslash/moveToNewFile_importEquals.ts b/tests/cases/fourslash/moveToNewFile_importEquals.ts index 589c4b32249fc..87199e15437a6 100644 --- a/tests/cases/fourslash/moveToNewFile_importEquals.ts +++ b/tests/cases/fourslash/moveToNewFile_importEquals.ts @@ -9,8 +9,7 @@ verify.moveToNewFile({ newFileContents: { "/a.ts": -`import i = require("./i"); -import j = require("./j"); +`import j = require("./j"); j;`, "/y.ts": `import i = require("./i"); diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts index fa0a5042be259..f1c86b53a51cc 100644 --- a/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts +++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts @@ -12,8 +12,7 @@ verify.moveToNewFile({ newFileContents: { "/tests/cases/fourslash/file.tsx": -`import React = require('react'); -1;`, +`1;`, "/tests/cases/fourslash/newFile.tsx": `import React = require('react'); From 8e2774ab399738fd9ca82280b8492bdba4c3c707 Mon Sep 17 00:00:00 2001 From: navya9singh Date: Fri, 19 Apr 2024 15:41:08 -0700 Subject: [PATCH 3/6] removing extra code --- src/compiler/commandLineParser.ts | 2 +- src/services/refactors/moveToFile.ts | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index d39aa9562c1b0..4aff5cdc2648e 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -3639,7 +3639,7 @@ export function convertJsonOption( convertJsonOption(opt.element, value, basePath, errors, propertyAssignment, valueExpression, sourceFile); } else if (!isString(opt.type)) { - return convertJsonOptionOfCustomType(opt, value as string, errors, valueExpression, sourceFile); + return convertJsonOptionOfCustomType(opt as CommandLineOptionOfCustomType, value as string, errors, valueExpression, sourceFile); } const validatedValue = validateJsonOptionValue(opt, value, errors, valueExpression, sourceFile); return isNullOrUndefined(validatedValue) ? validatedValue : normalizeNonListOptionValue(opt, basePath, validatedValue); diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index cd8c086e7dd03..0406f41d15212 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -548,11 +548,6 @@ function isExported(sourceFile: SourceFile, decl: TopLevelDeclarationStatement, /** @internal */ export function deleteUnusedImports(sourceFile: SourceFile, importDecl: SupportedImport, changes: textChanges.ChangeTracker, isUnused: (name: Identifier) => boolean): void { - forEachAliasDeclarationInImportOrRequire(importDecl, i => { - if (i.kind === SyntaxKind.ImportEqualsDeclaration) { - changes.delete(sourceFile, i); - } - }) switch (importDecl.kind) { case SyntaxKind.ImportDeclaration: deleteUnusedImportsInDeclaration(sourceFile, importDecl, changes, isUnused); @@ -585,11 +580,12 @@ function deleteUnusedImportsInDeclaration(sourceFile: SourceFile, importDecl: Im } if (namedBindings) { if (namedBindingsUnused) { - changes.replaceNode( - sourceFile, - importDecl.importClause, - factory.updateImportClause(importDecl.importClause, importDecl.importClause.isTypeOnly, name, /*namedBindings*/ undefined), - ); + // changes.replaceNode( + // sourceFile, + // importDecl.importClause, + // factory.updateImportClause(importDecl.importClause, importDecl.importClause.isTypeOnly, name, /*namedBindings*/ undefined), + // ); + changes.delete(sourceFile, importDecl.importClause); } else if (namedBindings.kind === SyntaxKind.NamedImports) { for (const element of namedBindings.elements) { From a4eca0b745bb693b4c3b5f149c07c3fea15303ec Mon Sep 17 00:00:00 2001 From: navya9singh Date: Fri, 19 Apr 2024 15:44:07 -0700 Subject: [PATCH 4/6] removing extra code --- src/services/refactors/moveToFile.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index 0406f41d15212..6f7c0a3979d7d 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -580,12 +580,11 @@ function deleteUnusedImportsInDeclaration(sourceFile: SourceFile, importDecl: Im } if (namedBindings) { if (namedBindingsUnused) { - // changes.replaceNode( - // sourceFile, - // importDecl.importClause, - // factory.updateImportClause(importDecl.importClause, importDecl.importClause.isTypeOnly, name, /*namedBindings*/ undefined), - // ); - changes.delete(sourceFile, importDecl.importClause); + changes.replaceNode( + sourceFile, + importDecl.importClause, + factory.updateImportClause(importDecl.importClause, importDecl.importClause.isTypeOnly, name, /*namedBindings*/ undefined), + ); } else if (namedBindings.kind === SyntaxKind.NamedImports) { for (const element of namedBindings.elements) { From e23ec28e2392fd2376256567378934ae8aa99f59 Mon Sep 17 00:00:00 2001 From: navya9singh Date: Thu, 25 Apr 2024 12:52:10 -0700 Subject: [PATCH 5/6] using `forEachAliasDeclarationInImportOrRequire` at more places --- src/services/refactors/moveToFile.ts | 79 +++---------------- .../fourslash/moveToNewFile_updateUses_js.ts | 2 +- 2 files changed, 10 insertions(+), 71 deletions(-) diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index 6f7c0a3979d7d..d1c3cee98c5b4 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -108,7 +108,6 @@ import { isValidTypeOnlyAliasUseSite, isVariableDeclaration, isVariableDeclarationInitializedToRequire, - isVariableDeclarationList, isVariableStatement, LanguageServiceHost, last, @@ -548,78 +547,18 @@ function isExported(sourceFile: SourceFile, decl: TopLevelDeclarationStatement, /** @internal */ export function deleteUnusedImports(sourceFile: SourceFile, importDecl: SupportedImport, changes: textChanges.ChangeTracker, isUnused: (name: Identifier) => boolean): void { - switch (importDecl.kind) { - case SyntaxKind.ImportDeclaration: - deleteUnusedImportsInDeclaration(sourceFile, importDecl, changes, isUnused); - break; - case SyntaxKind.ImportEqualsDeclaration: - if (isUnused(importDecl.name)) { - changes.delete(sourceFile, importDecl); - } - break; - case SyntaxKind.VariableDeclaration: - deleteUnusedImportsInVariableDeclaration(sourceFile, importDecl, changes, isUnused); - break; - default: - Debug.assertNever(importDecl, `Unexpected import decl kind ${(importDecl as SupportedImport).kind}`); - } -} - -function deleteUnusedImportsInDeclaration(sourceFile: SourceFile, importDecl: ImportDeclaration, changes: textChanges.ChangeTracker, isUnused: (name: Identifier) => boolean): void { - if (!importDecl.importClause) return; - const { name, namedBindings } = importDecl.importClause; - const defaultUnused = !name || isUnused(name); - const namedBindingsUnused = !namedBindings || - (namedBindings.kind === SyntaxKind.NamespaceImport ? isUnused(namedBindings.name) : namedBindings.elements.length !== 0 && namedBindings.elements.every(e => isUnused(e.name))); - if (defaultUnused && namedBindingsUnused) { - changes.delete(sourceFile, importDecl); - } - else { - if (name && defaultUnused) { - changes.delete(sourceFile, name); - } - if (namedBindings) { - if (namedBindingsUnused) { - changes.replaceNode( - sourceFile, - importDecl.importClause, - factory.updateImportClause(importDecl.importClause, importDecl.importClause.isTypeOnly, name, /*namedBindings*/ undefined), - ); - } - else if (namedBindings.kind === SyntaxKind.NamedImports) { - for (const element of namedBindings.elements) { - if (isUnused(element.name)) changes.delete(sourceFile, element); - } - } + if (importDecl.kind === SyntaxKind.ImportDeclaration && importDecl.importClause) { + const { name, namedBindings } = importDecl.importClause; + if ((!name || isUnused(name)) && (!namedBindings || namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length !== 0 && namedBindings.elements.every(e => isUnused(e.name)))) { + return changes.delete(sourceFile, importDecl); } } -} -function deleteUnusedImportsInVariableDeclaration(sourceFile: SourceFile, varDecl: VariableDeclaration, changes: textChanges.ChangeTracker, isUnused: (name: Identifier) => boolean) { - const { name } = varDecl; - switch (name.kind) { - case SyntaxKind.Identifier: - if (isUnused(name)) { - if (varDecl.initializer && isRequireCall(varDecl.initializer, /*requireStringLiteralLikeArgument*/ true)) { - changes.delete(sourceFile, isVariableDeclarationList(varDecl.parent) && length(varDecl.parent.declarations) === 1 ? varDecl.parent.parent : varDecl); - } - } - break; - case SyntaxKind.ArrayBindingPattern: - break; - case SyntaxKind.ObjectBindingPattern: - if (name.elements.every(e => isIdentifier(e.name) && isUnused(e.name))) { - changes.delete(sourceFile, isVariableDeclarationList(varDecl.parent) && varDecl.parent.declarations.length === 1 ? varDecl.parent.parent : varDecl); - } - else { - for (const element of name.elements) { - if (isIdentifier(element.name) && isUnused(element.name)) { - changes.delete(sourceFile, element.name); - } - } - } - break; - } + forEachAliasDeclarationInImportOrRequire(importDecl, i => { + if (i.name && isIdentifier(i.name) && isUnused(i.name)) { + changes.delete(sourceFile, i); + } + }); } /** @internal */ diff --git a/tests/cases/fourslash/moveToNewFile_updateUses_js.ts b/tests/cases/fourslash/moveToNewFile_updateUses_js.ts index 8addd34219781..b335f4932bac8 100644 --- a/tests/cases/fourslash/moveToNewFile_updateUses_js.ts +++ b/tests/cases/fourslash/moveToNewFile_updateUses_js.ts @@ -24,7 +24,7 @@ verify.moveToNewFile({ "/user.js": // TODO: GH#22330 -`const { x, } = require("./a"); +`const { x } = require("./a"); const { y } = require("./y"); `, }, From 37f8d0fbed4578f02273aff3289a757bf940b709 Mon Sep 17 00:00:00 2001 From: navya9singh Date: Thu, 25 Apr 2024 14:24:20 -0700 Subject: [PATCH 6/6] fixing typo --- src/services/refactors/moveToFile.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index d1c3cee98c5b4..dea0ebc4c43e1 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -252,7 +252,7 @@ export function getNewStatementsAndRemoveFromOldFile( importAdderForOldFile.writeFixes(changes, quotePreference); deleteMovedStatements(oldFile, toMove.ranges, changes); updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, targetFile.fileName, quotePreference); - addExportsInOldFIle(oldFile, usage.targetFileImportsFromOldFile, changes, useEsModuleSyntax); + addExportsInOldFile(oldFile, usage.targetFileImportsFromOldFile, changes, useEsModuleSyntax); addTargetFileImports(oldFile, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, checker, program, importAdderForNewFile); if (!isFullSourceFile(targetFile) && prologueDirectives.length) { // Ensure prologue directives come before imports @@ -307,7 +307,7 @@ export function deleteUnusedOldImports(oldFile: SourceFile, toMove: readonly Sta } } -function addExportsInOldFIle(oldFile: SourceFile, targetFileImportsFromOldFile: Map, changes: textChanges.ChangeTracker, useEsModuleSyntax: boolean) { +function addExportsInOldFile(oldFile: SourceFile, targetFileImportsFromOldFile: Map, changes: textChanges.ChangeTracker, useEsModuleSyntax: boolean) { const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`. targetFileImportsFromOldFile.forEach((_, symbol) => { if (!symbol.declarations) {