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

New: Add new rule prefer-message-ids #170

Merged
merged 1 commit into from Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
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