microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-14 02:15:12 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

replaceNode: Always use non-adjusted end (#22519)

Browse Source 
* 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
This commit is contained in:
Andy 2018-03-27 10:34:16 -07:00 committed by GitHub
parent 4fa96056ea
commit ea6740fa91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 70 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/services/codefixes/convertToEs6Module.ts
View File

@ -257,7 +257,7 @@ namespace ts.codefix {
changes.replaceNodeWithNodes(sourceFile, statement, newNodes);
}
else {
changes.replaceNode(sourceFile, statement, convertExportsDotXEquals(text, right), { useNonAdjustedEndPosition: true });
changes.replaceNode(sourceFile, statement, convertExportsDotXEquals(text, right));
}
}

25
src/services/codefixes/disableJsDiagnostics.ts
View File

@ -27,7 +27,7 @@ namespace ts.codefix {
fixId: undefined,
}];
if (isValidSuppressLocation(sourceFile, span.start)) {
if (isValidLocationToAddComment(sourceFile, span.start)) {
fixes.unshift({
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
changes: textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, span.start)),
@ -41,37 +41,22 @@ namespace ts.codefix {
getAllCodeActions: context => {
const seenLines = createMap<true>();
return codeFixAll(context, errorCodes, (changes, diag) => {
if (isValidSuppressLocation(diag.file!, diag.start!)) {
if (isValidLocationToAddComment(diag.file!, diag.start!)) {
makeChange(changes, diag.file!, diag.start!, seenLines);
}
});
},
});
function isValidSuppressLocation(sourceFile: SourceFile, position: number) {
export function isValidLocationToAddComment(sourceFile: SourceFile, position: number) {
return !isInComment(sourceFile, position) && !isInString(sourceFile, position) && !isInTemplateString(sourceFile, position);
}
function makeChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, seenLines?: Map<true>) {
const { line: lineNumber } = getLineAndCharacterOfPosition(sourceFile, position);
// Only need to add `// @ts-ignore` for a line once.
if (seenLines && !addToSeen(seenLines, lineNumber)) {
return;
if (!seenLines || addToSeen(seenLines, lineNumber)) {
changes.insertCommentBeforeLine(sourceFile, lineNumber, position, " @ts-ignore");
}
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
// First try to see if we can put the '// @ts-ignore' on the previous line.
// We need to make sure that we are not in the middle of a string literal or a comment.
// If so, we do not want to separate the node from its comment if we can.
// Otherwise, add an extra new line immediately before the error span.
const insertAtLineStart = isValidSuppressLocation(sourceFile, startPosition);
const token = getTouchingToken(sourceFile, insertAtLineStart ? startPosition : position, /*includeJsDocComment*/ false);
const clone = setStartsOnNewLine(getSynthesizedDeepClone(token), true);
addSyntheticLeadingComment(clone, SyntaxKind.SingleLineCommentTrivia, " @ts-ignore");
changes.replaceNode(sourceFile, token, clone, { preserveLeadingWhitespace: true, prefix: insertAtLineStart ? undefined : changes.newLineCharacter });
}
}

2
src/services/codefixes/fixExtendsInterfaceBecomesImplements.ts
View File

@ -27,7 +27,7 @@ namespace ts.codefix {
}
function doChanges(changes: textChanges.ChangeTracker, sourceFile: SourceFile, extendsToken: Node, heritageClauses: ReadonlyArray<HeritageClause>): void {
changes.replaceNode(sourceFile, extendsToken, createToken(SyntaxKind.ImplementsKeyword), textChanges.useNonAdjustedPositions);
changes.replaceNode(sourceFile, extendsToken, createToken(SyntaxKind.ImplementsKeyword));
// If there is already an implements clause, replace the implements keyword with a comma.
if (heritageClauses.length === 2 &&

2
src/services/codefixes/fixForgottenThisPropertyAccess.ts
View File

@ -30,6 +30,6 @@ namespace ts.codefix {
}
// TODO (https://github.com/Microsoft/TypeScript/issues/21246): use shared helper
suppressLeadingAndTrailingTrivia(token);
changes.replaceNode(sourceFile, token, createPropertyAccess(createThis(), token), textChanges.useNonAdjustedPositions);
changes.replaceNode(sourceFile, token, createPropertyAccess(createThis(), token));
}
}

3
src/services/codefixes/fixInvalidImportSyntax.ts
View File

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

27
src/services/codefixes/fixStrictClassInitialization.ts
View File

@ -10,27 +10,24 @@ namespace ts.codefix {
const propertyDeclaration = getPropertyDeclaration(context.sourceFile, context.span.start);
if (!propertyDeclaration) return;
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
const result = [
getActionForAddMissingUndefinedType(context, propertyDeclaration),
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration, newLineCharacter)
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration)
];
append(result, getActionForAddMissingInitializer(context, propertyDeclaration, newLineCharacter));
append(result, getActionForAddMissingInitializer(context, propertyDeclaration));
return result;
},
fixIds: [fixIdAddDefiniteAssignmentAssertions, fixIdAddUndefinedType, fixIdAddInitializer],
getAllCodeActions: context => {
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
return codeFixAll(context, errorCodes, (changes, diag) => {
const propertyDeclaration = getPropertyDeclaration(diag.file, diag.start);
if (!propertyDeclaration) return;
switch (context.fixId) {
case fixIdAddDefiniteAssignmentAssertions:
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration, newLineCharacter);
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration);
break;
case fixIdAddUndefinedType:
addUndefinedType(changes, diag.file, propertyDeclaration);
@ -40,7 +37,7 @@ namespace ts.codefix {
const initializer = getInitializer(checker, propertyDeclaration);
if (!initializer) return;
addInitializer(changes, diag.file, propertyDeclaration, initializer, newLineCharacter);
addInitializer(changes, diag.file, propertyDeclaration, initializer);
break;
default:
Debug.fail(JSON.stringify(context.fixId));
@ -54,13 +51,13 @@ namespace ts.codefix {
return isIdentifier(token) ? cast(token.parent, isPropertyDeclaration) : undefined;
}
function getActionForAddMissingDefiniteAssignmentAssertion (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction {
function getActionForAddMissingDefiniteAssignmentAssertion (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_definite_assignment_assertion_to_property_0), [propertyDeclaration.getText()]);
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration, newLineCharacter));
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration));
return { description, changes, fixId: fixIdAddDefiniteAssignmentAssertions };
}
function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): void {
function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
const property = updateProperty(
propertyDeclaration,
propertyDeclaration.decorators,
@ -70,7 +67,7 @@ namespace ts.codefix {
propertyDeclaration.type,
propertyDeclaration.initializer
);
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property, { suffix: newLineCharacter });
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
}
function getActionForAddMissingUndefinedType (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
@ -85,17 +82,17 @@ namespace ts.codefix {
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, createUnionTypeNode(types));
}
function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction | undefined {
function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction | undefined {
const checker = context.program.getTypeChecker();
const initializer = getInitializer(checker, propertyDeclaration);
if (!initializer) return undefined;
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_initializer_to_property_0), [propertyDeclaration.name.getText()]);
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer, newLineCharacter));
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer));
return { description, changes, fixId: fixIdAddInitializer };
}
function addInitializer (changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression, newLineCharacter: string): void {
function addInitializer (changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression): void {
const property = updateProperty(
propertyDeclaration,
propertyDeclaration.decorators,
@ -105,7 +102,7 @@ namespace ts.codefix {
propertyDeclaration.type,
initializer
);
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property, { suffix: newLineCharacter });
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
}
function getInitializer(checker: TypeChecker, propertyDeclaration: PropertyDeclaration): Expression | undefined {

2
src/services/codefixes/fixUnusedIdentifier.ts
View File

@ -165,7 +165,7 @@ namespace ts.codefix {
// and trailing trivia will remain.
suppressLeadingAndTrailingTrivia(newFunction);
changes.replaceNode(sourceFile, oldFunction, newFunction, textChanges.useNonAdjustedPositions);
changes.replaceNode(sourceFile, oldFunction, newFunction);
}
else {
changes.deleteNodeInList(sourceFile, parent);

2
src/services/codefixes/useDefaultImport.ts
View File

@ -38,6 +38,6 @@ namespace ts.codefix {
}
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
changes.replaceNode(sourceFile, info.importNode, makeImportDeclaration(info.name, /*namedImports*/ undefined, info.moduleSpecifier), textChanges.useNonAdjustedPositions);
changes.replaceNode(sourceFile, info.importNode, makeImportDeclaration(info.name, /*namedImports*/ undefined, info.moduleSpecifier));
}
}

8
src/services/refactors/extractSymbol.ts
View File

@ -1053,7 +1053,7 @@ namespace ts.refactor.extractSymbol {
changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariable, /*blankLineBetween*/ true);
// Consume
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
changeTracker.replaceNode(context.file, node, localReference);
}
else {
const newVariableDeclaration = createVariableDeclaration(localNameText, variableType, initializer);
@ -1070,7 +1070,7 @@ namespace ts.refactor.extractSymbol {
// Consume
const localReference = createIdentifier(localNameText);
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
changeTracker.replaceNode(context.file, node, localReference);
}
else if (node.parent.kind === SyntaxKind.ExpressionStatement && scope === findAncestor(node, isScope)) {
// If the parent is an expression statement and the target scope is the immediately enclosing one,
@ -1078,7 +1078,7 @@ namespace ts.refactor.extractSymbol {
const newVariableStatement = createVariableStatement(
/*modifiers*/ undefined,
createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const));
changeTracker.replaceNode(context.file, node.parent, newVariableStatement, textChanges.useNonAdjustedPositions);
changeTracker.replaceNode(context.file, node.parent, newVariableStatement);
}
else {
const newVariableStatement = createVariableStatement(
@ -1101,7 +1101,7 @@ namespace ts.refactor.extractSymbol {
}
else {
const localReference = createIdentifier(localNameText);
changeTracker.replaceNode(context.file, node, localReference, textChanges.useNonAdjustedPositions);
changeTracker.replaceNode(context.file, node, localReference);
}
}
}

67
src/services/textChanges.ts
View File

@ -103,10 +103,11 @@ namespace ts.textChanges {
enum ChangeKind {
Remove,
ReplaceWithSingleNode,
ReplaceWithMultipleNodes
ReplaceWithMultipleNodes,
Text,
}
type Change = ReplaceWithSingleNode | ReplaceWithMultipleNodes | RemoveNode;
type Change = ReplaceWithSingleNode | ReplaceWithMultipleNodes | RemoveNode | ChangeText;
interface BaseChange {
readonly sourceFile: SourceFile;
@ -132,6 +133,15 @@ namespace ts.textChanges {
readonly options?: InsertNodeOptions;
}
interface ChangeText extends BaseChange {
readonly kind: ChangeKind.Text;
readonly text: string;
}
function getAdjustedRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options: ConfigurableStartEnd): TextRange {
return { pos: getAdjustedStartPosition(sourceFile, startNode, options, Position.Start), end: getAdjustedEndPosition(sourceFile, endNode, options) };
}
function getAdjustedStartPosition(sourceFile: SourceFile, node: Node, options: ConfigurableStart, position: Position) {
if (options.useNonAdjustedStartPosition) {
return node.getStart(sourceFile);
@ -212,7 +222,7 @@ namespace ts.textChanges {
}
/** Public for tests only. Other callers should use `ChangeTracker.with`. */
constructor(public readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}
constructor(private readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}
public deleteRange(sourceFile: SourceFile, range: TextRange) {
this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
@ -280,41 +290,30 @@ namespace ts.textChanges {
return this;
}
// TODO (https://github.com/Microsoft/TypeScript/issues/21246): default should probably be useNonAdjustedPositions
public replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options: InsertNodeOptions = {}) {
this.changes.push({ kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range, options, node: newNode });
return this;
}
// TODO (https://github.com/Microsoft/TypeScript/issues/21246): default should probably be useNonAdjustedPositions
public replaceNode(sourceFile: SourceFile, oldNode: Node, newNode: Node, options: ChangeNodeOptions = {}) {
const pos = getAdjustedStartPosition(sourceFile, oldNode, options, Position.Start);
const end = getAdjustedEndPosition(sourceFile, oldNode, options);
return this.replaceRange(sourceFile, { pos, end }, newNode, options);
public replaceNode(sourceFile: SourceFile, oldNode: Node, newNode: Node, options: ChangeNodeOptions = useNonAdjustedPositions) {
return this.replaceRange(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, options), newNode, options);
}
// TODO (https://github.com/Microsoft/TypeScript/issues/21246): default should probably be useNonAdjustedPositions
public replaceNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, newNode: Node, options: ChangeNodeOptions = {}) {
const pos = getAdjustedStartPosition(sourceFile, startNode, options, Position.Start);
const end = getAdjustedEndPosition(sourceFile, endNode, options);
return this.replaceRange(sourceFile, { pos, end }, newNode, options);
public replaceNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, newNode: Node, options: ChangeNodeOptions = useNonAdjustedPositions) {
this.replaceRange(sourceFile, getAdjustedRange(sourceFile, startNode, endNode, options), newNode, options);
}
public replaceRangeWithNodes(sourceFile: SourceFile, range: TextRange, newNodes: ReadonlyArray<Node>, options: InsertNodeOptions = {}) {
private replaceRangeWithNodes(sourceFile: SourceFile, range: TextRange, newNodes: ReadonlyArray<Node>, options: InsertNodeOptions = {}) {
this.changes.push({ kind: ChangeKind.ReplaceWithMultipleNodes, sourceFile, range, options, nodes: newNodes });
return this;
}
public replaceNodeWithNodes(sourceFile: SourceFile, oldNode: Node, newNodes: ReadonlyArray<Node>, options: ChangeNodeOptions = useNonAdjustedPositions) {
const pos = getAdjustedStartPosition(sourceFile, oldNode, options, Position.Start);
const end = getAdjustedEndPosition(sourceFile, oldNode, options);
return this.replaceRangeWithNodes(sourceFile, { pos, end }, newNodes, options);
return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, options), newNodes, options);
}
public replaceNodeRangeWithNodes(sourceFile: SourceFile, startNode: Node, endNode: Node, newNodes: ReadonlyArray<Node>, options: ChangeNodeOptions = useNonAdjustedPositions) {
const pos = getAdjustedStartPosition(sourceFile, startNode, options, Position.Start);
const end = getAdjustedEndPosition(sourceFile, endNode, options);
return this.replaceRangeWithNodes(sourceFile, { pos, end }, newNodes, options);
return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, startNode, endNode, options), newNodes, options);
}
private insertNodeAt(sourceFile: SourceFile, pos: number, newNode: Node, options: InsertNodeOptions = {}) {
@ -344,6 +343,23 @@ namespace ts.textChanges {
this.replaceRange(sourceFile, { pos, end: pos }, createToken(modifier), { suffix: " " });
}
public insertCommentBeforeLine(sourceFile: SourceFile, lineNumber: number, position: number, commentText: string): void {
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
// First try to see if we can put the comment on the previous line.
// We need to make sure that we are not in the middle of a string literal or a comment.
// If so, we do not want to separate the node from its comment if we can.
// Otherwise, add an extra new line immediately before the error span.
const insertAtLineStart = codefix.isValidLocationToAddComment(sourceFile, startPosition);
const token = getTouchingToken(sourceFile, insertAtLineStart ? startPosition : position, /*includeJsDocComment*/ false);
const text = `${insertAtLineStart ? "" : this.newLineCharacter}${sourceFile.text.slice(lineStartPosition, startPosition)}//${commentText}${this.newLineCharacter}`;
this.insertText(sourceFile, token.getStart(sourceFile), text);
}
private insertText(sourceFile: SourceFile, pos: number, text: string): void {
this.changes.push({ kind: ChangeKind.Text, sourceFile, range: { pos, end: pos }, text });
}
/** Prefer this over replacing a node with another that has a type annotation, as it avoids reformatting the other parts of the node. */
public insertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void {
const end = (isFunctionLike(node)
@ -393,7 +409,7 @@ namespace ts.textChanges {
}
private replaceConstructorBody(sourceFile: SourceFile, ctr: ConstructorDeclaration, statements: ReadonlyArray<Statement>): void {
this.replaceNode(sourceFile, ctr.body, createBlock(statements, /*multiLine*/ true), { useNonAdjustedEndPosition: true });
this.replaceNode(sourceFile, ctr.body, createBlock(statements, /*multiLine*/ true));
}
public insertNodeAtEndOfScope(sourceFile: SourceFile, scope: Node, newNode: Node): void {
@ -608,7 +624,7 @@ namespace ts.textChanges {
const newCls = cls.kind === SyntaxKind.ClassDeclaration
? updateClassDeclaration(cls, cls.decorators, cls.modifiers, cls.name, cls.typeParameters, cls.heritageClauses, members)
: updateClassExpression(cls, cls.modifiers, cls.name, cls.typeParameters, cls.heritageClauses, members);
this.replaceNode(sourceFile, cls, newCls, { useNonAdjustedEndPosition: true });
this.replaceNode(sourceFile, cls, newCls);
});
}
@ -647,6 +663,9 @@ namespace ts.textChanges {
if (change.kind === ChangeKind.Remove) {
return "";
}
if (change.kind === ChangeKind.Text) {
return change.text;
}
const { options = {}, range: { pos } } = change;
const format = (n: Node) => getFormattedTextOfNode(n, sourceFile, pos, options, newLineCharacter, formatContext, validate);
@ -659,7 +678,7 @@ namespace ts.textChanges {
}
/** Note: this may mutate `nodeIn`. */
function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, options: ChangeNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText): string {
function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, options: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText): string {
const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter);
if (validate) validate(node, text);
const { options: formatOptions } = formatContext;