diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dfb6926..33e7027a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,23 @@ +### [5.0.3](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/compare/v5.0.2...v5.0.3) (2022-08-12) + + +### Bug Fixes + +* false positive with no-unused-message-ids from external violation reporting function ([#286](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/286)) ([01d0eef](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/01d0eef2885b1626642d925735c9cb59c1e611b8)) +* handle spread in context.report() in require-meta-fixable ([#288](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/288)) ([d768112](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/d7681123fe78c64a87f52fb719e83bdb3995b7c6)) +* handle spread in context.report() in require-meta-has-suggestions ([#287](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/287)) ([fdffb50](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/fdffb501bf62e25bc59a2f960abb0bcc9327e81d)) + ### [5.0.2](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/compare/v5.0.1...v5.0.2) (2022-08-04) ### Bug Fixes -* allow additional schema types in require-meta-schema ([#277](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/277)) ([5bf0648](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/5bf0648f50213fa30e9b623a8db376f41a3af411)) -* clarify report messages for no-missing-placeholders and no-unused-placeholders ([#278](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/278)) ([f5a5c24](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/f5a5c2458e79c405f5a47c802dbca111301f635c)) -* Improve violation reporting location for `no-unused-placeholders` ([#279](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/279)) ([27c0b65](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/27c0b6558a9531c3b87a2776b1553adf29834e87)) -* reporting location in no-missing-placeholders ([#280](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/280)) ([31ff45c](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/31ff45c8840b90f575800ccbb7a35d1ba09c2ba8)) +* allow additional schema types in `require-meta-schema` ([#277](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/277)) ([5bf0648](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/5bf0648f50213fa30e9b623a8db376f41a3af411)) +* clarify report messages for `no-missing-placeholders` and `no-unused-placeholders` ([#278](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/278)) ([f5a5c24](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/f5a5c2458e79c405f5a47c802dbca111301f635c)) +* improve violation reporting location for `no-unused-placeholders` ([#279](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/279)) ([27c0b65](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/27c0b6558a9531c3b87a2776b1553adf29834e87)) +* improve violation reporting location for `no-missing-placeholders` ([#280](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/issues/280)) ([31ff45c](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/commit/31ff45c8840b90f575800ccbb7a35d1ba09c2ba8)) ### [5.0.1](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/compare/v5.0.0...v5.0.1) (2022-07-18) diff --git a/lib/rules/no-unused-message-ids.js b/lib/rules/no-unused-message-ids.js index df793605..8aa4bfb0 100644 --- a/lib/rules/no-unused-message-ids.js +++ b/lib/rules/no-unused-message-ids.js @@ -30,7 +30,8 @@ module.exports = { const messageIdsUsed = new Set(); let contextIdentifiers; - let shouldPerformUnusedCheck = true; + let hasSeenUnknownMessageId = false; + let hasSeenViolationReport = false; const messageIdNodes = utils.getMessageIdNodes(info, scopeManager); if (!messageIdNodes) { @@ -44,22 +45,29 @@ module.exports = { }, 'Program:exit'() { - if (shouldPerformUnusedCheck) { - const messageIdNodesUnused = messageIdNodes.filter( - (node) => - !messageIdsUsed.has(utils.getKeyName(node, context.getScope())) - ); - - // Report any messageIds that were never used. - for (const messageIdNode of messageIdNodesUnused) { - context.report({ - node: messageIdNode, - messageId: 'unusedMessage', - data: { - messageId: utils.getKeyName(messageIdNode, context.getScope()), - }, - }); - } + if (hasSeenUnknownMessageId || !hasSeenViolationReport) { + /* + Bail out when the rule is likely to have false positives. + - If we saw a dynamic/unknown messageId + - If we couldn't find any violation reporting code, likely because a helper function from an external file is handling this + */ + return; + } + + const messageIdNodesUnused = messageIdNodes.filter( + (node) => + !messageIdsUsed.has(utils.getKeyName(node, context.getScope())) + ); + + // Report any messageIds that were never used. + for (const messageIdNode of messageIdNodesUnused) { + context.report({ + node: messageIdNode, + messageId: 'unusedMessage', + data: { + messageId: utils.getKeyName(messageIdNode, context.getScope()), + }, + }); } }, @@ -76,6 +84,8 @@ module.exports = { return; } + hasSeenViolationReport = true; + const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo); for (const { messageId } of reportMessagesAndDataArray.filter( @@ -90,7 +100,7 @@ module.exports = { values.some((val) => val.type !== 'Literal') ) { // When a dynamic messageId is used and we can't detect its value, disable the rule to avoid false positives. - shouldPerformUnusedCheck = false; + hasSeenUnknownMessageId = true; } values.forEach((val) => messageIdsUsed.add(val.value)); } @@ -101,17 +111,21 @@ module.exports = { // In order to reduce false positives, we will also check for messageId properties anywhere in the file. // This is helpful especially in the event that helper functions are used for reporting violations. if (node.key.type === 'Identifier' && node.key.name === 'messageId') { + hasSeenViolationReport = true; + const values = node.value.type === 'Literal' ? [node.value] : utils.findPossibleVariableValues(node.value, scopeManager); + if ( values.length === 0 || values.some((val) => val.type !== 'Literal') ) { // When a dynamic messageId is used and we can't detect its value, disable the rule to avoid false positives. - shouldPerformUnusedCheck = false; + hasSeenUnknownMessageId = true; } + values.forEach((val) => messageIdsUsed.add(val.value)); } }, diff --git a/lib/rules/require-meta-fixable.js b/lib/rules/require-meta-fixable.js index e03d3ae3..d2ce03d0 100644 --- a/lib/rules/require-meta-fixable.js +++ b/lib/rules/require-meta-fixable.js @@ -73,10 +73,9 @@ module.exports = { node.callee.property.name === 'report' && (node.arguments.length > 4 || (node.arguments.length === 1 && - node.arguments[0].type === 'ObjectExpression' && - node.arguments[0].properties.some( - (prop) => utils.getKeyName(prop) === 'fix' - ))) + utils + .evaluateObjectProperties(node.arguments[0], scopeManager) + .some((prop) => utils.getKeyName(prop) === 'fix'))) ) { usesFixFunctions = true; } diff --git a/lib/rules/require-meta-has-suggestions.js b/lib/rules/require-meta-has-suggestions.js index 939b5175..44ea702e 100644 --- a/lib/rules/require-meta-has-suggestions.js +++ b/lib/rules/require-meta-has-suggestions.js @@ -49,9 +49,9 @@ module.exports = { (node.arguments.length === 1 && node.arguments[0].type === 'ObjectExpression')) ) { - const suggestProp = node.arguments[0].properties.find( - (prop) => utils.getKeyName(prop) === 'suggest' - ); + const suggestProp = utils + .evaluateObjectProperties(node.arguments[0], scopeManager) + .find((prop) => utils.getKeyName(prop) === 'suggest'); if (suggestProp) { const staticValue = getStaticValue( suggestProp.value, diff --git a/package.json b/package.json index 65bff0f4..cf38d742 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-eslint-plugin", - "version": "5.0.2", + "version": "5.0.3", "description": "An ESLint plugin for linting ESLint plugins", "author": "Teddy Katz", "main": "./lib/index.js", diff --git a/tests/lib/rules/no-unused-message-ids.js b/tests/lib/rules/no-unused-message-ids.js index 0e92d49a..d35cadfe 100644 --- a/tests/lib/rules/no-unused-message-ids.js +++ b/tests/lib/rules/no-unused-message-ids.js @@ -220,6 +220,13 @@ ruleTester.run('no-unused-message-ids', rule, { } }; `, + // Ignore when we couldn't find any calls to `context.report()`, likely because an external helper function is in use. + ` + module.exports = { + meta: { messages: { foo: 'bar' } }, + create(context) {} + }; + `, ], invalid: [ @@ -313,29 +320,13 @@ ruleTester.run('no-unused-message-ids', rule, { }, ], }, - { - // messageId unused with no reports - code: ` - module.exports = { - meta: { messages: { foo: 'hello world' } }, - create(context) { } - }; - `, - errors: [ - { - messageId: 'unusedMessage', - data: { messageId: 'foo' }, - type: 'Property', - }, - ], - }, { // messageId unused with meta.messages in variable code: ` const messages = { foo: 'hello world' }; module.exports = { meta: { messages }, - create(context) { } + create(context) { context.report({node, messageId: 'other'}); } }; `, errors: [ @@ -353,7 +344,7 @@ ruleTester.run('no-unused-message-ids', rule, { const extraMeta = { messages: { ...extraMessages } }; module.exports = { meta: { ...extraMeta }, - create(context) { } + create(context) { context.report({node, messageId: 'other'}); } }; `, errors: [ diff --git a/tests/lib/rules/require-meta-fixable.js b/tests/lib/rules/require-meta-fixable.js index 75de8443..8dc6e65b 100644 --- a/tests/lib/rules/require-meta-fixable.js +++ b/tests/lib/rules/require-meta-fixable.js @@ -189,7 +189,7 @@ ruleTester.run('require-meta-fixable', rule, { `, options: [{ catchNoFixerButFixableProperty: true }], }, - // Spread. + // Spread in meta. ` const extra = { 'fixable': 'code' }; module.exports = { @@ -199,6 +199,19 @@ ruleTester.run('require-meta-fixable', rule, { } }; `, + // Spread in report. + { + code: ` + module.exports = { + meta: { fixable: 'code' }, + create(context) { + const extra = { fix: foo }; + context.report({node, message, ...extra}); + } + }; + `, + options: [{ catchNoFixerButFixableProperty: true }], + }, ], invalid: [ diff --git a/tests/lib/rules/require-meta-has-suggestions.js b/tests/lib/rules/require-meta-has-suggestions.js index fa3adc60..5b2cceff 100644 --- a/tests/lib/rules/require-meta-has-suggestions.js +++ b/tests/lib/rules/require-meta-has-suggestions.js @@ -171,7 +171,7 @@ ruleTester.run('require-meta-has-suggestions', rule, { } }; `, - // Unrelated spread syntax. + // Unrelated spread syntax in rule. { code: ` const extra = {}; @@ -185,7 +185,7 @@ ruleTester.run('require-meta-has-suggestions', rule, { ecmaVersion: 9, }, }, - // Related spread. + // Related spread in meta. ` const extra = { hasSuggestions: true }; module.exports = { @@ -195,6 +195,16 @@ ruleTester.run('require-meta-has-suggestions', rule, { } }; `, + // Spread in report. + ` + module.exports = { + meta: { hasSuggestions: true }, + create(context) { + const extra = { suggest: [{}] }; + context.report({node, message, ...extra }); + } + }; + `, ], invalid: [