Skip to content

Commit a995733

Browse files
not-an-aardvarknot-an-aardvark
committed
New: no-missing-placeholders rule
1 parent f29aa27 commit a995733

File tree

4 files changed

+385
-3
lines changed

4 files changed

+385
-3
lines changed

Diff for: ‎README.md

+6-3
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ Additionally, you can enable all recommended rules from this plugin:
5656
✔️ indicates that a rule is recommended.
5757
🛠 indicates that a rule is fixable.
5858

59-
* ✔️ 🛠 [no-deprecated-report-api](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-deprecated-report-api.md): Prohibits the deprecated `context.report(node, message)` API
60-
* [report-message-format](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/report-message-format.md): Enforces a consistent format for report messages
61-
* ✔️ [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
59+
Name | ✔️ | 🛠 | Description
60+
----- | ----- | ----- | -----
61+
[no-deprecated-report-api](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-deprecated-report-api.md) | ✔️ | 🛠 | Prohibits the deprecated `context.report(node, message)` API
62+
[report-message-format](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/report-message-format.md) | ✔️ | | Enforces a consistent format for report messages
63+
[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
64+
[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

Diff for: ‎docs/rules/no-missing-placeholders.md

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Disallow missing placeholders in rule report messages (no-missing-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+
// Resulting message: e.g. 'IfStatement nodes are not allowed.'
13+
```
14+
15+
However, if no `data` argument is provided, or no matching replacement key exists in the `data` argument, the raw curly brackets will end up in the report message. This is usually a mistake.
16+
17+
18+
## Rule Details
19+
20+
This rule aims to disallow missing placeholders in rule report messages.
21+
22+
Examples of **incorrect** code for this rule:
23+
24+
```js
25+
/*eslint eslint-plugin/no-missing-placeholders: error*/
26+
27+
module.exports = {
28+
create(context) {
29+
context.report({
30+
node,
31+
message: '{{something}} is wrong.'
32+
});
33+
34+
context.report({
35+
node,
36+
message: '{{something}} is wrong.',
37+
data: { somethingElse: 'foo' }
38+
});
39+
40+
context.report(node, '{{something}} is wrong.', { somethingElse: 'foo' });
41+
}
42+
};
43+
44+
```
45+
46+
Examples of **correct** code for this rule:
47+
48+
```js
49+
/*eslint eslint-plugin/no-missing-placeholders: error*/
50+
51+
module.exports = {
52+
create(context) {
53+
context.report({
54+
node,
55+
message: 'something is wrong.'
56+
});
57+
58+
context.report({
59+
node,
60+
message: '{{something}} is wrong.',
61+
data: { something: 'foo' }
62+
});
63+
64+
context.report(node, '{{something}} is wrong.', { something: 'foo' });
65+
}
66+
};
67+
68+
```
69+
70+
71+
## When Not To Use It
72+
73+
If you want to use rule messages that actually contain double-curly bracket text, you should turn off this rule.
74+
75+
## Further Reading
76+
77+
* [context.report() API](http://eslint.org/docs/developer-guide/working-with-rules#contextreport)

Diff for: ‎lib/rules/no-missing-placeholders.js

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* @fileoverview Disallow missing placeholders in rule 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+
/**
15+
* Gets report info for a context.report() call
16+
* TODO: Combine this with logic from no-deprecated-report-api
17+
* @param {ASTNode[]} reportArgs The arguments to the context.report() call
18+
* @returns {object}
19+
*/
20+
function getReportInfo (reportArgs) {
21+
if (reportArgs.length === 1) {
22+
if (reportArgs[0].type === 'ObjectExpression') {
23+
const messageProp = reportArgs[0].properties.find(prop => utils.getKeyName(prop) === 'message');
24+
const dataProp = reportArgs[0].properties.find(prop => utils.getKeyName(prop) === 'data');
25+
26+
return {
27+
message: messageProp && messageProp.value,
28+
data: dataProp && dataProp.value,
29+
};
30+
}
31+
} else if (reportArgs.length) {
32+
if (reportArgs[1].type === 'Literal' && typeof reportArgs[1].value === 'string') {
33+
return {
34+
message: reportArgs[1],
35+
data: reportArgs[2],
36+
};
37+
} else if (
38+
reportArgs[1].type === 'ObjectExpression' ||
39+
reportArgs[1].type === 'ArrayExpression' ||
40+
(reportArgs[1].type === 'Literal' && typeof reportArgs[1].value !== 'string')
41+
) {
42+
return {
43+
message: reportArgs[2],
44+
data: reportArgs[3],
45+
};
46+
}
47+
}
48+
return null;
49+
}
50+
51+
module.exports = {
52+
meta: {
53+
docs: {
54+
description: 'Disallow missing placeholders in rule report messages',
55+
category: 'Rules',
56+
recommended: false,
57+
},
58+
fixable: null,
59+
schema: [],
60+
},
61+
62+
create (context) {
63+
let contextIdentifiers;
64+
65+
// ----------------------------------------------------------------------
66+
// Public
67+
// ----------------------------------------------------------------------
68+
69+
return {
70+
Program (ast) {
71+
contextIdentifiers = utils.getContextIdentifiers(context, ast);
72+
},
73+
CallExpression (node) {
74+
if (
75+
node.callee.type === 'MemberExpression' &&
76+
contextIdentifiers.has(node.callee.object) &&
77+
node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
78+
) {
79+
const reportInfo = getReportInfo(node.arguments);
80+
81+
if (
82+
reportInfo &&
83+
reportInfo.message &&
84+
reportInfo.message.type === 'Literal' &&
85+
typeof reportInfo.message.value === 'string' &&
86+
(!reportInfo.data || reportInfo.data.type === 'ObjectExpression')
87+
) {
88+
// Same regex as the one ESLint uses
89+
// https://github.com/eslint/eslint/blob/e5446449d93668ccbdb79d78cc69f165ce4fde07/lib/eslint.js#L990
90+
const PLACEHOLDER_MATCHER = /\{\{\s*([^{}]+?)\s*\}\}/g;
91+
let match;
92+
93+
while ((match = PLACEHOLDER_MATCHER.exec(reportInfo.message.value))) { // eslint-disable-line no-extra-parens
94+
const matchingProperty = reportInfo.data &&
95+
reportInfo.data.properties.find(prop => utils.getKeyName(prop) === match[1]);
96+
97+
if (!matchingProperty) {
98+
context.report({
99+
node: reportInfo.message,
100+
message: 'The placeholder {{{{missingKey}}}} does not exist.',
101+
data: { missingKey: match[1] },
102+
});
103+
}
104+
}
105+
}
106+
}
107+
},
108+
};
109+
},
110+
};

Diff for: ‎tests/lib/rules/no-missing-placeholders.js

+192
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/**
2+
* @fileoverview Disallow missing placeholders in rule report messages
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
// ------------------------------------------------------------------------------
9+
// Requirements
10+
// ------------------------------------------------------------------------------
11+
12+
const rule = require('../../../lib/rules/no-missing-placeholders');
13+
const RuleTester = require('eslint').RuleTester;
14+
15+
/**
16+
* Create an error for the given key
17+
* @param {string} missingKey The placeholder that is missing
18+
* @returns {object} An expected error
19+
*/
20+
function error (missingKey) {
21+
return { type: 'Literal', message: `The placeholder {{${missingKey}}} does not exist.` };
22+
}
23+
24+
// ------------------------------------------------------------------------------
25+
// Tests
26+
// ------------------------------------------------------------------------------
27+
28+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
29+
ruleTester.run('no-missing-placeholders', rule, {
30+
31+
valid: [
32+
`
33+
module.exports = {
34+
create(context) {
35+
context.report({
36+
node,
37+
message: 'foo bar'
38+
});
39+
}
40+
};
41+
`,
42+
`
43+
module.exports = {
44+
create(context) {
45+
context.report({
46+
node,
47+
message: 'foo {{bar}}',
48+
data: { bar: 'baz' }
49+
});
50+
}
51+
};
52+
`,
53+
`
54+
module.exports = {
55+
create(context) {
56+
context.report({
57+
node,
58+
message: 'foo {{bar}}',
59+
data: { 'bar': 'baz' }
60+
});
61+
}
62+
};
63+
`,
64+
`
65+
module.exports = {
66+
create(context) {
67+
context.report({
68+
node,
69+
message: 'foo {{bar}}',
70+
data: { bar }
71+
});
72+
}
73+
};
74+
`,
75+
`
76+
module.exports = {
77+
create(context) {
78+
context.report({
79+
node,
80+
message: 'foo{{bar}}' + baz
81+
});
82+
}
83+
};
84+
`,
85+
`
86+
module.exports = context => {
87+
context.report(node, 'foo {{bar}}', { bar: 'baz' });
88+
};
89+
`,
90+
`
91+
module.exports = context => {
92+
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' });
93+
};
94+
`,
95+
`
96+
module.exports = {
97+
create(context) {
98+
context.report({
99+
node,
100+
message: 'foo{{ bar }}',
101+
data: { bar: 'baz' }
102+
});
103+
}
104+
};
105+
`,
106+
`
107+
module.exports = {
108+
create(context) {
109+
context.report({
110+
node,
111+
message: 'foo{{ bar }}',
112+
data: baz
113+
});
114+
}
115+
};
116+
`,
117+
],
118+
119+
invalid: [
120+
{
121+
code: `
122+
module.exports = {
123+
create(context) {
124+
context.report({
125+
node,
126+
message: 'foo {{bar}}'
127+
});
128+
}
129+
};
130+
`,
131+
errors: [error('bar')],
132+
},
133+
{
134+
code: `
135+
module.exports = {
136+
create(context) {
137+
context.report({
138+
node,
139+
message: 'foo {{bar}}',
140+
data: { baz: 'qux' }
141+
});
142+
}
143+
};
144+
`,
145+
errors: [error('bar')],
146+
},
147+
{
148+
code: `
149+
module.exports = {
150+
create(context) {
151+
context.report({
152+
node,
153+
message: 'foo {{hasOwnProperty}}',
154+
data: {}
155+
});
156+
}
157+
};
158+
`,
159+
errors: [error('hasOwnProperty')],
160+
},
161+
{
162+
code: `
163+
module.exports = context => {
164+
context.report(node, 'foo {{bar}}', { baz: 'qux' });
165+
};
166+
`,
167+
errors: [error('bar')],
168+
},
169+
{
170+
code: `
171+
module.exports = context => {
172+
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' });
173+
};
174+
`,
175+
errors: [error('bar')],
176+
},
177+
{
178+
code: `
179+
module.exports = {
180+
create(context) {
181+
context.report({
182+
node,
183+
message: 'foo{{ bar }}',
184+
data: { ' bar ': 'baz' }
185+
});
186+
}
187+
};
188+
`,
189+
errors: [error('bar')],
190+
},
191+
],
192+
});

0 commit comments

Comments
 (0)