Skip to content

Commit d7b8662

Browse files
authored
Fix for formatter crash for Move to file (microsoft#54199)
1 parent 8b825f7 commit d7b8662

File tree

2 files changed

+84
-10
lines changed

2 files changed

+84
-10
lines changed

src/services/textChanges.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@ import {
5252
getNewLineKind,
5353
getNewLineOrDefaultFromHost,
5454
getNodeId,
55+
getOriginalNode,
5556
getPrecedingNonSpaceCharacterPosition,
5657
getScriptKindFromFileName,
5758
getShebang,
59+
getSourceFileOfNode,
5860
getStartPositionOfLine,
5961
getTokenAtPosition,
6062
getTouchingToken,
@@ -1238,9 +1240,12 @@ namespace changesToText {
12381240

12391241
const textChanges = mapDefined(normalized, c => {
12401242
const span = createTextSpanFromRange(c.range);
1241-
const newText = computeNewText(c, sourceFile, newLineCharacter, formatContext, validate);
1243+
const targetSourceFile = c.kind === ChangeKind.ReplaceWithSingleNode ? getSourceFileOfNode(getOriginalNode(c.node)) ?? c.sourceFile :
1244+
c.kind === ChangeKind.ReplaceWithMultipleNodes ? getSourceFileOfNode(getOriginalNode(c.nodes[0])) ?? c.sourceFile :
1245+
c.sourceFile;
1246+
const newText = computeNewText(c, targetSourceFile, sourceFile, newLineCharacter, formatContext, validate);
12421247
// Filter out redundant changes.
1243-
if (span.length === newText.length && stringContainsAt(sourceFile.text, newText, span.start)) {
1248+
if (span.length === newText.length && stringContainsAt(targetSourceFile.text, newText, span.start)) {
12441249
return undefined;
12451250
}
12461251

@@ -1264,7 +1269,7 @@ namespace changesToText {
12641269
return applyChanges(nonFormattedText, changes) + newLineCharacter;
12651270
}
12661271

1267-
function computeNewText(change: Change, sourceFile: SourceFile, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
1272+
function computeNewText(change: Change, targetSourceFile: SourceFile, sourceFile: SourceFile, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
12681273
if (change.kind === ChangeKind.Remove) {
12691274
return "";
12701275
}
@@ -1273,26 +1278,26 @@ namespace changesToText {
12731278
}
12741279

12751280
const { options = {}, range: { pos } } = change;
1276-
const format = (n: Node) => getFormattedTextOfNode(n, sourceFile, pos, options, newLineCharacter, formatContext, validate);
1281+
const format = (n: Node) => getFormattedTextOfNode(n, targetSourceFile, sourceFile, pos, options, newLineCharacter, formatContext, validate);
12771282
const text = change.kind === ChangeKind.ReplaceWithMultipleNodes
12781283
? change.nodes.map(n => removeSuffix(format(n), newLineCharacter)).join(change.options?.joiner || newLineCharacter)
12791284
: format(change.node);
12801285
// strip initial indentation (spaces or tabs) if text will be inserted in the middle of the line
1281-
const noIndent = (options.indentation !== undefined || getLineStartPositionForPosition(pos, sourceFile) === pos) ? text : text.replace(/^\s+/, "");
1286+
const noIndent = (options.indentation !== undefined || getLineStartPositionForPosition(pos, targetSourceFile) === pos) ? text : text.replace(/^\s+/, "");
12821287
return (options.prefix || "") + noIndent
12831288
+ ((!options.suffix || endsWith(noIndent, options.suffix))
12841289
? "" : options.suffix);
12851290
}
12861291

12871292
/** Note: this may mutate `nodeIn`. */
1288-
function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
1289-
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
1293+
function getFormattedTextOfNode(nodeIn: Node, targetSourceFile: SourceFile, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string {
1294+
const { node, text } = getNonformattedText(nodeIn, targetSourceFile, newLineCharacter);
12901295
if (validate) validate(node, text);
1291-
const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile);
1296+
const formatOptions = getFormatCodeSettingsForWriting(formatContext, targetSourceFile);
12921297
const initialIndentation =
12931298
indentation !== undefined
12941299
? indentation
1295-
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
1300+
: formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, targetSourceFile) === pos);
12961301
if (delta === undefined) {
12971302
delta = formatting.SmartIndenter.shouldIndentChildNode(formatOptions, nodeIn) ? (formatOptions.indentSize || 0) : 0;
12981303
}
@@ -1303,7 +1308,7 @@ namespace changesToText {
13031308
return getLineAndCharacterOfPosition(this, pos);
13041309
}
13051310
};
1306-
const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, { ...formatContext, options: formatOptions });
1311+
const changes = formatting.formatNodeGivenIndentation(node, file, targetSourceFile.languageVariant, initialIndentation, delta, { ...formatContext, options: formatOptions });
13071312
return applyChanges(text, changes);
13081313
}
13091314

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//@Filename: /bar.ts
4+
////import { } from './somefile';
5+
////const a = 12;
6+
7+
//@Filename: /a.ts
8+
//// [|export type ProblemKind =
9+
//// | "NoResolution"
10+
//// | "UntypedResolution"
11+
//// | "FalseESM"
12+
//// | "FalseCJS"
13+
//// | "CJSResolvesToESM"
14+
//// | "Wildcard"
15+
//// | "UnexpectedESMSyntax"
16+
//// | "UnexpectedCJSSyntax"
17+
//// | "FallbackCondition"
18+
//// | "CJSOnlyExportsDefault"
19+
//// | "FalseExportDefault";
20+
21+
//// export interface Problem {
22+
//// kind: ProblemKind;
23+
//// entrypoint: string;
24+
//// }
25+
26+
//// export interface ProblemSummary {
27+
//// kind: ProblemKind;
28+
//// title: string;
29+
//// messages: {
30+
//// messageText: string;
31+
//// messageHtml: string;
32+
//// }[];
33+
//// }|]
34+
35+
verify.moveToFile({
36+
newFileContents: {
37+
"/a.ts":
38+
``,
39+
40+
"/bar.ts":
41+
`import { } from './somefile';
42+
const a = 12;
43+
export type ProblemKind = "NoResolution" |
44+
"UntypedResolution" |
45+
"FalseESM" |
46+
"FalseCJS" |
47+
"CJSResolvesToESM" |
48+
"Wildcard" |
49+
"UnexpectedESMSyntax" |
50+
"UnexpectedCJSSyntax" |
51+
"FallbackCondition" |
52+
"CJSOnlyExportsDefault" |
53+
"FalseExportDefault";
54+
export interface Problem {
55+
kind: ProblemKind;
56+
entrypoint: string;
57+
}
58+
export interface ProblemSummary {
59+
kind: ProblemKind;
60+
title: string;
61+
messages: {
62+
messageText: string;
63+
messageHtml: string;
64+
}[];
65+
}
66+
`,
67+
},
68+
interactiveRefactorArguments: { targetFile: "/bar.ts" }
69+
});

0 commit comments

Comments
 (0)