Skip to content

Commit de03394

Browse files
New: prefer-placeholders rule
1 parent d14cd05 commit de03394

File tree

4 files changed

+227
-0
lines changed

4 files changed

+227
-0
lines changed

Diff for: README.md

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ Name | ✔️ | 🛠 | Description
5353
[require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) | ✔️ | | Requires a `meta.fixable` property for fixable rules
5454
[no-missing-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-placeholders.md) | ✔️ | | Disallows missing placeholders in rule report messages
5555
[test-case-shorthand-strings](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/test-case-shorthand-strings.md) | | | Enforces consistent usage of shorthand strings for test cases with no options
56+
[prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | Disallows template literals as report messages
5657

5758
## Supported Presets
5859

Diff for: docs/rules/prefer-placeholders.md

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# disallow template literals as report messages (prefer-placeholders)
2+
3+
Report messages in rules can have placeholders surrounded by curly brackets.
4+
5+
```js
6+
context.report({
7+
node,
8+
message: '{{disallowedNode}} nodes are not allowed.',
9+
data: { disallowedNode: node.type }
10+
});
11+
```
12+
13+
Using placeholders is often preferred over using dynamic report messages, for a few reasons:
14+
15+
* They can help enforce a separation of the message and the data.
16+
* It will be easier to migrate when ESLint starts supporting placing lint messages in metadata (see [#6740](https://github.com/eslint/eslint/issues/6740))
17+
18+
## Rule Details
19+
20+
This rule aims to report string concatenation and template literals in report messages.
21+
22+
Examples of **incorrect** code for this rule:
23+
24+
```js
25+
/* eslint eslint-plugin/prefer-placeholders: error */
26+
27+
module.exports = {
28+
create(context) {
29+
context.report({
30+
node,
31+
message: `The node ${node.name} is not allowed to be used.`
32+
});
33+
34+
context.report({
35+
node,
36+
message: 'The node ' + node.name + ' is not allowed to be used.'
37+
});
38+
}
39+
};
40+
```
41+
42+
Examples of **correct** code for this rule:
43+
44+
```js
45+
/* eslint eslint-plugin/prefer-placeholders: error */
46+
47+
module.exports = {
48+
create(context) {
49+
context.report({
50+
node,
51+
message: 'The node {{name}} is not allowed to be used.',
52+
data: { name: node.name }
53+
});
54+
}
55+
};
56+
```
57+
58+
## When Not To Use It
59+
60+
If you need to use string concatenation in your report messages for some reason, don't turn on this rule.
61+
62+
## Further Reading
63+
64+
* [context.report() API](http://eslint.org/docs/developer-guide/working-with-rules#contextreport)

Diff for: lib/rules/prefer-placeholders.js

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @fileoverview disallow template literals as report messages
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
const utils = require('../utils');
9+
10+
// ------------------------------------------------------------------------------
11+
// Rule Definition
12+
// ------------------------------------------------------------------------------
13+
14+
module.exports = {
15+
meta: {
16+
docs: {
17+
description: 'disallow template literals as report messages',
18+
category: 'Rules',
19+
recommended: false,
20+
},
21+
fixable: null,
22+
schema: [],
23+
},
24+
25+
create (context) {
26+
let contextIdentifiers;
27+
28+
// ----------------------------------------------------------------------
29+
// Public
30+
// ----------------------------------------------------------------------
31+
32+
return {
33+
Program (node) {
34+
contextIdentifiers = utils.getContextIdentifiers(context, node);
35+
},
36+
CallExpression (node) {
37+
if (
38+
node.callee.type === 'MemberExpression' &&
39+
contextIdentifiers.has(node.callee.object) &&
40+
node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
41+
) {
42+
const reportInfo = utils.getReportInfo(node.arguments);
43+
44+
if (
45+
reportInfo && reportInfo.message && (
46+
(reportInfo.message.type === 'TemplateLiteral' && reportInfo.message.expressions.length) ||
47+
(reportInfo.message.type === 'BinaryExpression' && reportInfo.message.operator === '+')
48+
)
49+
) {
50+
context.report({
51+
node: reportInfo.message,
52+
message: 'Use report message placeholders instead of string concatenation.',
53+
});
54+
}
55+
}
56+
},
57+
};
58+
},
59+
};

Diff for: tests/lib/rules/prefer-placeholders.js

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* @fileoverview disallow template literals as report messages
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
// ------------------------------------------------------------------------------
9+
// Requirements
10+
// ------------------------------------------------------------------------------
11+
12+
const rule = require('../../../lib/rules/prefer-placeholders');
13+
const RuleTester = require('eslint').RuleTester;
14+
15+
// ------------------------------------------------------------------------------
16+
// Tests
17+
// ------------------------------------------------------------------------------
18+
19+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
20+
const ERROR = { message: 'Use report message placeholders instead of string concatenation.' };
21+
22+
23+
ruleTester.run('prefer-placeholders', rule, {
24+
valid: [
25+
`
26+
module.exports = {
27+
create(context) {
28+
context.report({
29+
node,
30+
message: '{{foo}} is bad.',
31+
data: { foo },
32+
});
33+
}
34+
};
35+
`,
36+
`
37+
module.exports = {
38+
create(context) {
39+
context.report({
40+
node,
41+
message: 'foo is bad.'
42+
});
43+
}
44+
};
45+
`,
46+
`
47+
module.exports = {
48+
create(context) {
49+
context.report({
50+
node,
51+
message: foo
52+
});
53+
}
54+
};
55+
`,
56+
`
57+
module.exports = {
58+
create(context) {
59+
context.report(node, 'foo is bad.');
60+
}
61+
};
62+
`,
63+
],
64+
65+
invalid: [
66+
{
67+
code: `
68+
module.exports = {
69+
create(context) {
70+
context.report({
71+
node,
72+
message: \`\${foo} is bad.\`
73+
});
74+
}
75+
};
76+
`,
77+
errors: [ERROR],
78+
},
79+
{
80+
code: `
81+
module.exports = {
82+
create(context) {
83+
context.report({
84+
node,
85+
message: foo + ' is bad.'
86+
});
87+
}
88+
};
89+
`,
90+
errors: [ERROR],
91+
},
92+
{
93+
code: `
94+
module.exports = {
95+
create(context) {
96+
context.report(node, \`\${foo} is bad.\`);
97+
}
98+
};
99+
`,
100+
errors: [ERROR],
101+
},
102+
],
103+
});

0 commit comments

Comments
 (0)