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

Add new rules no-missing-message-ids and no-unused-message-ids #254

Merged
merged 2 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 59 additions & 0 deletions docs/rules/no-missing-message-ids.md
Original file line number Diff line number Diff line change
@@ -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
60 changes: 60 additions & 0 deletions docs/rules/no-unused-message-ids.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions docs/rules/prefer-message-ids.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
88 changes: 88 additions & 0 deletions lib/rules/no-missing-message-ids.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
'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 this `messageId`.',
Copy link
Contributor

Choose a reason for hiding this comment

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

could the message be clearer, e.g. messageId "foo"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes updated.

},
},

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)
)
// Couldn't find this messageId in `meta.messages`.
context.report({
node: val,
messageId: 'missingMessage',
});
});
}
}
},
};
},
};
113 changes: 113 additions & 0 deletions lib/rules/no-unused-message-ids.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
'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: 'This message is never used.',
bmish marked this conversation as resolved.
Show resolved Hide resolved
},
},

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) {
for (const messageIdNode of messageIdNodes.filter(
(node) => !messageIdsUsed.has(node.key.name)
)) {
context.report({
node: messageIdNode,
messageId: 'unusedMessage',
});
}
}
},

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));
}
},
};
},
};