Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ignore message indentation #22340

Merged
merged 9 commits into from
Mar 6, 2018
77 changes: 43 additions & 34 deletions src/services/codefixes/disableJsDiagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,60 +9,69 @@ namespace ts.codefix {
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { sourceFile, program, span } = context;
const { sourceFile, program, span, host, formatContext } = context;

if (!isInJavaScriptFile(sourceFile) || !isCheckJsEnabledForFile(sourceFile, program.getCompilerOptions())) {
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 [{
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
changes: [createFileTextChanges(sourceFile.fileName, [getIgnoreCommentLocationForLocation(sourceFile, span.start, newLineCharacter).change])],
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}`),
])],
// fixId unnecessary because adding `// @ts-nocheck` even once will ignore every error in the file.
fixId: undefined,
}];
if (isValidSuppressLocation(sourceFile, span.start)) {
fixes.unshift({
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
changes: textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, span.start)),
fixId,
});
}

return fixes;
},
fixIds: [fixId],
getAllCodeActions: context => {
const seenLines = createMap<true>(); // Only need to add `// @ts-ignore` for a line once.
return codeFixAllWithTextChanges(context, errorCodes, (changes, err) => {
if (err.start !== undefined) {
const { lineNumber, change } = getIgnoreCommentLocationForLocation(err.file!, err.start, getNewLineOrDefaultFromHost(context.host, context.formatContext.options));
if (addToSeen(seenLines, lineNumber)) {
changes.push(change);
}
const seenLines = createMap<true>();
return codeFixAll(context, errorCodes, (changes, diag) => {
if (isValidSuppressLocation(diag.file!, diag.start!)) {
makeChange(changes, diag.file!, diag.start!, seenLines);
}
});
},
});

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);

// Only need to add `// @ts-ignore` for a line once.
if (seenLines && !addToSeen(seenLines, lineNumber)) {
return;
}

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.
// 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 (!isInComment(sourceFile, startPosition) && !isInString(sourceFile, startPosition) && !isInTemplateString(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 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);

// If all fails, add an extra new line immediately before the error span.
return { lineNumber, change: createTextChangeFromStartLength(position, 0, `${position === startPosition ? "" : newLineCharacter}// @ts-ignore${newLineCharacter}`) };
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 });
}
}
8 changes: 6 additions & 2 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
delta?: number;
/**
* Do not trim leading white spaces in the edit range
*/
preserveLeadingWhitespace?: boolean;
}

enum ChangeKind {
Expand Down Expand Up @@ -212,7 +216,7 @@ namespace ts.textChanges {
}

/** 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) {
this.changes.push({ kind: ChangeKind.Remove, sourceFile, range });
Expand Down Expand Up @@ -628,7 +632,7 @@ namespace ts.textChanges {
? change.nodes.map(n => removeSuffix(format(n), newLineCharacter)).join(newLineCharacter)
: format(change.node);
// 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 || "");
}

Expand Down
8 changes: 4 additions & 4 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile6.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
// @Filename: a.js
////var x = 0;
////
////function f(_a) {
//// [|f(x());|]
////}
////function f(_a) {[|
//// f(x());
////|]}

// Disable checking for next line
verify.rangeAfterCodeFix(`// @ts-ignore
verify.rangeAfterCodeFix(` // @ts-ignore
f(x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);

8 changes: 4 additions & 4 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
// @Filename: a.js
////var x = 0;
////
////function f(_a) {
//// [|x();|]
////}
////function f(_a) {[|
//// x();
////|]}

// Disable checking for next line
verify.rangeAfterCodeFix(`// @ts-ignore
verify.rangeAfterCodeFix(`// @ts-ignore
x();`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);

10 changes: 5 additions & 5 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile8.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
// @Filename: a.js
////var x = 0;
////
////function f(_a) {
////function f(_a) {[|
//// /** comment for f */
//// [|f(x());|]
////}
//// f(x());
////|]}

// Disable checking for next line
verify.rangeAfterCodeFix(`f(
verify.rangeAfterCodeFix(` /** comment for f */
// @ts-ignore
x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
f(x());`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);