diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index e90b8668ffc..cd505476e2b 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2985,7 +2985,7 @@ Actual: ${stringify(fullActual)}`); } public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) { - const isAvailable = this.getApplicableRefactors(this.getMarkerByName(markerName).position).length > 0; + const isAvailable = this.getApplicableRefactors(this.getMarkerByName(markerName)).length > 0; if (negative && isAvailable) { this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`); } @@ -3002,7 +3002,7 @@ Actual: ${stringify(fullActual)}`); } public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) { - let refactors = this.getApplicableRefactors(this.getSelection()); + let refactors = this.getApplicableRefactorsAtSelection(); refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName))); const isAvailable = refactors.length > 0; @@ -3022,11 +3022,11 @@ Actual: ${stringify(fullActual)}`); } public verifyRefactorsAvailable(names: ReadonlyArray): void { - assert.deepEqual(unique(this.getApplicableRefactors(this.getSelection()), r => r.name), names); + assert.deepEqual(unique(this.getApplicableRefactorsAtSelection(), r => r.name), names); } public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) { - const actualRefactors = this.getApplicableRefactors(this.getSelection()).filter(r => r.name === name && r.actions.some(a => a.name === actionName)); + const actualRefactors = this.getApplicableRefactorsAtSelection().filter(r => r.name === name && r.actions.some(a => a.name === actionName)); this.assertObjectsEqual(actualRefactors, refactors); } @@ -3047,7 +3047,7 @@ Actual: ${stringify(fullActual)}`); public applyRefactor({ refactorName, actionName, actionDescription, newContent: newContentWithRenameMarker }: FourSlashInterface.ApplyRefactorOptions) { const range = this.getSelection(); - const refactors = this.getApplicableRefactors(range); + const refactors = this.getApplicableRefactorsAtSelection(); const refactorsWithName = refactors.filter(r => r.name === refactorName); if (refactorsWithName.length === 0) { this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.\nAvailable refactors: ${refactors.map(r => r.name)}`); @@ -3125,7 +3125,7 @@ Actual: ${stringify(fullActual)}`); const action = ts.first(refactor.actions); assert(action.name === "Move to a new file" && action.description === "Move to a new file"); - const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactor.name, action.name, options.preferences || ts.emptyOptions)!; + const editInfo = this.languageService.getEditsForRefactor(range.fileName, this.formatCodeSettings, range, refactor.name, action.name, options.preferences || ts.emptyOptions)!; this.testNewFileContents(editInfo.edits, options.newFileContents, "move to new file"); } @@ -3165,21 +3165,21 @@ Actual: ${stringify(fullActual)}`); formattingOptions?: ts.FormatCodeSettings) { formattingOptions = formattingOptions || this.formatCodeSettings; - const markerPos = this.getMarkerByName(markerName).position; + const marker = this.getMarkerByName(markerName); - const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos, ts.emptyOptions); + const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position, ts.emptyOptions); const applicableRefactorToApply = ts.find(applicableRefactors, refactor => refactor.name === refactorNameToApply); if (!applicableRefactorToApply) { this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`); } - const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName, ts.emptyOptions)!; + const editInfo = this.languageService.getEditsForRefactor(marker.fileName, formattingOptions, marker.position, refactorNameToApply, actionName, ts.emptyOptions)!; for (const edit of editInfo.edits) { this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false); } - const actualContent = this.getFileContent(this.activeFile.fileName); + const actualContent = this.getFileContent(marker.fileName); if (actualContent !== expectedContent) { this.raiseError(`verifyFileAfterApplyingRefactors failed:\n${showTextDiff(expectedContent, actualContent)}`); @@ -3381,8 +3381,14 @@ Actual: ${stringify(fullActual)}`); test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved"); } - private getApplicableRefactors(positionOrRange: number | ts.TextRange, preferences = ts.emptyOptions): ReadonlyArray { - return this.languageService.getApplicableRefactors(this.activeFile.fileName, positionOrRange, preferences) || ts.emptyArray; + private getApplicableRefactorsAtSelection() { + return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName); + } + private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions): ReadonlyArray { + return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences); + } + private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions): ReadonlyArray { + return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences) || ts.emptyArray; } } diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index b10f2969906..9953293e3d1 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -340,7 +340,7 @@ namespace ts.refactor { const { name, namedBindings } = importDecl.importClause; const defaultUnused = !name || isUnused(name); const namedBindingsUnused = !namedBindings || - (namedBindings.kind === SyntaxKind.NamespaceImport ? isUnused(namedBindings.name) : namedBindings.elements.every(e => isUnused(e.name))); + (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); } diff --git a/tests/cases/fourslash/moveToNewFile_updateUses.ts b/tests/cases/fourslash/moveToNewFile_updateUses.ts index b067889625e..b1ab56cd763 100644 --- a/tests/cases/fourslash/moveToNewFile_updateUses.ts +++ b/tests/cases/fourslash/moveToNewFile_updateUses.ts @@ -6,9 +6,9 @@ // @Filename: /user.ts ////import { x, y } from "./a"; -//// - -// TODO: GH#23728 Shouldn't need `////` above +////import { x as x2 } from "./a"; +////import { y as y2 } from "./a"; +////import {} from "./a"; verify.moveToNewFile({ newFileContents: { @@ -22,6 +22,8 @@ verify.moveToNewFile({ "/user.ts": `import { x } from "./a"; import { y } from "./y"; -`, +import { x as x2 } from "./a"; +import { y as y2 } from "./y"; +import {} from "./a";`, }, });