diff --git a/docs/rules/comment-directive.md b/docs/rules/comment-directive.md index 33ad1fa9d..b6c30b2a6 100644 --- a/docs/rules/comment-directive.md +++ b/docs/rules/comment-directive.md @@ -21,8 +21,6 @@ It supports usage of the following comments: We can't write HTML comments in tags. ::: -This rule doesn't throw any warning. - ## :book: Rule Details ESLint doesn't provide any API to enhance `eslint-disable` functionality and ESLint rules cannot affect other rules. But ESLint provides [processors API](https://eslint.org/docs/developer-guide/working-with-plugins#processors-in-plugins). @@ -88,9 +86,45 @@ The `eslint-disable`-like comments can include descriptions to explain why the c +## :wrench: Options + +```json +{ + "vue/comment-directive": ["error", { + "reportUnusedDisableDirectives": false + }] +} +``` + +- `reportUnusedDisableDirectives` ... If `true`, to report unused `eslint-disable` HTML comments. default `false` + +### `{ "reportUnusedDisableDirectives": true }` + + + +```vue + +``` + + + +::: warning Note +Unused reports cannot be suppressed with `eslint-disable` HTML comments. +::: + ## :books: Further reading -- [Disabling rules with inline comments](https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments) +- [Disabling rules with inline comments] + +[Disabling rules with inline comments]: https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments ## :mag: Implementation diff --git a/lib/processor.js b/lib/processor.js index a45827632..b018e0bb8 100644 --- a/lib/processor.js +++ b/lib/processor.js @@ -3,64 +3,161 @@ */ 'use strict' +/** + * @typedef {import('eslint').Linter.LintMessage} LintMessage + */ +/** + * @typedef {object} GroupState + * @property {Set} GroupState.disableAllKeys + * @property {Map} GroupState.disableRuleKeys + */ + module.exports = { preprocess(code) { return [code] }, + /** + * @param {LintMessage[][]} messages + * @returns {LintMessage[]} + */ postprocess(messages) { const state = { + /** @type {GroupState} */ block: { - disableAll: false, - disableRules: new Set() + disableAllKeys: new Set(), + disableRuleKeys: new Map() }, + /** @type {GroupState} */ line: { - disableAll: false, - disableRules: new Set() + disableAllKeys: new Set(), + disableRuleKeys: new Map() } } + const usedDisableDirectiveKeys = [] + /** @type {Map} */ + const unusedDisableDirectiveReports = new Map() // Filter messages which are in disabled area. - return messages[0].filter((message) => { + const filteredMessages = messages[0].filter((message) => { if (message.ruleId === 'vue/comment-directive') { - const rules = message.message.split(' ') - const type = rules.shift() - const group = rules.shift() - switch (type) { - case '--': - state[group].disableAll = true + const directiveType = message.messageId + const data = message.message.split(' ') + switch (directiveType) { + case 'disableBlock': + state.block.disableAllKeys.add(data[1]) + break + case 'disableLine': + state.line.disableAllKeys.add(data[1]) + break + case 'enableBlock': + state.block.disableAllKeys.clear() + break + case 'enableLine': + state.line.disableAllKeys.clear() break - case '++': - state[group].disableAll = false + case 'disableBlockRule': + addDisableRule(state.block.disableRuleKeys, data[1], data[2]) break - case '-': - for (const rule of rules) { - state[group].disableRules.add(rule) - } + case 'disableLineRule': + addDisableRule(state.line.disableRuleKeys, data[1], data[2]) break - case '+': - for (const rule of rules) { - state[group].disableRules.delete(rule) - } + case 'enableBlockRule': + state.block.disableRuleKeys.delete(data[1]) + break + case 'enableLineRule': + state.line.disableRuleKeys.delete(data[1]) break case 'clear': - state.block.disableAll = false - state.block.disableRules.clear() - state.line.disableAll = false - state.line.disableRules.clear() + state.block.disableAllKeys.clear() + state.block.disableRuleKeys.clear() + state.line.disableAllKeys.clear() + state.line.disableRuleKeys.clear() + break + default: + // unused eslint-disable comments report + unusedDisableDirectiveReports.set(messageToKey(message), message) break } return false } else { - return !( - state.block.disableAll || - state.line.disableAll || - state.block.disableRules.has(message.ruleId) || - state.line.disableRules.has(message.ruleId) - ) + const disableDirectiveKeys = [] + if (state.block.disableAllKeys.size) { + disableDirectiveKeys.push(...state.block.disableAllKeys) + } + if (state.line.disableAllKeys.size) { + disableDirectiveKeys.push(...state.line.disableAllKeys) + } + if (state.block.disableRuleKeys.has(message.ruleId)) { + disableDirectiveKeys.push( + ...state.block.disableRuleKeys.get(message.ruleId) + ) + } + if (state.line.disableRuleKeys.has(message.ruleId)) { + disableDirectiveKeys.push( + ...state.line.disableRuleKeys.get(message.ruleId) + ) + } + + if (disableDirectiveKeys.length) { + // Store used eslint-disable comment key + usedDisableDirectiveKeys.push(...disableDirectiveKeys) + return false + } else { + return true + } } }) + + if (unusedDisableDirectiveReports.size) { + for (const key of usedDisableDirectiveKeys) { + // Remove used eslint-disable comments + unusedDisableDirectiveReports.delete(key) + } + // Reports unused eslint-disable comments + filteredMessages.push(...unusedDisableDirectiveReports.values()) + filteredMessages.sort(compareLocations) + } + + return filteredMessages }, supportsAutofix: true } + +/** + * @param {Map} disableRuleKeys + * @param {string} rule + * @param {string} key + */ +function addDisableRule(disableRuleKeys, rule, key) { + let keys = disableRuleKeys.get(rule) + if (keys) { + keys.push(key) + } else { + keys = [key] + disableRuleKeys.set(rule, keys) + } +} + +/** + * @param {LintMessage} message + * @returns {string} message key + */ +function messageToKey(message) { + return `line:${message.line},column${ + // -1 because +1 by ESLint's `report-translator`. + message.column - 1 + }` +} + +/** + * Compares the locations of two objects in a source file + * @param {{line: number, column: number}} itemA The first object + * @param {{line: number, column: number}} itemB The second object + * @returns {number} A value less than 1 if itemA appears before itemB in the source file, greater than 1 if + * itemA appears after itemB in the source file, or 0 if itemA and itemB have the same location. + */ +function compareLocations(itemA, itemB) { + return itemA.line - itemB.line || itemA.column - itemB.column +} diff --git a/lib/rules/comment-directive.js b/lib/rules/comment-directive.js index 9b3842ca9..74d528a96 100644 --- a/lib/rules/comment-directive.js +++ b/lib/rules/comment-directive.js @@ -5,12 +5,24 @@ 'use strict' +/** + * @typedef {import('eslint').Rule.RuleContext} RuleContext + * @typedef {import('vue-eslint-parser').AST.VDocumentFragment} VDocumentFragment + * @typedef {import('vue-eslint-parser').AST.VElement} VElement + * @typedef {import('vue-eslint-parser').AST.Token} Token + * @typedef {import('vue-eslint-parser').AST.Location} Location + */ +/** + * @typedef {object} RuleAndLocation + * @property {string} RuleAndLocation.ruleId + * @property {number} RuleAndLocation.index + */ // ----------------------------------------------------------------------------- // Helpers // ----------------------------------------------------------------------------- -const COMMENT_DIRECTIVE_B = /^\s*(eslint-(?:en|dis)able)(?:\s+(\S|\S[\s\S]*\S))?\s*$/ -const COMMENT_DIRECTIVE_L = /^\s*(eslint-disable(?:-next)?-line)(?:\s+(\S|\S[\s\S]*\S))?\s*$/ +const COMMENT_DIRECTIVE_B = /^\s*(eslint-(?:en|dis)able)(?:\s+|$)/ +const COMMENT_DIRECTIVE_L = /^\s*(eslint-disable(?:-next)?-line)(?:\s+|$)/ /** * Remove the ignored part from a given directive comment and trim it. @@ -18,26 +30,40 @@ const COMMENT_DIRECTIVE_L = /^\s*(eslint-disable(?:-next)?-line)(?:\s+(\S|\S[\s\ * @returns {string} The stripped text. */ function stripDirectiveComment(value) { - return value.split(/\s-{2,}\s/u)[0].trim() + return value.split(/\s-{2,}\s/u)[0] } /** * Parse a given comment. * @param {RegExp} pattern The RegExp pattern to parse. * @param {string} comment The comment value to parse. - * @returns {({type:string,rules:string[]})|null} The parsing result. + * @returns {({type:string,rules:RuleAndLocation[]})|null} The parsing result. */ function parse(pattern, comment) { - const match = pattern.exec(stripDirectiveComment(comment)) + const text = stripDirectiveComment(comment) + const match = pattern.exec(text) if (match == null) { return null } const type = match[1] - const rules = (match[2] || '') - .split(',') - .map((s) => s.trim()) - .filter(Boolean) + + /** @type {RuleAndLocation[]} */ + const rules = [] + + const rulesRe = /([^,\s]+)[,\s]*/g + let startIndex = match[0].length + rulesRe.lastIndex = startIndex + + let res + while ((res = rulesRe.exec(text))) { + const ruleId = res[1].trim() + rules.push({ + ruleId, + index: startIndex + }) + startIndex = rulesRe.lastIndex + } return { type, rules } } @@ -46,18 +72,21 @@ function parse(pattern, comment) { * Enable rules. * @param {RuleContext} context The rule context. * @param {{line:number,column:number}} loc The location information to enable. - * @param {string} group The group to enable. - * @param {string[]} rules The rule IDs to enable. + * @param { 'block' | 'line' } group The group to enable. + * @param {string | null} rule The rule ID to enable. * @returns {void} */ -function enable(context, loc, group, rules) { - if (rules.length === 0) { - context.report({ loc, message: '++ {{group}}', data: { group } }) +function enable(context, loc, group, rule) { + if (!rule) { + context.report({ + loc, + messageId: group === 'block' ? 'enableBlock' : 'enableLine' + }) } else { context.report({ loc, - message: '+ {{group}} {{rules}}', - data: { group, rules: rules.join(' ') } + messageId: group === 'block' ? 'enableBlockRule' : 'enableLineRule', + data: { rule } }) } } @@ -66,18 +95,23 @@ function enable(context, loc, group, rules) { * Disable rules. * @param {RuleContext} context The rule context. * @param {{line:number,column:number}} loc The location information to disable. - * @param {string} group The group to disable. - * @param {string[]} rules The rule IDs to disable. + * @param { 'block' | 'line' } group The group to disable. + * @param {string | null} rule The rule ID to disable. + * @param {string} key The disable directive key. * @returns {void} */ -function disable(context, loc, group, rules) { - if (rules.length === 0) { - context.report({ loc, message: '-- {{group}}', data: { group } }) +function disable(context, loc, group, rule, key) { + if (!rule) { + context.report({ + loc, + messageId: group === 'block' ? 'disableBlock' : 'disableLine', + data: { key } + }) } else { context.report({ loc, - message: '- {{group}} {{rules}}', - data: { group, rules: rules.join(' ') } + messageId: group === 'block' ? 'disableBlockRule' : 'disableLineRule', + data: { rule, key } }) } } @@ -87,15 +121,40 @@ function disable(context, loc, group, rules) { * If the comment is `eslint-disable` or `eslint-enable` then it reports the comment. * @param {RuleContext} context The rule context. * @param {Token} comment The comment token to process. + * @param {boolean} reportUnusedDisableDirectives To report unused eslint-disable comments. * @returns {void} */ -function processBlock(context, comment) { +function processBlock(context, comment, reportUnusedDisableDirectives) { const parsed = parse(COMMENT_DIRECTIVE_B, comment.value) if (parsed != null) { if (parsed.type === 'eslint-disable') { - disable(context, comment.loc.start, 'block', parsed.rules) + if (parsed.rules.length) { + const rules = reportUnusedDisableDirectives + ? reportUnusedRules(context, comment, parsed.type, parsed.rules) + : parsed.rules + for (const rule of rules) { + disable( + context, + comment.loc.start, + 'block', + rule.ruleId, + rule.key || '*' + ) + } + } else { + const key = reportUnusedDisableDirectives + ? reportUnused(context, comment, parsed.type) + : '' + disable(context, comment.loc.start, 'block', null, key) + } } else { - enable(context, comment.loc.start, 'block', parsed.rules) + if (parsed.rules.length) { + for (const rule of parsed.rules) { + enable(context, comment.loc.start, 'block', rule.ruleId) + } + } else { + enable(context, comment.loc.start, 'block', null) + } } } } @@ -105,26 +164,109 @@ function processBlock(context, comment) { * If the comment is `eslint-disable-line` or `eslint-disable-next-line` then it reports the comment. * @param {RuleContext} context The rule context. * @param {Token} comment The comment token to process. + * @param {boolean} reportUnusedDisableDirectives To report unused eslint-disable comments. * @returns {void} */ -function processLine(context, comment) { +function processLine(context, comment, reportUnusedDisableDirectives) { const parsed = parse(COMMENT_DIRECTIVE_L, comment.value) if (parsed != null && comment.loc.start.line === comment.loc.end.line) { const line = comment.loc.start.line + (parsed.type === 'eslint-disable-line' ? 0 : 1) const column = -1 - disable(context, { line, column }, 'line', parsed.rules) - enable(context, { line: line + 1, column }, 'line', parsed.rules) + if (parsed.rules.length) { + const rules = reportUnusedDisableDirectives + ? reportUnusedRules(context, comment, parsed.type, parsed.rules) + : parsed.rules + for (const rule of rules) { + disable(context, { line, column }, 'line', rule.ruleId, rule.key || '') + enable(context, { line: line + 1, column }, 'line', rule.ruleId) + } + } else { + const key = reportUnusedDisableDirectives + ? reportUnused(context, comment, parsed.type) + : '' + disable(context, { line, column }, 'line', null, key) + enable(context, { line: line + 1, column }, 'line', null) + } } } +/** + * Reports unused disable directive. + * Do not check the use of directives here. Filter the directives used with postprocess. + * @param {RuleContext} context The rule context. + * @param {Token} comment The comment token to report. + * @param {string} kind The comment directive kind. + * @returns {string} The report key + */ +function reportUnused(context, comment, kind) { + const loc = comment.loc + + context.report({ + loc, + messageId: 'unused', + data: { kind } + }) + + return locToKey(loc.start) +} + +/** + * Reports unused disable directive rules. + * Do not check the use of directives here. Filter the directives used with postprocess. + * @param {RuleContext} context The rule context. + * @param {Token} comment The comment token to report. + * @param {string} kind The comment directive kind. + * @param {RuleAndLocation[]} rules To report rule. + * @returns { { ruleId: string; key: string; }[] } + */ +function reportUnusedRules(context, comment, kind, rules) { + const sourceCode = context.getSourceCode() + const commentStart = comment.range[0] + 4 /* ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 0) + }) }) describe('eslint-disable-line', () => { @@ -373,4 +386,169 @@ describe('comment-directive', () => { assert.deepEqual(messages[1].ruleId, 'vue/no-duplicate-attributes') }) }) + + describe('reportUnusedDisableDirectives', () => { + const linter = new eslint.CLIEngine({ + parser: require.resolve('vue-eslint-parser'), + parserOptions: { + ecmaVersion: 2015 + }, + plugins: ['vue'], + rules: { + 'no-unused-vars': 'error', + 'vue/comment-directive': [ + 'error', + { reportUnusedDisableDirectives: true } + ], + 'vue/no-parsing-error': 'error', + 'vue/no-duplicate-attributes': 'error' + }, + useEslintrc: false + }) + it('report unused ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 1) + assert.deepEqual(messages[0].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[0].message, + 'Unused eslint-disable directive (no problems were reported).' + ) + assert.deepEqual(messages[0].line, 3) + assert.deepEqual(messages[0].column, 11) + }) + + it('dont report unused ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 0) + }) + it('disable and report unused ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 1) + assert.deepEqual(messages[0].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[0].message, + 'Unused eslint-disable directive (no problems were reported).' + ) + assert.deepEqual(messages[0].line, 6) + assert.deepEqual(messages[0].column, 11) + }) + + it('report unused ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 2) + + assert.deepEqual(messages[0].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[0].message, + "Unused eslint-disable directive (no problems were reported from 'vue/no-duplicate-attributes')." + ) + assert.deepEqual(messages[0].line, 3) + assert.deepEqual(messages[0].column, 31) + + assert.deepEqual(messages[1].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[1].message, + "Unused eslint-disable directive (no problems were reported from 'vue/no-parsing-error')." + ) + assert.deepEqual(messages[1].line, 3) + assert.deepEqual(messages[1].column, 60) + }) + + it('report unused ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 4) + + assert.deepEqual(messages[0].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[0].message, + "Unused eslint-disable-next-line directive (no problems were reported from 'vue/no-duplicate-attributes')." + ) + assert.deepEqual(messages[0].line, 3) + assert.deepEqual(messages[0].column, 41) + + assert.deepEqual(messages[1].ruleId, 'vue/comment-directive') + assert.deepEqual( + messages[1].message, + "Unused eslint-disable-next-line directive (no problems were reported from 'vue/no-parsing-error')." + ) + assert.deepEqual(messages[1].line, 3) + assert.deepEqual(messages[1].column, 70) + + assert.deepEqual(messages[2].ruleId, 'vue/no-parsing-error') + assert.deepEqual(messages[2].line, 5) + assert.deepEqual(messages[3].ruleId, 'vue/no-duplicate-attributes') + assert.deepEqual(messages[3].line, 5) + }) + + it('dont report used ', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 0) + }) + + it('dont report used, with duplicate eslint-disable', () => { + const code = ` + + ` + const messages = linter.executeOnText(code, 'test.vue').results[0] + .messages + + assert.deepEqual(messages.length, 0) + }) + }) })