Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9163646

Browse files
feat!: Rule Tester checks for missing placeholder data in the message (#18073)
* feat!: rule tester checks for missing placeholder data for the reported message* chore: incorporate bmishs suggestions* chore: differentiate message between a single and multiple missing data properties* fix: check for missing placeholders with data specified* feat: share regular expression for message placeholders* feat: ignore introduced message placeholders* refactor: simplified logic for getting unsubstituted message placeholders* docs: also use term unsubstituted for migration guideCo-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>* docs: clarify placeholder versus dataCo-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>* chore: message grammar fixesCo-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>* chore: update error messages for the grammar fixes* fix: remove unnecessary check for added placeholders* chore: split up object to avoid referencing the placeholder matcher via module.exports* chore: add mention of the issue for the migration guide* chore: stylize rule tester in migration guide* chore: clarify message for unsubstituted placeholders and introducefixture for non-string data values---------Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
1 parent53f0f47 commit9163646

File tree

9 files changed

+393
-7
lines changed

9 files changed

+393
-7
lines changed

‎docs/src/use/migrate-to-9.0.0.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,10 +553,11 @@ In order to aid in the development of high-quality custom rules that are free fr
553553
1. **Suggestions must generate valid syntax.** In order for rule suggestions to be helpful, they need to be valid syntax. `RuleTester` now parses the output of suggestions using the same language options as the `code` value and throws an error if parsing fails.
554554
1. **Test cases must be unique.** Identical test cases can cause confusion and be hard to detect manually in a long test file. Duplicates are now automatically detected and can be safely removed.
555555
1. **`filename` and `only` must be of the expected type.** `RuleTester` now checks the type of `filename` and `only` properties of test objects. If specified, `filename` must be a string value. If specified, `only` must be a boolean value.
556+
1. **Messages cannot have unsubstituted placeholders.** The `RuleTester` now also checks if there are {% raw %}`{{ placeholder }}` {% endraw %} still in the message as their values were not passed via `data` in the respective `context.report()` call.
556557
557558
**To address:** Run your rule tests using `RuleTester` and fix any errors that occur. The changes you'll need to make to satisfy`RuleTester` are compatiblewith ESLintv8.x.
558559

559-
**RelatedIssues(s):** [#15104](https://github.com/eslint/eslint/issues/15104), [#15735](https://github.com/eslint/eslint/issues/15735), [#16908](https://github.com/eslint/eslint/issues/16908)
560+
**RelatedIssues(s):** [#15104](https://github.com/eslint/eslint/issues/15104), [#15735](https://github.com/eslint/eslint/issues/15735), [#16908](https://github.com/eslint/eslint/issues/16908), [#18016](https://github.com/eslint/eslint/issues/18016)
560561

561562
##<a name="flat-eslint"></a>`FlatESLint` is now`ESLint`
562563

‎lib/linter/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"use strict";
22

33
const{ Linter}=require("./linter");
4-
constinterpolate=require("./interpolate");
4+
const{interpolate}=require("./interpolate");
55
constSourceCodeFixer=require("./source-code-fixer");
66

77
module.exports={

‎lib/linter/interpolate.js‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,30 @@
99
// Public Interface
1010
//------------------------------------------------------------------------------
1111

12-
module.exports=(text,data)=>{
12+
/**
13+
* Returns a global expression matching placeholders in messages.
14+
*@returns {RegExp} Global regular expression matching placeholders
15+
*/
16+
functiongetPlaceholderMatcher(){
17+
return/\{\{([^{}]+?)\}\}/gu;
18+
}
19+
20+
/**
21+
* Replaces {{ placeholders }} in the message with the provided data.
22+
* Does not replace placeholders not available in the data.
23+
*@param {string} text Original message with potential placeholders
24+
*@param {Record<string, string>} data Map of placeholder name to its value
25+
*@returns {string} Message with replaced placeholders
26+
*/
27+
functioninterpolate(text,data){
1328
if(!data){
1429
returntext;
1530
}
1631

32+
constmatcher=getPlaceholderMatcher();
33+
1734
// Substitution content for any {{ }} markers.
18-
returntext.replace(/\{\{([^{}]+?)\}\}/gu,(fullMatch,termWithWhitespace)=>{
35+
returntext.replace(matcher,(fullMatch,termWithWhitespace)=>{
1936
constterm=termWithWhitespace.trim();
2037

2138
if(termindata){
@@ -25,4 +42,9 @@ module.exports = (text, data) => {
2542
// Preserve old behavior: If parameter name not provided, don't replace it.
2643
returnfullMatch;
2744
});
45+
}
46+
47+
module.exports={
48+
getPlaceholderMatcher,
49+
interpolate
2850
};

‎lib/linter/report-translator.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
constassert=require("assert");
1313
construleFixer=require("./rule-fixer");
14-
constinterpolate=require("./interpolate");
14+
const{interpolate}=require("./interpolate");
1515

1616
//------------------------------------------------------------------------------
1717
// Typedefs

‎lib/rule-tester/rule-tester.js‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ const
1717
equal=require("fast-deep-equal"),
1818
Traverser=require("../shared/traverser"),
1919
{ getRuleOptionsSchema}=require("../config/flat-config-helpers"),
20-
{ Linter, SourceCodeFixer, interpolate}=require("../linter"),
20+
{ Linter, SourceCodeFixer}=require("../linter"),
21+
{ interpolate, getPlaceholderMatcher}=require("../linter/interpolate"),
2122
stringify=require("json-stable-stringify-without-jsonify");
2223

2324
const{ FlatConfigArray}=require("../config/flat-config-array");
@@ -304,6 +305,39 @@ function throwForbiddenMethodError(methodName, prototype) {
304305
};
305306
}
306307

308+
/**
309+
* Extracts names of {{ placeholders }} from the reported message.
310+
*@param {string} message Reported message
311+
*@returns {string[]} Array of placeholder names
312+
*/
313+
functiongetMessagePlaceholders(message){
314+
constmatcher=getPlaceholderMatcher();
315+
316+
returnArray.from(message.matchAll(matcher),([,name])=>name.trim());
317+
}
318+
319+
/**
320+
* Returns the placeholders in the reported messages but
321+
* only includes the placeholders available in the raw message and not in the provided data.
322+
*@param {string} message The reported message
323+
*@param {string} raw The raw message specified in the rule meta.messages
324+
*@param {undefined|Record<unknown, unknown>} data The passed
325+
*@returns {string[]} Missing placeholder names
326+
*/
327+
functiongetUnsubstitutedMessagePlaceholders(message,raw,data={}){
328+
constunsubstituted=getMessagePlaceholders(message);
329+
330+
if(unsubstituted.length===0){
331+
return[];
332+
}
333+
334+
// Remove false positives by only counting placeholders in the raw message, which were not provided in the data matcher or added with a data property
335+
constknown=getMessagePlaceholders(raw);
336+
constprovided=Object.keys(data);
337+
338+
returnunsubstituted.filter(name=>known.includes(name)&&!provided.includes(name));
339+
}
340+
307341
constmetaSchemaDescription=`
308342
\t- If the rule has options, set \`meta.schema\` to an array or non-empty object to enable options validation.
309343
\t- If the rule doesn't have options, omit \`meta.schema\` to enforce that no options can be passed to the rule.
@@ -997,6 +1031,18 @@ class RuleTester {
9971031
error.messageId,
9981032
`messageId '${message.messageId}' does not match expected messageId '${error.messageId}'.`
9991033
);
1034+
1035+
constunsubstitutedPlaceholders=getUnsubstitutedMessagePlaceholders(
1036+
message.message,
1037+
rule.meta.messages[message.messageId],
1038+
error.data
1039+
);
1040+
1041+
assert.ok(
1042+
unsubstitutedPlaceholders.length===0,
1043+
`The reported message has${unsubstitutedPlaceholders.length>1 ?`unsubstituted placeholders:${unsubstitutedPlaceholders.map(name=>`'${name}'`).join(", ")}` :`an unsubstituted placeholder '${unsubstitutedPlaceholders[0]}'`}. Please provide the missing${unsubstitutedPlaceholders.length>1 ?"values" :"value"} via the 'data' property in the context.report() call.`
1044+
);
1045+
10001046
if(hasOwnProperty(error,"data")){
10011047

10021048
/*
@@ -1096,6 +1142,18 @@ class RuleTester {
10961142
expectedSuggestion.messageId,
10971143
`${suggestionPrefix} messageId should be '${expectedSuggestion.messageId}' but got '${actualSuggestion.messageId}' instead.`
10981144
);
1145+
1146+
constunsubstitutedPlaceholders=getUnsubstitutedMessagePlaceholders(
1147+
actualSuggestion.desc,
1148+
rule.meta.messages[expectedSuggestion.messageId],
1149+
expectedSuggestion.data
1150+
);
1151+
1152+
assert.ok(
1153+
unsubstitutedPlaceholders.length===0,
1154+
`The message of the suggestion has${unsubstitutedPlaceholders.length>1 ?`unsubstituted placeholders:${unsubstitutedPlaceholders.map(name=>`'${name}'`).join(", ")}` :`an unsubstituted placeholder '${unsubstitutedPlaceholders[0]}'`}. Please provide the missing${unsubstitutedPlaceholders.length>1 ?"values" :"value"} via the 'data' property for the suggestion in the context.report() call.`
1155+
);
1156+
10991157
if(hasOwnProperty(expectedSuggestion,"data")){
11001158
constunformattedMetaMessage=rule.meta.messages[expectedSuggestion.messageId];
11011159
constrehydratedDesc=interpolate(unformattedMetaMessage,expectedSuggestion.data);

‎tests/fixtures/testers/rule-tester/messageId.js‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,110 @@ module.exports.withMessageOnly = {
3434
};
3535
}
3636
};
37+
38+
module.exports.withMissingData={
39+
meta:{
40+
messages:{
41+
avoidFoo:"Avoid using variables named '{{ name }}'.",
42+
unused:"An unused key"
43+
}
44+
},
45+
create(context){
46+
return{
47+
Identifier(node){
48+
if(node.name==="foo"){
49+
context.report({
50+
node,
51+
messageId:"avoidFoo",
52+
});
53+
}
54+
}
55+
};
56+
}
57+
};
58+
59+
module.exports.withMultipleMissingDataProperties={
60+
meta:{
61+
messages:{
62+
avoidFoo:"Avoid using {{ type }} named '{{ name }}'.",
63+
unused:"An unused key"
64+
}
65+
},
66+
create(context){
67+
return{
68+
Identifier(node){
69+
if(node.name==="foo"){
70+
context.report({
71+
node,
72+
messageId:"avoidFoo",
73+
});
74+
}
75+
}
76+
};
77+
}
78+
};
79+
80+
module.exports.withPlaceholdersInData={
81+
meta:{
82+
messages:{
83+
avoidFoo:"Avoid using variables named '{{ name }}'.",
84+
unused:"An unused key"
85+
}
86+
},
87+
create(context){
88+
return{
89+
Identifier(node){
90+
if(node.name==="foo"){
91+
context.report({
92+
node,
93+
messageId:"avoidFoo",
94+
data:{name:'{{ placeholder }}'},
95+
});
96+
}
97+
}
98+
};
99+
}
100+
};
101+
102+
module.exports.withSamePlaceholdersInData={
103+
meta:{
104+
messages:{
105+
avoidFoo:"Avoid using variables named '{{ name }}'.",
106+
unused:"An unused key"
107+
}
108+
},
109+
create(context){
110+
return{
111+
Identifier(node){
112+
if(node.name==="foo"){
113+
context.report({
114+
node,
115+
messageId:"avoidFoo",
116+
data:{name:'{{ name }}'},
117+
});
118+
}
119+
}
120+
};
121+
}
122+
};
123+
124+
module.exports.withNonStringData={
125+
meta:{
126+
messages:{
127+
avoid:"Avoid using the value '{{ value }}'.",
128+
}
129+
},
130+
create(context){
131+
return{
132+
Literal(node){
133+
if(node.value===0){
134+
context.report({
135+
node,
136+
messageId:"avoid",
137+
data:{value:0},
138+
});
139+
}
140+
}
141+
};
142+
}
143+
};

‎tests/fixtures/testers/rule-tester/suggestions.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,3 +198,61 @@ module.exports.withFailingFixer = {
198198
};
199199
}
200200
};
201+
202+
module.exports.withMissingPlaceholderData={
203+
meta:{
204+
messages:{
205+
avoidFoo:"Avoid using identifiers named '{{ name }}'.",
206+
renameFoo:"Rename identifier 'foo' to '{{ newName }}'"
207+
},
208+
hasSuggestions:true
209+
},
210+
create(context){
211+
return{
212+
Identifier(node){
213+
if(node.name==="foo"){
214+
context.report({
215+
node,
216+
messageId:"avoidFoo",
217+
data:{
218+
name:"foo"
219+
},
220+
suggest:[{
221+
messageId:"renameFoo",
222+
fix:fixer=>fixer.replaceText(node,"bar")
223+
}]
224+
});
225+
}
226+
}
227+
};
228+
}
229+
};
230+
231+
module.exports.withMultipleMissingPlaceholderDataProperties={
232+
meta:{
233+
messages:{
234+
avoidFoo:"Avoid using identifiers named '{{ name }}'.",
235+
rename:"Rename identifier '{{ currentName }}' to '{{ newName }}'"
236+
},
237+
hasSuggestions:true
238+
},
239+
create(context){
240+
return{
241+
Identifier(node){
242+
if(node.name==="foo"){
243+
context.report({
244+
node,
245+
messageId:"avoidFoo",
246+
data:{
247+
name:"foo"
248+
},
249+
suggest:[{
250+
messageId:"rename",
251+
fix:fixer=>fixer.replaceText(node,"bar")
252+
}]
253+
});
254+
}
255+
}
256+
};
257+
}
258+
};

‎tests/lib/linter/interpolate.js‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,40 @@
55
//------------------------------------------------------------------------------
66

77
constassert=require("chai").assert;
8-
constinterpolate=require("../../../lib/linter/interpolate");
8+
const{ getPlaceholderMatcher,interpolate}=require("../../../lib/linter/interpolate");
99

1010
//------------------------------------------------------------------------------
1111
// Tests
1212
//------------------------------------------------------------------------------
1313

14+
describe("getPlaceholderMatcher",()=>{
15+
it("returns a global regular expression",()=>{
16+
constmatcher=getPlaceholderMatcher();
17+
18+
assert.strictEqual(matcher.global,true);
19+
});
20+
21+
it("matches text with placeholders",()=>{
22+
constmatcher=getPlaceholderMatcher();
23+
24+
assert.match("{{ placeholder }}",matcher);
25+
});
26+
27+
it("does not match text without placeholders",()=>{
28+
constmatcher=getPlaceholderMatcher();
29+
30+
assert.notMatch("no placeholders in sight",matcher);
31+
});
32+
33+
it("captures the text inside the placeholder",()=>{
34+
constmatcher=getPlaceholderMatcher();
35+
consttext="{{ placeholder }}";
36+
constmatches=Array.from(text.matchAll(matcher));
37+
38+
assert.deepStrictEqual(matches,[[text," placeholder "]]);
39+
});
40+
});
41+
1442
describe("interpolate()",()=>{
1543
it("passes through text without {{ }}",()=>{
1644
constmessage="This is a very important message!";

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp