Skip to content

Commit ea6740f

Browse files
author
Andy
authored
replaceNode: Always use non-adjusted end (microsoft#22519)
* replaceNode: Always use non-adjusted end * Never adjust start position either * Fix excess property checks, remove unnecessary arguments * Make 'insertText' and 'newLineCharacter' private * Use replaceNode in one more place now that it doesn't affect comments * Update replaceNodeRange too * Always ask for ChangeNodeOptions
1 parent 4fa9605 commit ea6740f

10 files changed

Lines changed: 70 additions & 70 deletions

src/services/codefixes/convertToEs6Module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ namespace ts.codefix {
257257
changes.replaceNodeWithNodes(sourceFile, statement, newNodes);
258258
}
259259
else {
260-
changes.replaceNode(sourceFile, statement, convertExportsDotXEquals(text, right), { useNonAdjustedEndPosition: true });
260+
changes.replaceNode(sourceFile, statement, convertExportsDotXEquals(text, right));
261261
}
262262
}
263263

src/services/codefixes/disableJsDiagnostics.ts

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace ts.codefix {
2727
fixId: undefined,
2828
}];
2929

30-
if (isValidSuppressLocation(sourceFile, span.start)) {
30+
if (isValidLocationToAddComment(sourceFile, span.start)) {
3131
fixes.unshift({
3232
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
3333
changes: textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, span.start)),
@@ -41,37 +41,22 @@ namespace ts.codefix {
4141
getAllCodeActions: context => {
4242
const seenLines = createMap<true>();
4343
return codeFixAll(context, errorCodes, (changes, diag) => {
44-
if (isValidSuppressLocation(diag.file!, diag.start!)) {
44+
if (isValidLocationToAddComment(diag.file!, diag.start!)) {
4545
makeChange(changes, diag.file!, diag.start!, seenLines);
4646
}
4747
});
4848
},
4949
});
5050

51-
function isValidSuppressLocation(sourceFile: SourceFile, position: number) {
51+
export function isValidLocationToAddComment(sourceFile: SourceFile, position: number) {
5252
return !isInComment(sourceFile, position) && !isInString(sourceFile, position) && !isInTemplateString(sourceFile, position);
5353
}
5454

5555
function makeChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, seenLines?: Map<true>) {
5656
const { line: lineNumber } = getLineAndCharacterOfPosition(sourceFile, position);
57-
5857
// Only need to add `// @ts-ignore` for a line once.
59-
if (seenLines && !addToSeen(seenLines, lineNumber)) {
60-
return;
58+
if (!seenLines || addToSeen(seenLines, lineNumber)) {
59+
changes.insertCommentBeforeLine(sourceFile, lineNumber, position, " @ts-ignore");
6160
}
62-
63-
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
64-
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
65-
66-
// First try to see if we can put the '// @ts-ignore' on the previous line.
67-
// We need to make sure that we are not in the middle of a string literal or a comment.
68-
// If so, we do not want to separate the node from its comment if we can.
69-
// Otherwise, add an extra new line immediately before the error span.
70-
const insertAtLineStart = isValidSuppressLocation(sourceFile, startPosition);
71-
72-
const token = getTouchingToken(sourceFile, insertAtLineStart ? startPosition : position, /*includeJsDocComment*/ false);
73-
const clone = setStartsOnNewLine(getSynthesizedDeepClone(token), true);
74-
addSyntheticLeadingComment(clone, SyntaxKind.SingleLineCommentTrivia, " @ts-ignore");
75-
changes.replaceNode(sourceFile, token, clone, { preserveLeadingWhitespace: true, prefix: insertAtLineStart ? undefined : changes.newLineCharacter });
7661
}
7762
}

src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace ts.codefix {
2727
}
2828

2929
function doChanges(changes: textChanges.ChangeTracker, sourceFile: SourceFile, extendsToken: Node, heritageClauses: ReadonlyArray<HeritageClause>): void {
30-
changes.replaceNode(sourceFile, extendsToken, createToken(SyntaxKind.ImplementsKeyword), textChanges.useNonAdjustedPositions);
30+
changes.replaceNode(sourceFile, extendsToken, createToken(SyntaxKind.ImplementsKeyword));
3131

3232
// If there is already an implements clause, replace the implements keyword with a comma.
3333
if (heritageClauses.length === 2 &&

src/services/codefixes/fixForgottenThisPropertyAccess.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,6 @@ namespace ts.codefix {
3030
}
3131
// TODO (https://github.com/Microsoft/TypeScript/issues/21246): use shared helper
3232
suppressLeadingAndTrailingTrivia(token);
33-
changes.replaceNode(sourceFile, token, createPropertyAccess(createThis(), token), textChanges.useNonAdjustedPositions);
33+
changes.replaceNode(sourceFile, token, createPropertyAccess(createThis(), token));
3434
}
3535
}

src/services/codefixes/fixInvalidImportSyntax.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ namespace ts.codefix {
4242
}
4343

4444
function createAction(context: CodeFixContext, sourceFile: SourceFile, node: Node, replacement: Node): CodeAction {
45-
// TODO: GH#21246 Should be able to use `replaceNode`, but be sure to preserve comments (see `codeFixCalledES2015Import11.ts`)
46-
const changes = textChanges.ChangeTracker.with(context, t => t.replaceRange(sourceFile, { pos: node.getStart(), end: node.end }, replacement));
45+
const changes = textChanges.ChangeTracker.with(context, t => t.replaceNode(sourceFile, node, replacement));
4746
return {
4847
description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Replace_import_with_0), [changes[0].textChanges[0].newText]),
4948
changes,

src/services/codefixes/fixStrictClassInitialization.ts

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,24 @@ namespace ts.codefix {
1010
const propertyDeclaration = getPropertyDeclaration(context.sourceFile, context.span.start);
1111
if (!propertyDeclaration) return;
1212

13-
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
1413
const result = [
1514
getActionForAddMissingUndefinedType(context, propertyDeclaration),
16-
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration, newLineCharacter)
15+
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration)
1716
];
1817

19-
append(result, getActionForAddMissingInitializer(context, propertyDeclaration, newLineCharacter));
18+
append(result, getActionForAddMissingInitializer(context, propertyDeclaration));
2019

2120
return result;
2221
},
2322
fixIds: [fixIdAddDefiniteAssignmentAssertions, fixIdAddUndefinedType, fixIdAddInitializer],
2423
getAllCodeActions: context => {
25-
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
26-
2724
return codeFixAll(context, errorCodes, (changes, diag) => {
2825
const propertyDeclaration = getPropertyDeclaration(diag.file, diag.start);
2926
if (!propertyDeclaration) return;
3027

3128
switch (context.fixId) {
3229
case fixIdAddDefiniteAssignmentAssertions:
33-
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration, newLineCharacter);
30+
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration);
3431
break;
3532
case fixIdAddUndefinedType:
3633
addUndefinedType(changes, diag.file, propertyDeclaration);
@@ -40,7 +37,7 @@ namespace ts.codefix {
4037
const initializer = getInitializer(checker, propertyDeclaration);
4138
if (!initializer) return;
4239

43-
addInitializer(changes, diag.file, propertyDeclaration, initializer, newLineCharacter);
40+
addInitializer(changes, diag.file, propertyDeclaration, initializer);
4441
break;
4542
default:
4643
Debug.fail(JSON.stringify(context.fixId));
@@ -54,13 +51,13 @@ namespace ts.codefix {
5451
return isIdentifier(token) ? cast(token.parent, isPropertyDeclaration) : undefined;
5552
}
5653

57-
function getActionForAddMissingDefiniteAssignmentAssertion (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction {
54+
function getActionForAddMissingDefiniteAssignmentAssertion (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
5855
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_definite_assignment_assertion_to_property_0), [propertyDeclaration.getText()]);
59-
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration, newLineCharacter));
56+
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration));
6057
return { description, changes, fixId: fixIdAddDefiniteAssignmentAssertions };
6158
}
6259

63-
function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): void {
60+
function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
6461
const property = updateProperty(
6562
propertyDeclaration,
6663
propertyDeclaration.decorators,
@@ -70,7 +67,7 @@ namespace ts.codefix {
7067
propertyDeclaration.type,
7168
propertyDeclaration.initializer
7269
);
73-
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property, { suffix: newLineCharacter });
70+
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
7471
}
7572

7673
function getActionForAddMissingUndefinedType (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
@@ -85,17 +82,17 @@ namespace ts.codefix {
8582
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, createUnionTypeNode(types));
8683
}
8784

88-
function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction | undefined {
85+
function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction | undefined {
8986
const checker = context.program.getTypeChecker();
9087
const initializer = getInitializer(checker, propertyDeclaration);
9188
if (!initializer) return undefined;
9289

9390
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_initializer_to_property_0), [propertyDeclaration.name.getText()]);
94-
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer, newLineCharacter));
91+
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer));
9592
return { description, changes, fixId: fixIdAddInitializer };
9693
}
9794

98-
function addInitializer (changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression, newLineCharacter: string): void {
95+
function addInitializer (changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression): void {
9996
const property = updateProperty(
10097
propertyDeclaration,
10198
propertyDeclaration.decorators,
@@ -105,7 +102,7 @@ namespace ts.codefix {
105102
propertyDeclaration.type,
106103
initializer
107104
);
108-
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property, { suffix: newLineCharacter });
105+
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
109106
}
110107

111108
function getInitializer(checker: TypeChecker, propertyDeclaration: PropertyDeclaration): Expression | undefined {

src/services/codefixes/fixUnusedIdentifier.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ namespace ts.codefix {
165165
// and trailing trivia will remain.
166166
suppressLeadingAndTrailingTrivia(newFunction);
167167

168-
changes.replaceNode(sourceFile, oldFunction, newFunction, textChanges.useNonAdjustedPositions);
168+
changes.replaceNode(sourceFile, oldFunction, newFunction);
169169
}
170170
else {
171171
changes.deleteNodeInList(sourceFile, parent);

src/services/codefixes/useDefaultImport.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,6 @@ namespace ts.codefix {
3838
}
3939

4040
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
41-
changes.replaceNode(sourceFile, info.importNode, makeImportDeclaration(info.name, /*namedImports*/ undefined, info.moduleSpecifier), textChanges.useNonAdjustedPositions);
41+
changes.replaceNode(sourceFile, info.importNode, makeImportDeclaration(info.name, /*namedImports*/ undefined, info.moduleSpecifier));
4242
}
4343
}

src/services/refactors/extractSymbol.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,7 @@ namespace ts.refactor.extractSymbol {
10531053
changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariable, /*blankLineBetween*/ true);
10541054

10551055
// Consume
1056-
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
1056+
changeTracker.replaceNode(context.file, node, localReference);
10571057
}
10581058
else {
10591059
const newVariableDeclaration = createVariableDeclaration(localNameText, variableType, initializer);
@@ -1070,15 +1070,15 @@ namespace ts.refactor.extractSymbol {
10701070

10711071
// Consume
10721072
const localReference = createIdentifier(localNameText);
1073-
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
1073+
changeTracker.replaceNode(context.file, node, localReference);
10741074
}
10751075
else if (node.parent.kind === SyntaxKind.ExpressionStatement && scope === findAncestor(node, isScope)) {
10761076
// If the parent is an expression statement and the target scope is the immediately enclosing one,
10771077
// replace the statement with the declaration.
10781078
const newVariableStatement = createVariableStatement(
10791079
/*modifiers*/ undefined,
10801080
createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const));
1081-
changeTracker.replaceNode(context.file, node.parent, newVariableStatement, textChanges.useNonAdjustedPositions);
1081+
changeTracker.replaceNode(context.file, node.parent, newVariableStatement);
10821082
}
10831083
else {
10841084
const newVariableStatement = createVariableStatement(
@@ -1101,7 +1101,7 @@ namespace ts.refactor.extractSymbol {
11011101
}
11021102
else {
11031103
const localReference = createIdentifier(localNameText);
1104-
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
1104+
changeTracker.replaceNode(context.file, node, localReference);
11051105
}
11061106
}
11071107
}

0 commit comments

Comments
 (0)