Skip to content

Commit

Permalink
fix(57302): Cannot move symbols from tsx to ts files (#57305)
Browse files Browse the repository at this point in the history
  • Loading branch information
a-tarasyuk authored Feb 16, 2024
1 parent c4de2af commit c18c1c2
Show file tree
Hide file tree
Showing 6 changed files with 589 additions and 17 deletions.
14 changes: 8 additions & 6 deletions src/services/refactors/moveToFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -893,12 +893,10 @@ export interface TopLevelVariableDeclaration extends VariableDeclaration {
export type TopLevelDeclaration = NonVariableTopLevelDeclaration | TopLevelVariableDeclaration | BindingElement;

/** @internal */
export function createNewFileName(oldFile: SourceFile, program: Program, context: RefactorContext, host: LanguageServiceHost): string {
export function createNewFileName(oldFile: SourceFile, program: Program, host: LanguageServiceHost, toMove: ToMove | undefined): string {
const checker = program.getTypeChecker();
const toMove = getStatementsToMove(context);
let usage;
if (toMove) {
usage = getUsageInfo(oldFile, toMove.all, checker);
const usage = getUsageInfo(oldFile, toMove.all, checker);
const currentDirectory = getDirectoryPath(oldFile.fileName);
const extension = extensionFromPath(oldFile.fileName);
const newFileName = combinePaths(
Expand Down Expand Up @@ -974,6 +972,11 @@ export function getStatementsToMove(context: RefactorContext): ToMove | undefine
return all.length === 0 ? undefined : { all, ranges };
}

/** @internal */
export function containsJsx(statements: readonly Statement[] | undefined) {
return find(statements, statement => !!(statement.transformFlags & TransformFlags.ContainsJsx));
}

function isAllowedStatementToMove(statement: Statement): boolean {
// Filters imports and prologue directives out of the range of statements to move.
// Imports will be copied to the new file anyway, and may still be needed in the old file.
Expand All @@ -1000,8 +1003,7 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[],
const oldImportsNeededByTargetFile = new Map<Symbol, /*isValidTypeOnlyUseSite*/ boolean>();
const targetFileImportsFromOldFile = new Set<Symbol>();

const containsJsx = find(toMove, statement => !!(statement.transformFlags & TransformFlags.ContainsJsx));
const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx);
const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx(toMove));

if (jsxNamespaceSymbol) { // Might not exist (e.g. in non-compiling code)
oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, false);
Expand Down
8 changes: 3 additions & 5 deletions src/services/refactors/moveToNewFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
nodeSeenTracker,
Program,
QuotePreference,
RefactorContext,
RefactorEditInfo,
SourceFile,
Symbol,
Expand Down Expand Up @@ -75,16 +74,15 @@ 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, context));
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences));
return { edits, renameFilename: undefined, renameLocation: undefined };
},
});

function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences, context: RefactorContext): void {
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void {
const checker = program.getTypeChecker();
const usage = getUsageInfo(oldFile, toMove.all, checker);

const newFilename = createNewFileName(oldFile, program, context, host);
const newFilename = createNewFileName(oldFile, program, host, toMove);

// If previous file was global, this is easy.
changes.createNewFile(oldFile, newFilename, getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFilename, preferences));
Expand Down
21 changes: 15 additions & 6 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ import {
import * as NavigateTo from "./_namespaces/ts.NavigateTo";
import * as NavigationBar from "./_namespaces/ts.NavigationBar";
import {
containsJsx,
createNewFileName,
getStatementsToMove,
} from "./_namespaces/ts.refactor";
import * as classifier from "./classifier";
import * as classifier2020 from "./classifier2020";
Expand Down Expand Up @@ -3030,13 +3032,20 @@ export function createLanguageService(
const sourceFile = getValidSourceFile(fileName);
const allFiles = Debug.checkDefined(program.getSourceFiles());
const extension = extensionFromPath(fileName);
const files = mapDefined(allFiles, file =>
!program?.isSourceFileFromExternalLibrary(sourceFile) &&
!(sourceFile === getValidSourceFile(file.fileName) || extension === Extension.Ts && extensionFromPath(file.fileName) === Extension.Dts || extension === Extension.Dts && startsWith(getBaseFileName(file.fileName), "lib.") && extensionFromPath(file.fileName) === Extension.Dts)
&& extension === extensionFromPath(file.fileName) ? file.fileName : undefined);
const toMove = getStatementsToMove(getRefactorContext(sourceFile, positionOrRange, preferences, emptyOptions));
const toMoveContainsJsx = containsJsx(toMove?.all);

const files = mapDefined(allFiles, file => {
const fileNameExtension = extensionFromPath(file.fileName);
const isValidSourceFile = !program?.isSourceFileFromExternalLibrary(sourceFile) && !(
sourceFile === getValidSourceFile(file.fileName) ||
extension === Extension.Ts && fileNameExtension === Extension.Dts ||
extension === Extension.Dts && startsWith(getBaseFileName(file.fileName), "lib.") && fileNameExtension === Extension.Dts
);
return isValidSourceFile && (extension === fileNameExtension || (extension === Extension.Tsx && fileNameExtension === Extension.Ts || extension === Extension.Jsx && fileNameExtension === Extension.Js) && !toMoveContainsJsx) ? file.fileName : undefined;
});

const newFileName = createNewFileName(sourceFile, program, getRefactorContext(sourceFile, positionOrRange, preferences, emptyOptions), host);
return { newFileName, files };
return { newFileName: createNewFileName(sourceFile, program, host, toMove), files };
}

function getEditsForRefactor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,61 @@ import { value1 } from "../node_modules/.cache/someFile.d.ts";`,
});
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "skips lib.d.ts files", session);
});

it("should show ts files when moving non-tsx content from tsx file", () => {
const file1: File = {
path: "/bar.tsx",
content: `export function bar() { }`,
};
const file2: File = {
path: "/foo.ts",
content: "export function foo() { }",
};
const tsconfig: File = {
path: "/tsconfig.json",
content: jsonToReadableText({
jsx: "preserve",
files: ["./foo.ts", "./bar.tsx"],
}),
};

const host = createServerHost([file1, file2, tsconfig]);
const session = new TestSession(host);
openFilesForSession([file1], session);

session.executeCommandSeq<ts.server.protocol.GetMoveToRefactoringFileSuggestionsRequest>({
command: ts.server.protocol.CommandTypes.GetMoveToRefactoringFileSuggestions,
arguments: { file: file1.path, line: 1, offset: 7 },
});
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "should show ts files when moving non-tsx content from tsx file", session);
});

it("should show js files when moving non-jsx content from jsx file", () => {
const file1: File = {
path: "/bar.jsx",
content: `export function bar() { }`,
};
const file2: File = {
path: "/foo.js",
content: "export function foo() { }",
};
const tsconfig: File = {
path: "/tsconfig.json",
content: jsonToReadableText({
allowJS: true,
jsx: "preserve",
files: ["./foo.js", "./bar.jsx"],
}),
};

const host = createServerHost([file1, file2, tsconfig]);
const session = new TestSession(host);
openFilesForSession([file1], session);

session.executeCommandSeq<ts.server.protocol.GetMoveToRefactoringFileSuggestionsRequest>({
command: ts.server.protocol.CommandTypes.GetMoveToRefactoringFileSuggestions,
arguments: { file: file1.path, line: 1, offset: 7 },
});
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "should show js files when moving non-jsx content from jsx file", session);
});
});
Loading

0 comments on commit c18c1c2

Please sign in to comment.