diff --git a/README.md b/README.md index 614c637c..cbb6bd9b 100644 --- a/README.md +++ b/README.md @@ -67,8 +67,10 @@ Name | ✔️ | 🛠 | 💡 | Description [no-deprecated-context-methods](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-deprecated-context-methods.md) | ✔️ | 🛠 | | disallow usage of deprecated methods on rule context objects [no-deprecated-report-api](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-deprecated-report-api.md) | ✔️ | 🛠 | | disallow the version of `context.report()` with multiple arguments [no-identical-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-identical-tests.md) | ✔️ | 🛠 | | disallow identical tests +[no-missing-message-ids](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-message-ids.md) | | | | disallow `messageId`s that are missing from `meta.messages` [no-missing-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-placeholders.md) | ✔️ | | | disallow missing placeholders in rule report messages [no-only-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-only-tests.md) | ✔️ | | 💡 | disallow the test case property `only` +[no-unused-message-ids](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-unused-message-ids.md) | | | | disallow unused `messageId`s in `meta.messages` [no-unused-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-unused-placeholders.md) | ✔️ | | | disallow unused placeholders in rule report messages [no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 | | disallow unnecessary calls to `sourceCode.getFirstToken()` and `sourceCode.getLastToken()` [prefer-message-ids](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-message-ids.md) | | | | require using `messageId` instead of `message` to report rule violations diff --git a/docs/rules/no-missing-message-ids.md b/docs/rules/no-missing-message-ids.md new file mode 100644 index 00000000..76ee3599 --- /dev/null +++ b/docs/rules/no-missing-message-ids.md @@ -0,0 +1,59 @@ +# Disallow `messageId`s that are missing from `meta.messages` (no-missing-message-ids) + +When using `meta.messages` and `messageId` to report rule violations, it's possible to mistakenly use a `messageId` that doesn't exist in `meta.messages`. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```js +/* eslint eslint-plugin/no-missing-message-ids: error */ + +module.exports = { + meta: { + messages: { + foo: 'hello world', + }, + }, + create(context) { + return { + CallExpression(node) { + context.report({ + node, + messageId: 'abc', + }); + }, + }; + }, +}; +``` + +Examples of **correct** code for this rule: + +```js +/* eslint eslint-plugin/no-missing-message-ids: error */ + +module.exports = { + meta: { + messages: { + foo: 'hello world', + }, + }, + create(context) { + return { + CallExpression(node) { + context.report({ + node, + messageId: 'foo', + }); + }, + }; + }, +}; +``` + +## Further Reading + +* [messageIds API](https://eslint.org/docs/developer-guide/working-with-rules#messageids) +* [no-unused-message-ids](./no-unused-message-ids.md) rule +* [prefer-message-ids](./prefer-message-ids.md) rule diff --git a/docs/rules/no-unused-message-ids.md b/docs/rules/no-unused-message-ids.md new file mode 100644 index 00000000..758140ab --- /dev/null +++ b/docs/rules/no-unused-message-ids.md @@ -0,0 +1,60 @@ +# Disallow unused `messageId`s in `meta.messages` (no-unused-message-ids) + +When using `meta.messages` and `messageId` to report rule violations, it's possible to mistakenly leave a message in `meta.messages` that is never used. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```js +/* eslint eslint-plugin/no-unused-message-ids: error */ + +module.exports = { + meta: { + messages: { + foo: 'hello world', + bar: 'lorem ipsum', // this message is never used + }, + }, + create(context) { + return { + CallExpression(node) { + context.report({ + node, + messageId: 'foo', + }); + }, + }; + }, +}; +``` + +Examples of **correct** code for this rule: + +```js +/* eslint eslint-plugin/no-unused-message-ids: error */ + +module.exports = { + meta: { + messages: { + foo: 'hello world', + }, + }, + create(context) { + return { + CallExpression(node) { + context.report({ + node, + messageId: 'foo', + }); + }, + }; + }, +}; +``` + +## Further Reading + +* [messageIds API](https://eslint.org/docs/developer-guide/working-with-rules#messageids) +* [no-missing-message-ids](./no-missing-message-ids.md) rule +* [prefer-message-ids](./prefer-message-ids.md) rule diff --git a/docs/rules/prefer-message-ids.md b/docs/rules/prefer-message-ids.md index 25985332..4455df26 100644 --- a/docs/rules/prefer-message-ids.md +++ b/docs/rules/prefer-message-ids.md @@ -55,3 +55,5 @@ module.exports = { ## Further Reading * [messageIds API](https://eslint.org/docs/developer-guide/working-with-rules#messageids) +* [no-invalid-message-ids](./no-invalid-message-ids.md) rule +* [no-missing-message-ids](./no-missing-message-ids.md) rule diff --git a/lib/rules/no-missing-message-ids.js b/lib/rules/no-missing-message-ids.js new file mode 100644 index 00000000..f10c9507 --- /dev/null +++ b/lib/rules/no-missing-message-ids.js @@ -0,0 +1,97 @@ +'use strict'; + +const utils = require('../utils'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'disallow `messageId`s that are missing from `meta.messages`', + category: 'Rules', + recommended: false, + url: 'https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/no-missing-message-ids.md', + }, + fixable: null, + schema: [], + messages: { + missingMessage: + '`meta.messages` is missing the messageId "{{messageId}}".', + }, + }, + + create(context) { + const sourceCode = context.getSourceCode(); + const { scopeManager } = sourceCode; + const ruleInfo = utils.getRuleInfo(sourceCode); + + const messagesNode = utils.getMessagesNode(ruleInfo, scopeManager); + + let contextIdentifiers; + + if (!messagesNode || messagesNode.type !== 'ObjectExpression') { + // If we can't find `meta.messages`, disable the rule. + return {}; + } + + return { + Program(ast) { + contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast); + }, + + CallExpression(node) { + // Check for messageId properties used in known calls to context.report(); + if ( + node.callee.type === 'MemberExpression' && + contextIdentifiers.has(node.callee.object) && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'report' + ) { + const reportInfo = utils.getReportInfo(node.arguments, context); + if (!reportInfo) { + return; + } + + const reportMessagesAndDataArray = + utils.collectReportViolationAndSuggestionData(reportInfo); + for (const { messageId } of reportMessagesAndDataArray.filter( + (obj) => obj.messageId + )) { + const values = + messageId.type === 'Literal' + ? [messageId] + : utils.findPossibleVariableValues(messageId, scopeManager); + + // Look for any possible string values we found for this messageId. + values.forEach((val) => { + if ( + val.type === 'Literal' && + val.value !== null && + val.value !== '' && + !utils.getMessageIdNodeById( + val.value, + ruleInfo, + scopeManager, + context.getScope() + ) + ) + // Couldn't find this messageId in `meta.messages`. + context.report({ + node: val, + messageId: 'missingMessage', + data: { + messageId: val.value, + }, + }); + }); + } + } + }, + }; + }, +}; diff --git a/lib/rules/no-unused-message-ids.js b/lib/rules/no-unused-message-ids.js new file mode 100644 index 00000000..a1724a60 --- /dev/null +++ b/lib/rules/no-unused-message-ids.js @@ -0,0 +1,120 @@ +'use strict'; + +const utils = require('../utils'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'disallow unused `messageId`s in `meta.messages`', + category: 'Rules', + recommended: false, + url: 'https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/no-unused-message-ids.md', + }, + fixable: null, + schema: [], + messages: { + unusedMessage: 'The messageId "{{messageId}}" is never used.', + }, + }, + + create(context) { + const sourceCode = context.getSourceCode(); + const { scopeManager } = sourceCode; + const info = utils.getRuleInfo(sourceCode); + + const messageIdsUsed = new Set(); + let contextIdentifiers; + let shouldPerformUnusedCheck = true; + + const messageIdNodes = utils.getMessageIdNodes(info, scopeManager); + if (!messageIdNodes) { + // If we can't find `meta.messages`, disable the rule. + return {}; + } + + return { + Program(ast) { + contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast); + }, + + '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()), + }, + }); + } + } + }, + + CallExpression(node) { + // Check for messageId properties used in known calls to context.report(); + if ( + node.callee.type === 'MemberExpression' && + contextIdentifiers.has(node.callee.object) && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'report' + ) { + const reportInfo = utils.getReportInfo(node.arguments, context); + if (!reportInfo) { + return; + } + + const reportMessagesAndDataArray = + utils.collectReportViolationAndSuggestionData(reportInfo); + for (const { messageId } of reportMessagesAndDataArray.filter( + (obj) => obj.messageId + )) { + const values = + messageId.type === 'Literal' + ? [messageId] + : utils.findPossibleVariableValues(messageId, 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; + } + values.forEach((val) => messageIdsUsed.add(val.value)); + } + } + }, + + Property(node) { + // 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') { + 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; + } + values.forEach((val) => messageIdsUsed.add(val.value)); + } + }, + }; + }, +}; diff --git a/lib/utils.js b/lib/utils.js index 26138755..5b744964 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -284,6 +284,30 @@ function findVariableValue(node, scopeManager) { } } +/** + * Retrieve all possible elements from an array. + * If a ternary conditional expression is involved, retrieve the elements that may exist on both sides of it. + * Ex: [a, b, c] will return [a, b, c] + * Ex: foo ? [a, b, c] : [d, e, f] will return [a, b, c, d, e, f] + * @param {Node} node + * @returns {Node[]} the list of elements + */ +function collectArrayElements(node) { + if (!node) { + return []; + } + if (node.type === 'ArrayExpression') { + return node.elements; + } + if (node.type === 'ConditionalExpression') { + return [ + ...collectArrayElements(node.consequent), + ...collectArrayElements(node.alternate), + ]; + } + return []; +} + module.exports = { /** * Performs static analysis on an AST to try to determine the final value of `module.exports`. @@ -354,14 +378,24 @@ module.exports = { /** * Gets the key name of a Property, if it can be determined statically. * @param {ASTNode} node The `Property` node + * @param {Scope} scope * @returns {string|null} The key name, or `null` if the name cannot be determined statically. */ - getKeyName(property) { + getKeyName(property, scope) { if (!property.key) { // likely a SpreadElement or another non-standard node return null; } - if (!property.computed && property.key.type === 'Identifier') { + if (property.key.type === 'Identifier') { + if (property.computed) { + // Variable key: { [myVariable]: 'hello world' } + if (scope) { + const staticValue = getStaticValue(property.key, scope); + return staticValue ? staticValue.value : null; + } + // TODO: ensure scope is always passed to getKeyName() so we don't need to handle the case where it's not passed. + return null; + } return property.key.name; } if (property.key.type === 'Literal') { @@ -669,7 +703,7 @@ module.exports = { fix: reportInfo.fix, }, // Suggestion messages - ...((reportInfo.suggest && reportInfo.suggest.elements) || []) + ...collectArrayElements(reportInfo.suggest) .map((suggestObjNode) => { if (suggestObjNode.type !== 'ObjectExpression') { // Ignore non-objects (like variables or function calls). @@ -762,4 +796,91 @@ module.exports = { return [property]; }); }, + + /** + * Get the `meta.messages` node from a rule. + * @param {RuleInfo} ruleInfo + * @param {ScopeManager} scopeManager + * @returns {Node|undefined} + */ + getMessagesNode(ruleInfo, scopeManager) { + if (!ruleInfo) { + return; + } + + const metaNode = ruleInfo.meta; + const messagesNode = module.exports + .evaluateObjectProperties(metaNode, scopeManager) + .find( + (p) => + p.type === 'Property' && module.exports.getKeyName(p) === 'messages' + ); + + if (messagesNode) { + if (messagesNode.value.type === 'ObjectExpression') { + return messagesNode.value; + } + const value = findVariableValue(messagesNode.value, scopeManager); + if (value && value.type === 'ObjectExpression') { + return value; + } + } + }, + + /** + * Get the list of messageId properties from `meta.messages` for a rule. + * @param {RuleInfo} ruleInfo + * @param {ScopeManager} scopeManager + * @returns {Node[]|undefined} + */ + getMessageIdNodes(ruleInfo, scopeManager) { + const messagesNode = module.exports.getMessagesNode(ruleInfo, scopeManager); + + return messagesNode && messagesNode.type === 'ObjectExpression' + ? module.exports.evaluateObjectProperties(messagesNode, scopeManager) + : undefined; + }, + + /** + * Get the messageId property from a rule's `meta.messages` that matches the given `messageId`. + * @param {String} messageId - the messageId to check for + * @param {RuleInfo} ruleInfo + * @param {ScopeManager} scopeManager + * @param {Scope} scope + * @returns {Node|undefined} The matching messageId property from `meta.messages`. + */ + getMessageIdNodeById(messageId, ruleInfo, scopeManager, scope) { + return module.exports + .getMessageIdNodes(ruleInfo, scopeManager) + .find( + (p) => + p.type === 'Property' && + module.exports.getKeyName(p, scope) === messageId + ); + }, + + /** + * Get the possible values that a variable was initialized to at some point. + * @param {Node} node - the Identifier node for the variable. + * @param {ScopeManager} scopeManager + * @returns {Node[]} the values that the given variable could be initialized to. + */ + findPossibleVariableValues(node, scopeManager) { + const variable = findVariable( + scopeManager.acquire(node) || scopeManager.globalScope, + node + ); + return ((variable && variable.references) || []).flatMap((ref) => { + if ( + ref.writeExpr && + (ref.writeExpr.parent.type !== 'AssignmentExpression' || + ref.writeExpr.parent.operator === '=') + ) { + // Given node `x`, get `123` from `x = 123;`. + // Ignore assignments with other operators like `x += 'abc';'`; + return [ref.writeExpr]; + } + return []; + }); + }, }; diff --git a/tests/lib/rules/no-missing-message-ids.js b/tests/lib/rules/no-missing-message-ids.js new file mode 100644 index 00000000..22a465dd --- /dev/null +++ b/tests/lib/rules/no-missing-message-ids.js @@ -0,0 +1,291 @@ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-missing-message-ids'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 9 } }); + +ruleTester.run('no-missing-message-ids', rule, { + valid: [ + // message + ` + module.exports = { + create(context) { + context.report({ node, message: 'foo' }); + } + }; + `, + // messageId + ` + module.exports = { + meta: { messages: { someMessageId: 'some message' } }, + create(context) { + context.report({ node, messageId: 'someMessageId' }); + } + }; + `, + // Suggestion with messageId + ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + context.report({ node, suggest: [{messageId:'foo'}] }); + } + }; + `, + // messageId + ` + module.exports = { + meta: { messages: { someMessageId: 'some message' } }, + create(context) { + let messageId = null; + messageId = undefined; + messageId = ""; + messageId = 'someMessageId'; + context.report({ node, messageId }); + } + }; + `, + // messageId variable with multiple possible values and unexpected operator + ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + let messageId = 'foo'; + messageId += 'bar'; // ignored since not = operator + context.report({ node, messageId }); + } + }; + `, + { + // ESM + code: ` + export default { + meta: { messages: { foo: 'hello world' } }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + parserOptions: { sourceType: 'module' }, + }, + // unrelated function 1 + ` + module.exports = { + meta: { messages: {} }, + create(context) { + foo.report({ node, messageId: 'foo' }); + } + }; + `, + // unrelated function 2 + ` + module.exports = { + meta: { messages: {} }, + create(context) { + context.foo({ node, messageId: 'foo' }); + } + }; + `, + // not the right context function + ` + module.exports = { + meta: { messages: {} }, + create() { + context.foo({ node, messageId: 'foo' }); + } + }; + `, + // report outside rule + ` + context.report({ node, messageId: 'foo' }); + module.exports = { + meta: { messages: {} }, + create(context) {} + }; + `, + // test + ` + new RuleTester().run('foo', bar, { + invalid: [ + { code: 'foo', errors: [{messageId: 'foo'}] }, + ] + }); + `, + // `meta.messages` has a message (in variable) + ` + const messages = { someMessageId: 'some message' }; + module.exports = { + meta: { messages }, + create(context) { + context.report({ node, messageId: 'someMessageId' }); + } + }; + `, + // `meta.messages` has no static value. + ` + module.exports = { + meta: { messages }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + // `messageId` is not a literal + ` + module.exports = { + meta: { messages: {} }, + create(context) { + context.report({ node, messageId: FOO }); + } + }; + `, + // `context.report` with no args. + ` + module.exports = { + meta: { messages }, + create(context) { + context.report(); + } + }; + `, + // `meta.messages` empty + ` + module.exports = { + meta: { messages: {} }, + create(context) { + context.report(); + } + }; + `, + // `meta.messages` missing + ` + module.exports = { + meta: { }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + // `meta` missing + ` + module.exports = { + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + // with variable messageId key + ` + const MESSAGE_ID = 'foo'; + const messages = { + [MESSAGE_ID]: 'hello world', + }; + module.exports = { + meta: { messages }, + create(context) { + context.report({node, messageId: MESSAGE_ID}); + } + }; + `, + ], + + invalid: [ + { + // Missing message + code: ` + module.exports = { + meta: { messages: { } }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + errors: [ + { + messageId: 'missingMessage', + data: { messageId: 'foo' }, + type: 'Literal', + }, + ], + }, + { + // Missing messages with multiple possible values + code: ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + let messageId = 'abc'; + messageId = 'def'; + if (foo) { messageId = 'foo'; } else { messageId = 'bar'; } + context.report({ node, messageId }); + } + }; + `, + errors: [ + { + messageId: 'missingMessage', + data: { messageId: 'abc' }, + type: 'Literal', + }, + { + messageId: 'missingMessage', + data: { messageId: 'def' }, + type: 'Literal', + }, + { + messageId: 'missingMessage', + data: { messageId: 'bar' }, + type: 'Literal', + }, + ], + }, + { + // Missing message with spreads + code: ` + const extraMessages = { }; + const extraMeta = { messages: { ...extraMessages } }; + module.exports = { + meta: { ...extraMeta }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + errors: [ + { + messageId: 'missingMessage', + data: { messageId: 'foo' }, + type: 'Literal', + }, + ], + }, + { + // ESM + code: ` + export default { + meta: { messages: { } }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + messageId: 'missingMessage', + data: { messageId: 'foo' }, + type: 'Literal', + }, + ], + }, + ], +}); diff --git a/tests/lib/rules/no-unused-message-ids.js b/tests/lib/rules/no-unused-message-ids.js new file mode 100644 index 00000000..0e92d49a --- /dev/null +++ b/tests/lib/rules/no-unused-message-ids.js @@ -0,0 +1,390 @@ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-unused-message-ids'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 9 } }); + +ruleTester.run('no-unused-message-ids', rule, { + valid: [ + // message + ` + module.exports = { + create(context) { + context.report({ node, message: 'foo' }); + } + }; + `, + // messageId + ` + module.exports = { + meta: { messages: { someMessageId: 'some message' } }, + create(context) { + context.report({ node, messageId: 'someMessageId' }); + } + }; + `, + // Suggestion with messageId + ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + context.report({ node, suggest: [{messageId:'foo'}] }); + } + }; + `, + { + // ESM + code: ` + export default { + meta: { messages: { foo: 'hello world' } }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + parserOptions: { sourceType: 'module' }, + }, + // unrelated function 1 + ` + module.exports = { + create(context) { + foo.report({ node, messageId: 'foo' }); + } + }; + `, + // unrelated function 2 + ` + module.exports = { + create(context) { + context.foo({ node, message: 'foo' }); + } + }; + `, + // report outside rule + ` + context.report({ node, messageId: 'foo' }); + module.exports = { + create(context) {} + }; + `, + // test + ` + new RuleTester().run('foo', bar, { + invalid: [ + { code: 'foo', errors: [{messageId: 'foo'}] }, + ] + }); + `, + // `meta.messages` has a message (in variable) + ` + const messages = { someMessageId: 'some message' }; + module.exports = { + meta: { messages }, + create(context) { + context.report({ node, messageId: 'someMessageId' }); + } + }; + `, + // `meta.messages` has no static value. + ` + module.exports = { + meta: { messages }, + create(context) { + context.report({ node, messageId: 'foo' }); + } + }; + `, + // `messageId` is not a literal + ` + module.exports = { + meta: { messages: {} }, + create(context) { + context.report({ node, messageId: FOO }); + } + }; + `, + // `context.report` with no args. + ` + module.exports = { + meta: { messages }, + create(context) { + context.report(); + } + }; + `, + // `meta.messages` empty + ` + module.exports = { + meta: { messages: {} }, + create(context) { + context.report(); + } + }; + `, + // `meta.messages` missing + ` + module.exports = { + meta: { }, + create(context) { + context.report(); + } + }; + `, + // `meta` missing + ` + module.exports = { + create(context) { + context.report(); + } + }; + `, + // messageId variable with multiple possible values + ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + let messageId; + if (foo) { messageId = 'abc'; } else { messageId = getMessageId(); } + context.report({ node, messageId }); + } + }; + `, + // helper function for report + ` + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + function report({ node, messageId }) { + context.report({ node, messageId }); + } + report({ node, messageId: 'foo' }); + } + }; + `, + // helper function outside rule with dynamic messageId + ` + function report({ node, messageId }) { + context.report({ node, messageId }); + } + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + report({ node, messageId: 'foo' }); + } + }; + `, + // helper function outside rule with literal messageId + ` + function reportFoo(node) { + context.report({ node, messageId: 'foo' }); + } + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + reportFoo(node); + } + }; + `, + // helper function outside rule with variable messageId + ` + function reportFoo(node) { + const messageId = 'foo'; + context.report({ node, messageId }); + } + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + reportFoo(node); + } + }; + `, + // with variable messageId key + ` + const MESSAGE_ID = 'foo'; + const messages = { + [MESSAGE_ID]: 'hello world', + }; + module.exports = { + meta: { messages }, + create(context) { + context.report({node, messageId: MESSAGE_ID}); + } + }; + `, + ], + + invalid: [ + { + // Unused message + code: ` + module.exports = { + meta: { messages: { foo: 'hello world '} }, + create(context) { + context.report({ node, messageId: 'bar' }); + } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // Unused message with spreads + code: ` + const extraMessages = { foo: 'hello world' }; + const extraMeta = { messages: { ...extraMessages } }; + module.exports = { + meta: { ...extraMeta }, + create(context) { + context.report({ node, messageId: 'bar' }); + } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // ESM + code: ` + export default { + meta: { messages: { foo: 'hello world' } }, + create(context) { + context.report({ node, messageId: 'bar' }); + } + }; + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // `meta` / `create` in variables + code: ` + const meta = { messages: { foo: 'hello world' }}; + const create = function (context) { context.report({ node, messageId: 'bar' }); } + module.exports = { meta, create }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // messageId unused with multiple messages + code: ` + module.exports = { + meta: { messages: { foo: 'hello world', bar: 'hello world 2' } }, + create(context) { + context.report({ node, messageId: 'bar' }); + } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // 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) { } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // messageId unused with meta.messages in spreads + code: ` + const extraMessages = { foo: 'hello world' }; + const extraMeta = { messages: { ...extraMessages } }; + module.exports = { + meta: { ...extraMeta }, + create(context) { } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + { + // helper function outside rule with variable messageId + code: ` + function reportFoo(node) { + const messageId = 'bar'; + context.report({ node, messageId }); + } + module.exports = { + meta: { messages: { foo: 'hello world' } }, + create(context) { + reportFoo(node); + } + }; + `, + errors: [ + { + messageId: 'unusedMessage', + data: { messageId: 'foo' }, + type: 'Property', + }, + ], + }, + ], +}); diff --git a/tests/lib/utils.js b/tests/lib/utils.js index 9b797706..c6ecb382 100644 --- a/tests/lib/utils.js +++ b/tests/lib/utils.js @@ -572,13 +572,13 @@ describe('utils', () => { Object.keys(CASES).forEach((ruleSource) => { it(ruleSource, () => { const ast = espree.parse(ruleSource, { ecmaVersion: 6, range: true }); - const scope = eslintScope.analyze(ast, { + const scopeManager = eslintScope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', nodejsScope: true, }); - const identifiers = utils.getContextIdentifiers(scope, ast); + const identifiers = utils.getContextIdentifiers(scopeManager, ast); assert( identifiers instanceof Set, @@ -611,15 +611,51 @@ describe('utils', () => { '({ [foo]: 1 })': null, '({ [tag`foo`]: 1 })': null, '({ ["foo" + "bar"]: 1 })': null, + '({ [key]: 1 })': null, + 'const key = "foo"; ({ [key]: 1 });': { + getNode(ast) { + return ast.body[1].expression.properties[0]; + }, + result: 'foo', + resultWithoutScope: null, + }, }; Object.keys(CASES).forEach((objectSource) => { it(objectSource, () => { const ast = espree.parse(objectSource, { ecmaVersion: 6, range: true }); + const scopeManager = eslintScope.analyze(ast, { + ignoreEval: true, + ecmaVersion: 6, + sourceType: 'script', + nodejsScope: true, + }); - assert.strictEqual( - utils.getKeyName(ast.body[0].expression.properties[0]), - CASES[objectSource] - ); + const caseInfo = CASES[objectSource]; + if (typeof caseInfo === 'object' && caseInfo !== null) { + // Object-style test case used when we need to specify additional information for this test case. + assert.strictEqual( + utils.getKeyName(caseInfo.getNode(ast), scopeManager.globalScope), + caseInfo.result + ); + + if ( + Object.prototype.hasOwnProperty.call(caseInfo, 'resultWithoutScope') + ) { + // Ensure the behavior is correct when `scope` is omitted from the parameters. + assert.strictEqual( + utils.getKeyName(caseInfo.getNode(ast)), + caseInfo.resultWithoutScope + ); + } + } else { + assert.strictEqual( + utils.getKeyName( + ast.body[0].expression.properties[0], + scopeManager.globalScope + ), + caseInfo + ); + } }); }); @@ -657,14 +693,14 @@ describe('utils', () => { ecmaVersion: 8, range: true, }); - const scope = eslintScope.analyze(ast, { + const scopeManager = eslintScope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', nodejsScope: true, }); assert.deepEqual( - utils.getTestInfo(scope, ast), + utils.getTestInfo(scopeManager, ast), [], 'Expected no tests to be found' ); @@ -723,13 +759,13 @@ describe('utils', () => { Object.keys(CASES).forEach((testSource) => { it(testSource, () => { const ast = espree.parse(testSource, { ecmaVersion: 6, range: true }); - const scope = eslintScope.analyze(ast, { + const scopeManager = eslintScope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', nodejsScope: true, }); - const testInfo = utils.getTestInfo(scope, ast); + const testInfo = utils.getTestInfo(scopeManager, ast); assert.strictEqual( testInfo.length, @@ -917,13 +953,13 @@ describe('utils', () => { Object.keys(CASES).forEach((testSource) => { it(testSource, () => { const ast = espree.parse(testSource, { ecmaVersion: 6, range: true }); - const scope = eslintScope.analyze(ast, { + const scopeManager = eslintScope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', nodejsScope: true, }); - const testInfo = utils.getTestInfo(scope, ast); + const testInfo = utils.getTestInfo(scopeManager, ast); assert.strictEqual( testInfo.length, @@ -1019,7 +1055,7 @@ describe('utils', () => { Object.keys(CASES).forEach((testSource) => { it(testSource, () => { const ast = espree.parse(testSource, { ecmaVersion: 6, range: true }); - const scope = eslintScope.analyze(ast, { + const scopeManager = eslintScope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', @@ -1033,7 +1069,7 @@ describe('utils', () => { }); assert.strictEqual( - utils.getSourceCodeIdentifiers(scope, ast).size, + utils.getSourceCodeIdentifiers(scopeManager, ast).size, CASES[testSource] ); }); @@ -1128,6 +1164,21 @@ describe('utils', () => { }, ], }, + { + // Suggestions using a ternary/conditional expression. + code: ` + context.report({ + node: {}, + messageId: "messageId1", + suggest: foo ? [{messageId:'messageId2'}] : [{messageId: 'messageId3'}] + }); + `, + shouldMatch: [ + { messageId: { type: 'Literal', value: 'messageId1' } }, + { messageId: { type: 'Literal', value: 'messageId2' } }, + { messageId: { type: 'Literal', value: 'messageId3' } }, + ], + }, { // No suggestions. code: ` @@ -1351,4 +1402,186 @@ describe('utils', () => { assert.deepEqual(result, []); }); }); + + describe('getMessagesNode', function () { + [ + { + code: 'module.exports = { meta: { messages: {} }, create(context) {} };', + getResult(ast) { + return ast.body[0].expression.right.properties[0].value.properties[0] + .value; + }, + }, + { + // variable + code: ` + const messages = { foo: 'hello world' }; + module.exports = { meta: { messages }, create(context) {} }; + `, + getResult(ast) { + return ast.body[0].declarations[0].init; + }, + }, + { + // spread + code: ` + const extra = { messages: { foo: 'hello world' } }; + module.exports = { meta: { ...extra }, create(context) {} }; + `, + getResult(ast) { + return ast.body[0].declarations[0].init.properties[0].value; + }, + }, + { + code: `module.exports = { meta: FOO, create(context) {} };`, + getResult() {}, // returns undefined + }, + { + code: `module.exports = { create(context) {} };`, + getResult() {}, // returns undefined + }, + ].forEach((testCase) => { + describe(testCase.code, () => { + it('returns the right node', () => { + const ast = espree.parse(testCase.code, { + ecmaVersion: 9, + range: true, + }); + const scopeManager = eslintScope.analyze(ast); + const ruleInfo = utils.getRuleInfo({ ast, scopeManager }); + assert.strictEqual( + utils.getMessagesNode(ruleInfo, scopeManager), + testCase.getResult(ast) + ); + }); + }); + }); + }); + + describe('getMessageIdNodes', function () { + [ + { + code: 'module.exports = { meta: { messages: { foo: "hello world" } }, create(context) {} };', + getResult(ast) { + return ast.body[0].expression.right.properties[0].value.properties[0] + .value.properties; + }, + }, + { + // variable + code: ` + const messages = { foo: 'hello world' }; + module.exports = { meta: { messages }, create(context) {} }; + `, + getResult(ast) { + return ast.body[0].declarations[0].init.properties; + }, + }, + { + // spread + code: ` + const extra2 = { foo: 'hello world' }; + const extra = { messages: { ...extra2 } }; + module.exports = { meta: { ...extra }, create(context) {} }; + `, + getResult(ast) { + return ast.body[0].declarations[0].init.properties; + }, + }, + ].forEach((testCase) => { + describe(testCase.code, () => { + it('returns the right node', () => { + const ast = espree.parse(testCase.code, { + ecmaVersion: 9, + range: true, + }); + const scopeManager = eslintScope.analyze(ast); + const ruleInfo = utils.getRuleInfo({ ast, scopeManager }); + assert.deepEqual( + utils.getMessageIdNodes(ruleInfo, scopeManager), + testCase.getResult(ast) + ); + }); + }); + }); + }); + + describe('getMessageIdNodeById', function () { + [ + { + code: 'module.exports = { meta: { messages: { foo: "hello world" } }, create(context) {} };', + run(ruleInfo, scopeManager) { + return utils.getMessageIdNodeById( + 'foo', + ruleInfo, + scopeManager, + scopeManager.globalScope + ); + }, + getResult(ast) { + return ast.body[0].expression.right.properties[0].value.properties[0] + .value.properties[0]; + }, + }, + { + code: 'module.exports = { meta: { messages: { foo: "hello world" } }, create(context) {} };', + run(ruleInfo, scopeManager) { + return utils.getMessageIdNodeById( + 'bar', + ruleInfo, + scopeManager, + scopeManager.globalScope + ); + }, + getResult() {}, // returns undefined + }, + ].forEach((testCase) => { + describe(testCase.code, () => { + it('returns the right node', () => { + const ast = espree.parse(testCase.code, { + ecmaVersion: 9, + range: true, + }); + const scopeManager = eslintScope.analyze(ast); + const ruleInfo = utils.getRuleInfo({ ast, scopeManager }); + assert.strictEqual( + testCase.run(ruleInfo, scopeManager), + testCase.getResult(ast) + ); + }); + }); + }); + }); + + describe('findPossibleVariableValues', function () { + it('returns the right nodes', () => { + const code = + 'let x = 123; x = 456; x = foo(); if (foo) { x = 789; } x(); console.log(x); x += "shouldIgnore"; x + "shouldIgnore";'; + const ast = espree.parse(code, { + ecmaVersion: 9, + range: true, + }); + + // Add parent to each node. + estraverse.traverse(ast, { + enter(node, parent) { + node.parent = parent; + }, + }); + + const scopeManager = eslintScope.analyze(ast); + assert.deepEqual( + utils.findPossibleVariableValues( + ast.body[0].declarations[0].id, + scopeManager + ), + [ + ast.body[0].declarations[0].init, + ast.body[1].expression.right, + ast.body[2].expression.right, + ast.body[3].consequent.body[0].expression.right, + ] + ); + }); + }); });