Skip to content

Commit

Permalink
New: Add new rule prefer-message-ids (#170)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed Jul 26, 2021
1 parent fcbb65c commit 95021dd
Show file tree
Hide file tree
Showing 24 changed files with 298 additions and 56 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -57,6 +57,7 @@ Name | ✔️ | 🛠 | 💡 | Description
[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-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
[prefer-object-rule](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-object-rule.md) | | 🛠 | | disallow rule exports where the export is a function
[prefer-output-null](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-output-null.md) | | 🛠 | | disallow invalid RuleTester test cases where the `output` matches the `code`
[prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | | require using placeholders for dynamic report messages
Expand Down
57 changes: 57 additions & 0 deletions docs/rules/prefer-message-ids.md
@@ -0,0 +1,57 @@
# Require using `messageId` instead of `message` to report rule violations (prefer-message-ids)

When reporting a rule violation, it's preferred to provide the violation message with the `messageId` property instead of the `message` property. Message IDs provide the following benefits:

* Rule violation messages can be stored in a central `meta.messages` object for convenient management
* Rule violation messages do not need to be repeated in both the rule file and rule test file

## Rule Details

This rule catches usages of the `message` property when reporting a rule violation.

Examples of **incorrect** code for this rule:

```js
/* eslint eslint-plugin/prefer-message-ids: error */

module.exports = {
create (context) {
return {
CallExpression (node) {
context.report({
node,
message: 'Some error message.',
});
},
};
},
};
```

Examples of **correct** code for this rule:

```js
/* eslint eslint-plugin/prefer-message-ids: error */

module.exports = {
meta: {
messages: {
someMessageId: 'Some error message',
},
},
create (context) {
return {
CallExpression (node) {
context.report({
node,
messageId: 'someMessageId',
});
},
};
},
};
```

## Further Reading

* [messageIds API](https://eslint.org/docs/developer-guide/working-with-rules#messageids)
5 changes: 4 additions & 1 deletion lib/rules/consistent-output.js
Expand Up @@ -26,6 +26,9 @@ module.exports = {
enum: ['always', 'consistent'],
},
],
messages: {
missingOutput: 'This test case should have an output assertion.',
},
},

create (context) {
Expand All @@ -48,7 +51,7 @@ module.exports = {
casesWithoutOutput.forEach(testCase => {
context.report({
node: testCase,
message: 'This test case should have an output assertion.',
messageId: 'missingOutput',
});
});
}
Expand Down
6 changes: 4 additions & 2 deletions lib/rules/meta-property-ordering.js
Expand Up @@ -23,13 +23,15 @@ module.exports = {
type: 'array',
elements: { type: 'string' },
}],
messages: {
inconsistentOrder: 'The meta properties should be placed in a consistent order: [{{order}}].',
},
},

create (context) {
const sourceCode = context.getSourceCode();
const info = getRuleInfo(sourceCode);

const message = 'The meta properties should be placed in a consistent order: [{{order}}].';
const order = context.options[0] || ['type', 'docs', 'fixable', 'schema', 'messages'];

const orderMap = new Map(order.map((name, i) => [name, i]));
Expand Down Expand Up @@ -67,7 +69,7 @@ module.exports = {
for (const violatingProp of violatingProps) {
context.report({
node: violatingProp,
message,
messageId: 'inconsistentOrder',
data: {
order: knownProps.map(getKeyName).join(', '),
},
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-deprecated-context-methods.js
Expand Up @@ -44,6 +44,9 @@ module.exports = {
},
fixable: 'code',
schema: [],
messages: {
newFormat: 'Use `{{contextName}}.getSourceCode().{{replacement}}` instead of `{{contextName}}.{{original}}`.',
},
},

create (context) {
Expand All @@ -66,7 +69,7 @@ module.exports = {
contextId =>
context.report({
node: contextId.parent,
message: 'Use `{{contextName}}.getSourceCode().{{replacement}}` instead of `{{contextName}}.{{original}}`.',
messageId: 'newFormat',
data: {
contextName: contextId.name,
original: contextId.parent.property.name,
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-deprecated-report-api.js
Expand Up @@ -21,6 +21,9 @@ module.exports = {
},
fixable: 'code', // or "code" or "whitespace"
schema: [],
messages: {
useNewAPI: 'Use the new-style context.report() API.',
},
},

create (context) {
Expand All @@ -44,7 +47,7 @@ module.exports = {
) {
context.report({
node: node.callee.property,
message: 'Use the new-style context.report() API.',
messageId: 'useNewAPI',
fix (fixer) {
const openingParen = sourceCode.getTokenBefore(node.arguments[0]);
const closingParen = sourceCode.getLastToken(node);
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-missing-placeholders.js
Expand Up @@ -22,6 +22,9 @@ module.exports = {
},
fixable: null,
schema: [],
messages: {
placeholderDoesNotExist: 'The placeholder {{{{missingKey}}}} does not exist.',
},
},

create (context) {
Expand Down Expand Up @@ -66,7 +69,7 @@ module.exports = {
if (!matchingProperty) {
context.report({
node: reportInfo.message,
message: 'The placeholder {{{{missingKey}}}} does not exist.',
messageId: 'placeholderDoesNotExist',
data: { missingKey: match[1] },
});
}
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-unused-placeholders.js
Expand Up @@ -22,6 +22,9 @@ module.exports = {
},
fixable: null,
schema: [],
messages: {
placeholderUnused: 'The placeholder {{{{unusedKey}}}} is unused.',
},
},

create (context) {
Expand Down Expand Up @@ -69,7 +72,7 @@ module.exports = {
if (!placeholdersInMessage.has(key)) {
context.report({
node: reportInfo.message,
message: 'The placeholder {{{{unusedKey}}}} is unused.',
messageId: 'placeholderUnused',
data: { unusedKey: key },
});
}
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-useless-token-range.js
Expand Up @@ -21,6 +21,9 @@ module.exports = {
},
fixable: 'code',
schema: [],
messages: {
useReplacement: "Use '{{replacementText}}' instead.",
},
},

create (context) {
Expand Down Expand Up @@ -125,7 +128,7 @@ module.exports = {
sourceCode.text.slice(identifier.parent.parent.range[1], fullRangeAccess.range[1]);
context.report({
node: identifier.parent.parent,
message: "Use '{{replacementText}}' instead.",
messageId: 'useReplacement',
data: { replacementText },
fix (fixer) {
return fixer.replaceText(identifier.parent.parent, sourceCode.getText(identifier.parent.parent.arguments[0]));
Expand Down
54 changes: 54 additions & 0 deletions lib/rules/prefer-message-ids.js
@@ -0,0 +1,54 @@
'use strict';

const utils = require('../utils');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'require using `messageId` instead of `message` to report rule violations',
category: 'Rules',
recommended: false,
},
fixable: null,
schema: [],
messages: {
foundMessage: 'Use `messageId` instead of `message`.',
},
},

create (context) {
let contextIdentifiers;

// ----------------------------------------------------------------------
// Public
// ----------------------------------------------------------------------

return {
Program (ast) {
contextIdentifiers = utils.getContextIdentifiers(context, ast);
},
CallExpression (node) {
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 || !reportInfo.message) {
return;
}

context.report({
node: reportInfo.message.parent,
messageId: 'foundMessage',
});
}
},
};
},
};
6 changes: 4 additions & 2 deletions lib/rules/prefer-output-null.js
Expand Up @@ -21,14 +21,16 @@ module.exports = {
},
fixable: 'code',
schema: [],
messages: {
useOutputNull: 'Use `output: null` to assert that a test case is not autofixed.',
},
},

create (context) {
// ----------------------------------------------------------------------
// Public
// ----------------------------------------------------------------------

const message = 'Use `output: null` to assert that a test case is not autofixed.';
const sourceCode = context.getSourceCode();

return {
Expand All @@ -54,7 +56,7 @@ module.exports = {
if (output && sourceCode.getText(output.value) === sourceCode.getText(code.value)) {
context.report({
node: output,
message,
messageId: 'useOutputNull',
fix: fixer => fixer.replaceText(output.value, 'null'),
});
}
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/prefer-placeholders.js
Expand Up @@ -22,6 +22,9 @@ module.exports = {
},
fixable: null,
schema: [],
messages: {
usePlaceholders: 'Use report message placeholders instead of string concatenation.',
},
},

create (context) {
Expand Down Expand Up @@ -80,7 +83,7 @@ module.exports = {
) {
context.report({
node: messageNode,
message: 'Use report message placeholders instead of string concatenation.',
messageId: 'usePlaceholders',
});
}
}
Expand Down
6 changes: 4 additions & 2 deletions lib/rules/prefer-replace-text.js
Expand Up @@ -21,11 +21,13 @@ module.exports = {
},
fixable: null,
schema: [],
messages: {
useReplaceText: 'Use replaceText instead of replaceTextRange.',
},
},

create (context) {
const sourceCode = context.getSourceCode();
const message = 'Use replaceText instead of replaceTextRange.';
let funcInfo = {
upper: null,
codePath: null,
Expand Down Expand Up @@ -74,7 +76,7 @@ module.exports = {
if (isIdenticalNodeRange) {
context.report({
node,
message,
messageId: 'useReplaceText',
});
}
}
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/report-message-format.js
Expand Up @@ -24,6 +24,9 @@ module.exports = {
schema: [
{ type: 'string' },
],
messages: {
noMatch: "Report message does not match the pattern '{{pattern}}'.",
},
},

create (context) {
Expand All @@ -44,7 +47,7 @@ module.exports = {
) {
context.report({
node: message,
message: "Report message does not match the pattern '{{pattern}}'.",
messageId: 'noMatch',
data: { pattern: context.options[0] || '' },
});
}
Expand Down
6 changes: 4 additions & 2 deletions lib/rules/test-case-property-ordering.js
Expand Up @@ -24,13 +24,15 @@ module.exports = {
type: 'array',
elements: { type: 'string' },
}],
messages: {
inconsistentOrder: 'The properties of a test case should be placed in a consistent order: [{{order}}].',
},
},

create (context) {
// ----------------------------------------------------------------------
// Public
// ----------------------------------------------------------------------
const message = 'The properties of a test case should be placed in a consistent order: [{{order}}].';
const order = context.options[0] || [
'filename',
'code',
Expand Down Expand Up @@ -63,7 +65,7 @@ module.exports = {

context.report({
node: properties[i],
message,
messageId: 'inconsistentOrder',
data: { order: orderMsg.join(', ') },
fix (fixer) {
return orderMsg.map((key, index) => {
Expand Down
5 changes: 4 additions & 1 deletion lib/rules/test-case-shorthand-strings.js
Expand Up @@ -21,6 +21,9 @@ module.exports = {
},
fixable: 'code',
schema: [{ enum: ['as-needed', 'never', 'consistent', 'consistent-as-needed'] }],
messages: {
useShorthand: 'Use {{preferred}} for this test case instead of {{actual}}.',
},
},

create (context) {
Expand Down Expand Up @@ -62,7 +65,7 @@ module.exports = {
}[shorthandOption]).forEach(badCaseInfo => {
context.report({
node: badCaseInfo.node,
message: 'Use {{preferred}} for this test case instead of {{actual}}.',
messageId: 'useShorthand',
data: {
preferred: badCaseInfo.shorthand ? 'an object' : 'a string',
actual: badCaseInfo.shorthand ? 'a string' : 'an object',
Expand Down

0 comments on commit 95021dd

Please sign in to comment.