From c0e922e10ab9184756eadce61d4eaff90bbb7c4b Mon Sep 17 00:00:00 2001 From: mizdra Date: Sat, 28 Sep 2024 12:34:28 +0900 Subject: [PATCH 1/5] add a test case for adding a disable comment in template literal --- src/fix/disable-per-line.test.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/fix/disable-per-line.test.ts b/src/fix/disable-per-line.test.ts index d4161602..6d8c5133 100644 --- a/src/fix/disable-per-line.test.ts +++ b/src/fix/disable-per-line.test.ts @@ -140,6 +140,35 @@ describe('disable-per-line', () => { var val3" `); }); + test('add a disable comment in template literal', async () => { + expect( + await tester.test({ + code: [ + 'const foo = `', + ' This is a template literal', + // eslint-disable-next-line no-template-curly-in-string + ' ${void 1}', + '`;', + ], + rules: { 'no-void': 'error' }, + }), + ).toMatchInlineSnapshot(` + "const foo = \` + This is a template literal + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + // eslint-disable-next-line no-void + \${void 1} + \`;" + `); + }); describe('add a disable comment for JSX', () => { test('when descriptionPosition is sameLine', async () => { expect( From 59bc3a439c57c1c2789d2bd164684a5e36926079 Mon Sep 17 00:00:00 2001 From: mizdra Date: Sat, 28 Sep 2024 13:10:25 +0900 Subject: [PATCH 2/5] refactor: add column arg to `insertDescriptionCommentStatementBeforeLine` and `insertDisableCommentStatementBeforeLine` --- .eslintrc.cjs | 2 ++ src/fix/disable-per-file.ts | 2 ++ src/fix/disable-per-line.ts | 5 ++++- src/util/eslint.ts | 10 ++++++---- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 93fea2ae..7e4ab001 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -42,6 +42,8 @@ module.exports = { ], }, ], + // 煩い上にこれに怒られてもリファクタリングしようという気持ちにならないので off + 'max-params': 'off', }, overrides: [ // for typescript diff --git a/src/fix/disable-per-file.ts b/src/fix/disable-per-file.ts index 0a60aca8..c0a4e4ac 100644 --- a/src/fix/disable-per-file.ts +++ b/src/fix/disable-per-file.ts @@ -54,6 +54,7 @@ function generateFix( fixer, sourceCode, line: lineToInsert, + column: 0, description, }), ); @@ -76,6 +77,7 @@ function generateFix( fixer, sourceCode, line: lineToInsert, + column: 0, scope: 'file', ruleIds: ruleIdsToDisable, description: isPreviousLine ? undefined : description, diff --git a/src/fix/disable-per-line.ts b/src/fix/disable-per-line.ts index 134490d2..e171de5e 100644 --- a/src/fix/disable-per-line.ts +++ b/src/fix/disable-per-line.ts @@ -30,6 +30,7 @@ function generateFixesPerLine( description: string | undefined, descriptionPosition: DescriptionPosition | undefined, line: number, + column: number, messagesInLine: Linter.LintMessage[], ): Rule.Fix | null { const { fixer, sourceCode } = context; @@ -48,6 +49,7 @@ function generateFixesPerLine( fixer, sourceCode, line: disableCommentPerLine ? disableCommentPerLine.loc.start.line : line, + column, description, }), ); @@ -69,6 +71,7 @@ function generateFixesPerLine( fixer, sourceCode, line, + column, scope: 'next-line', ruleIds: ruleIdsToDisable, description: isPreviousLine ? undefined : description, @@ -85,7 +88,7 @@ export function createFixToDisablePerLine(context: FixContext, args: FixToDisabl const lineToMessages = groupBy(context.messages, (message) => message.line); const fixes: Rule.Fix[] = []; for (const [line, messagesInLine] of lineToMessages) { - const fix = generateFixesPerLine(context, args.description, args.descriptionPosition, line, messagesInLine); + const fix = generateFixesPerLine(context, args.description, args.descriptionPosition, line, 0, messagesInLine); if (fix) fixes.push(fix); } return fixes; diff --git a/src/util/eslint.ts b/src/util/eslint.ts index cbcb18a2..ec09ea31 100644 --- a/src/util/eslint.ts +++ b/src/util/eslint.ts @@ -136,11 +136,12 @@ export function insertDescriptionCommentStatementBeforeLine(args: { fixer: Rule.RuleFixer; sourceCode: SourceCode; line: number; + column: number; description: string; }): Rule.Fix { - const { fixer, sourceCode, line, description } = args; + const { fixer, sourceCode, line, column, description } = args; const indent = getIndentFromLine(sourceCode, line); - const headNodeIndex = sourceCode.getIndexFromLoc({ line, column: 0 }); + const headNodeIndex = sourceCode.getIndexFromLoc({ line, column }); if (isLineInJSXText(sourceCode, line)) { const commentText = toCommentText({ type: 'Block', text: description }); @@ -175,13 +176,14 @@ export function insertDisableCommentStatementBeforeLine(args: { fixer: Rule.RuleFixer; sourceCode: SourceCode; line: number; + column: number; scope: 'file' | 'next-line'; ruleIds: string[]; description: string | undefined; }) { - const { fixer, sourceCode, line, scope, ruleIds, description } = args; + const { fixer, sourceCode, line, column, scope, ruleIds, description } = args; const indent = getIndentFromLine(sourceCode, line); - const headNodeIndex = sourceCode.getIndexFromLoc({ line, column: 0 }); + const headNodeIndex = sourceCode.getIndexFromLoc({ line, column }); const isInJSXText = isLineInJSXText(sourceCode, line); const type = isInJSXText || scope === 'file' ? 'Block' : 'Line'; const disableCommentText = toDisableCommentText({ From c806068e1d5bcf6549dcc6f26d04a99f53479cd8 Mon Sep 17 00:00:00 2001 From: mizdra Date: Sat, 28 Sep 2024 14:07:27 +0900 Subject: [PATCH 3/5] fix: fix adding a disable comment in template literal --- src/fix/disable-per-line.test.ts | 13 ++----------- src/fix/disable-per-line.ts | 16 +++++++++++++--- src/util/eslint.ts | 21 +++++++++++++++++++++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/fix/disable-per-line.test.ts b/src/fix/disable-per-line.test.ts index 6d8c5133..406c9dce 100644 --- a/src/fix/disable-per-line.test.ts +++ b/src/fix/disable-per-line.test.ts @@ -155,17 +155,8 @@ describe('disable-per-line', () => { ).toMatchInlineSnapshot(` "const foo = \` This is a template literal - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - // eslint-disable-next-line no-void - \${void 1} + \${ // eslint-disable-next-line no-void + void 1} \`;" `); }); diff --git a/src/fix/disable-per-line.ts b/src/fix/disable-per-line.ts index e171de5e..2ac3c464 100644 --- a/src/fix/disable-per-line.ts +++ b/src/fix/disable-per-line.ts @@ -4,8 +4,10 @@ import { mergeFixes } from '../eslint/report-translator.js'; import { groupBy, unique } from '../util/array.js'; import { DisableComment, + getStartColumnOfTemplateExpression, insertDescriptionCommentStatementBeforeLine, insertDisableCommentStatementBeforeLine, + isLineInTemplateLiteral, mergeDescription, mergeRuleIds, parseDisableComment, @@ -85,10 +87,18 @@ function generateFixesPerLine( * Create fix to add disable comment per line. */ export function createFixToDisablePerLine(context: FixContext, args: FixToDisablePerLineArgs): Rule.Fix[] { - const lineToMessages = groupBy(context.messages, (message) => message.line); + const groupedMessages = groupBy(context.messages, (message) => { + if (isLineInTemplateLiteral(context.sourceCode, message.line)) { + const column = getStartColumnOfTemplateExpression(context.sourceCode, message); + return `${message.line}:${column}`; + } else { + return `${message.line}:0`; + } + }); const fixes: Rule.Fix[] = []; - for (const [line, messagesInLine] of lineToMessages) { - const fix = generateFixesPerLine(context, args.description, args.descriptionPosition, line, 0, messagesInLine); + for (const [lineAndColumn, messagesInLine] of groupedMessages) { + const [line, column] = lineAndColumn.split(':').map(Number) as [number, number]; + const fix = generateFixesPerLine(context, args.description, args.descriptionPosition, line, column, messagesInLine); if (fix) fixes.push(fix); } return fixes; diff --git a/src/util/eslint.ts b/src/util/eslint.ts index ec09ea31..8662d30d 100644 --- a/src/util/eslint.ts +++ b/src/util/eslint.ts @@ -109,6 +109,27 @@ function isLineInJSXText(sourceCode: SourceCode, line: number): boolean { return headNode?.type === 'JSXText'; } +export function isLineInTemplateLiteral(sourceCode: SourceCode, line: number): boolean { + const headNodeIndex = sourceCode.getIndexFromLoc({ line, column: 0 }); + const headNode = sourceCode.getNodeByRangeIndex(headNodeIndex); + return headNode?.type === 'TemplateElement'; +} + +export function getStartColumnOfTemplateExpression(sourceCode: SourceCode, message: Linter.LintMessage): number { + for (let i = message.column; i >= 1; i--) { + const index = sourceCode.getIndexFromLoc({ + line: message.line, + // Convert 1-indexed to 0-indexed + column: i - 1, + }); + const node = sourceCode.getNodeByRangeIndex(index); + if (node?.type === 'TemplateElement') { + return i; + } + } + throw new Error(`unreachable: The line ${message.line} does not have a template element.`); +} + /** * Merge the ruleIds of the disable comments. * @param a The ruleIds of first disable comment From 59fa9c95e1b92e72ce29d28dd46e994d68e19e39 Mon Sep 17 00:00:00 2001 From: mizdra Date: Sat, 28 Sep 2024 14:13:18 +0900 Subject: [PATCH 4/5] improve test cases --- src/fix/disable-per-line.test.ts | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/fix/disable-per-line.test.ts b/src/fix/disable-per-line.test.ts index 406c9dce..1de989c4 100644 --- a/src/fix/disable-per-line.test.ts +++ b/src/fix/disable-per-line.test.ts @@ -144,8 +144,19 @@ describe('disable-per-line', () => { expect( await tester.test({ code: [ - 'const foo = `', - ' This is a template literal', + '`', + // eslint-disable-next-line no-template-curly-in-string + '${void 1}', + // eslint-disable-next-line no-template-curly-in-string + '${0 + void 1}', + '${', + 'void 1', + '}', + // eslint-disable-next-line no-template-curly-in-string + '${`${void 1}`}', + '`;', + // MEMO: Code that includes indents can be fixed, but it will not be formatted prettily. This is a limitation of eslint-interactive. + 'const withIndent = `', // eslint-disable-next-line no-template-curly-in-string ' ${void 1}', '`;', @@ -153,8 +164,19 @@ describe('disable-per-line', () => { rules: { 'no-void': 'error' }, }), ).toMatchInlineSnapshot(` - "const foo = \` - This is a template literal + "\` + \${// eslint-disable-next-line no-void + void 1} + \${// eslint-disable-next-line no-void + 0 + void 1} + \${ + // eslint-disable-next-line no-void + void 1 + } + \${\`\${// eslint-disable-next-line no-void + void 1}\`} + \`; + const withIndent = \` \${ // eslint-disable-next-line no-void void 1} \`;" From 78aa7f44cf8d57a75a0eef51802a9425e50469db Mon Sep 17 00:00:00 2001 From: mizdra Date: Sat, 28 Sep 2024 14:16:41 +0900 Subject: [PATCH 5/5] add test cases for fixtures --- fixtures/lib/add-disable-comment.jsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fixtures/lib/add-disable-comment.jsx b/fixtures/lib/add-disable-comment.jsx index 2e1291a5..d87fe6a8 100644 --- a/fixtures/lib/add-disable-comment.jsx +++ b/fixtures/lib/add-disable-comment.jsx @@ -13,3 +13,7 @@ const jsx = (
{2 ** 10}
); +const foo = ` + This is a template literal + ${2 ** 10} +`;