Fix ignore message indentation (#22340)
* Update baselines for user tests * Add explicit indentation * Fix https://github.com/Microsoft/TypeScript/issues/21355: Format `// @ts-ignore` added by quick fix * Extract check to a separate function * Consolidate checking for valid insert location * Code review comments * Do not return makeChange
This commit is contained in:
parent
2fb7e643f4
commit
b2dd610e92
5 changed files with 62 additions and 49 deletions
|
@ -9,60 +9,69 @@ namespace ts.codefix {
|
||||||
registerCodeFix({
|
registerCodeFix({
|
||||||
errorCodes,
|
errorCodes,
|
||||||
getCodeActions(context) {
|
getCodeActions(context) {
|
||||||
const { sourceFile, program, span } = context;
|
const { sourceFile, program, span, host, formatContext } = context;
|
||||||
|
|
||||||
if (!isInJavaScriptFile(sourceFile) || !isCheckJsEnabledForFile(sourceFile, program.getCompilerOptions())) {
|
if (!isInJavaScriptFile(sourceFile) || !isCheckJsEnabledForFile(sourceFile, program.getCompilerOptions())) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
|
const fixes: CodeFixAction[] = [
|
||||||
|
{
|
||||||
|
description: getLocaleSpecificMessage(Diagnostics.Disable_checking_for_this_file),
|
||||||
|
changes: [createFileTextChanges(sourceFile.fileName, [
|
||||||
|
createTextChange(sourceFile.checkJsDirective
|
||||||
|
? createTextSpanFromBounds(sourceFile.checkJsDirective.pos, sourceFile.checkJsDirective.end)
|
||||||
|
: createTextSpan(0, 0), `// @ts-nocheck${getNewLineOrDefaultFromHost(host, formatContext.options)}`),
|
||||||
|
])],
|
||||||
|
// fixId unnecessary because adding `// @ts-nocheck` even once will ignore every error in the file.
|
||||||
|
fixId: undefined,
|
||||||
|
}];
|
||||||
|
|
||||||
return [{
|
if (isValidSuppressLocation(sourceFile, span.start)) {
|
||||||
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
|
fixes.unshift({
|
||||||
changes: [createFileTextChanges(sourceFile.fileName, [getIgnoreCommentLocationForLocation(sourceFile, span.start, newLineCharacter).change])],
|
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
|
||||||
fixId,
|
changes: textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, span.start)),
|
||||||
},
|
fixId,
|
||||||
{
|
});
|
||||||
description: getLocaleSpecificMessage(Diagnostics.Disable_checking_for_this_file),
|
}
|
||||||
changes: [createFileTextChanges(sourceFile.fileName, [
|
|
||||||
createTextChange(sourceFile.checkJsDirective ? createTextSpanFromBounds(sourceFile.checkJsDirective.pos, sourceFile.checkJsDirective.end) : createTextSpan(0, 0), `// @ts-nocheck${newLineCharacter}`),
|
return fixes;
|
||||||
])],
|
|
||||||
// fixId unnecessary because adding `// @ts-nocheck` even once will ignore every error in the file.
|
|
||||||
fixId: undefined,
|
|
||||||
}];
|
|
||||||
},
|
},
|
||||||
fixIds: [fixId],
|
fixIds: [fixId],
|
||||||
getAllCodeActions: context => {
|
getAllCodeActions: context => {
|
||||||
const seenLines = createMap<true>(); // Only need to add `// @ts-ignore` for a line once.
|
const seenLines = createMap<true>();
|
||||||
return codeFixAllWithTextChanges(context, errorCodes, (changes, err) => {
|
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||||
if (err.start !== undefined) {
|
if (isValidSuppressLocation(diag.file!, diag.start!)) {
|
||||||
const { lineNumber, change } = getIgnoreCommentLocationForLocation(err.file!, err.start, getNewLineOrDefaultFromHost(context.host, context.formatContext.options));
|
makeChange(changes, diag.file!, diag.start!, seenLines);
|
||||||
if (addToSeen(seenLines, lineNumber)) {
|
|
||||||
changes.push(change);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
function getIgnoreCommentLocationForLocation(sourceFile: SourceFile, position: number, newLineCharacter: string): { lineNumber: number, change: TextChange } {
|
function isValidSuppressLocation(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);
|
const { line: lineNumber } = getLineAndCharacterOfPosition(sourceFile, position);
|
||||||
|
|
||||||
|
// Only need to add `// @ts-ignore` for a line once.
|
||||||
|
if (seenLines && !addToSeen(seenLines, lineNumber)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
|
const lineStartPosition = getStartPositionOfLine(lineNumber, sourceFile);
|
||||||
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
|
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);
|
||||||
|
|
||||||
// First try to see if we can put the '// @ts-ignore' on the previous line.
|
// 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.
|
// We need to make sure that we are not in the middle of a string literal or a comment.
|
||||||
// We also want to check if the previous line holds a comment for a node on the next line
|
// If so, we do not want to separate the node from its comment if we can.
|
||||||
// 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.
|
||||||
if (!isInComment(sourceFile, startPosition) && !isInString(sourceFile, startPosition) && !isInTemplateString(sourceFile, startPosition)) {
|
const insertAtLineStart = isValidSuppressLocation(sourceFile, startPosition);
|
||||||
const token = getTouchingToken(sourceFile, startPosition, /*includeJsDocComment*/ false);
|
|
||||||
const tokenLeadingComments = getLeadingCommentRangesOfNode(token, sourceFile);
|
|
||||||
if (!tokenLeadingComments || !tokenLeadingComments.length || tokenLeadingComments[0].pos >= startPosition) {
|
|
||||||
return { lineNumber, change: createTextChangeFromStartLength(startPosition, 0, `// @ts-ignore${newLineCharacter}`) };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// If all fails, add an extra new line immediately before the error span.
|
const token = getTouchingToken(sourceFile, insertAtLineStart ? startPosition : position, /*includeJsDocComment*/ false);
|
||||||
return { lineNumber, change: createTextChangeFromStartLength(position, 0, `${position === startPosition ? "" : newLineCharacter}// @ts-ignore${newLineCharacter}`) };
|
const clone = setStartsOnNewLine(getSynthesizedDeepClone(token), true);
|
||||||
|
addSyntheticLeadingComment(clone, SyntaxKind.SingleLineCommentTrivia, " @ts-ignore");
|
||||||
|
changes.replaceNode(sourceFile, token, clone, { preserveLeadingWhitespace: true, prefix: insertAtLineStart ? undefined : changes.newLineCharacter });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -94,6 +94,10 @@ namespace ts.textChanges {
|
||||||
* Text of inserted node will be formatted with this delta, otherwise delta will be inferred from the new node kind
|
* Text of inserted node will be formatted with this delta, otherwise delta will be inferred from the new node kind
|
||||||
*/
|
*/
|
||||||
delta?: number;
|
delta?: number;
|
||||||
|
/**
|
||||||
|
* Do not trim leading white spaces in the edit range
|
||||||
|
*/
|
||||||
|
preserveLeadingWhitespace?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
enum ChangeKind {
|
enum ChangeKind {
|
||||||
|
@ -212,7 +216,7 @@ namespace ts.textChanges {
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Public for tests only. Other callers should use `ChangeTracker.with`. */
|
/** Public for tests only. Other callers should use `ChangeTracker.with`. */
|
||||||
constructor(private readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}
|
constructor(public readonly newLineCharacter: string, private readonly formatContext: formatting.FormatContext) {}
|
||||||
|
|
||||||
public deleteRange(sourceFile: SourceFile, range: TextRange) {
|
public deleteRange(sourceFile: SourceFile, range: TextRange) {
|
||||||
this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
|
this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
|
||||||
|
@ -628,7 +632,7 @@ namespace ts.textChanges {
|
||||||
? change.nodes.map(n => removeSuffix(format(n), newLineCharacter)).join(newLineCharacter)
|
? change.nodes.map(n => removeSuffix(format(n), newLineCharacter)).join(newLineCharacter)
|
||||||
: format(change.node);
|
: format(change.node);
|
||||||
// strip initial indentation (spaces or tabs) if text will be inserted in the middle of the line
|
// strip initial indentation (spaces or tabs) if text will be inserted in the middle of the line
|
||||||
const noIndent = (options.indentation !== undefined || getLineStartPositionForPosition(pos, sourceFile) === pos) ? text : text.replace(/^\s+/, "");
|
const noIndent = (options.preserveLeadingWhitespace || options.indentation !== undefined || getLineStartPositionForPosition(pos, sourceFile) === pos) ? text : text.replace(/^\s+/, "");
|
||||||
return (options.prefix || "") + noIndent + (options.suffix || "");
|
return (options.prefix || "") + noIndent + (options.suffix || "");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -7,11 +7,11 @@
|
||||||
// @Filename: a.js
|
// @Filename: a.js
|
||||||
////var x = 0;
|
////var x = 0;
|
||||||
////
|
////
|
||||||
////function f(_a) {
|
////function f(_a) {[|
|
||||||
//// [|f(x());|]
|
//// f(x());
|
||||||
////}
|
////|]}
|
||||||
|
|
||||||
// Disable checking for next line
|
// Disable checking for next line
|
||||||
verify.rangeAfterCodeFix(`// @ts-ignore
|
verify.rangeAfterCodeFix(` // @ts-ignore
|
||||||
f(x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
f(x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
||||||
|
|
||||||
|
|
|
@ -7,11 +7,11 @@
|
||||||
// @Filename: a.js
|
// @Filename: a.js
|
||||||
////var x = 0;
|
////var x = 0;
|
||||||
////
|
////
|
||||||
////function f(_a) {
|
////function f(_a) {[|
|
||||||
//// [|x();|]
|
//// x();
|
||||||
////}
|
////|]}
|
||||||
|
|
||||||
// Disable checking for next line
|
// Disable checking for next line
|
||||||
verify.rangeAfterCodeFix(`// @ts-ignore
|
verify.rangeAfterCodeFix(`// @ts-ignore
|
||||||
x();`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
x();`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
||||||
|
|
||||||
|
|
|
@ -7,13 +7,13 @@
|
||||||
// @Filename: a.js
|
// @Filename: a.js
|
||||||
////var x = 0;
|
////var x = 0;
|
||||||
////
|
////
|
||||||
////function f(_a) {
|
////function f(_a) {[|
|
||||||
//// /** comment for f */
|
//// /** comment for f */
|
||||||
//// [|f(x());|]
|
//// f(x());
|
||||||
////}
|
////|]}
|
||||||
|
|
||||||
// Disable checking for next line
|
// Disable checking for next line
|
||||||
verify.rangeAfterCodeFix(`f(
|
verify.rangeAfterCodeFix(` /** comment for f */
|
||||||
// @ts-ignore
|
// @ts-ignore
|
||||||
x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
f(x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue