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
50 changes: 27 additions & 23 deletions src/services/codefixes/disableJsDiagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ 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())) {
if (!isInJavaScriptFile(sourceFile) || !isCheckJsEnabledForFile(sourceFile, program.getCompilerOptions()) || !isValidSuppressLocation(sourceFile, span.start)) {
return undefined;
}

const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
const newLineCharacter = getNewLineOrDefaultFromHost(host, formatContext.options);

return [{
description: getLocaleSpecificMessage(Diagnostics.Ignore_this_error_message),
changes: [createFileTextChanges(sourceFile.fileName, [getIgnoreCommentLocationForLocation(sourceFile, span.start, newLineCharacter).change])],
changes: textChanges.ChangeTracker.with(context, t => makeChange(t, sourceFile, span.start, newLineCharacter)),
fixId,
},
{
Expand All @@ -33,36 +33,40 @@ namespace ts.codefix {
},
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 newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
const seenLines = createMap<true>();
return codeFixAll(context, errorCodes, (changes, diag) => {
if (isValidSuppressLocation(diag.file!, diag.start!)) {
return makeChange(changes, diag.file!, diag.start!, newLineCharacter, 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, newLineCharacter: string, 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, { preseveLeadingWhiteSpaces: true, prefix: insertAtLineStart ? undefined : newLineCharacter });
}
}
6 changes: 5 additions & 1 deletion 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
*/
preseveLeadingWhiteSpaces?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preserveLeadingWhitespace

}

enum ChangeKind {
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.preseveLeadingWhiteSpaces || 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);