feat: ignore introduced message placeholders

eslint · mdjermanovic · Feb 9, 2024 · Feb 2, 2024 · Feb 2, 2024 · Feb 5, 2024
commit a39edf77ea0ec4022be61f8a8f44203fc7360a07
@@ -316,6 +316,29 @@ function getMessagePlaceholders(message) {
     return Array.from(message.matchAll(matcher), ([, name]) => name.trim());
 }
 
+/**
+ * Returns the placeholders in the reported messages but
+ * only includes the placeholders available in the raw message if data is not specified or
+ * excludes introduced placeholders specified in the data values.
+ * @param {string} message The reported message
+ * @param {string} raw The raw message specified in the rule meta.messages
+ * @param {undefined|Record<unknown, unknown>} data The passed
+ * @returns {string[]} Missing placeholder names
+ */
+function getMissingMessagePlaceholders(message, raw, data) {
+    const existing = getMessagePlaceholders(message);
+
+    if (data === void 0) {
+        const available = getMessagePlaceholders(raw);
+
+        return existing.filter(name => available.includes(name));
+    }
+
+    const introduced = new Set(Object.values(data).flatMap(value => (typeof value === "string" ? getMessagePlaceholders(value) : [])));
+
+    return existing.filter(name => !introduced.has(name));
-    const existing = getMessagePlaceholders(message);
-
-    if (data === void 0) {
-        const available = getMessagePlaceholders(raw);
-
-        return existing.filter(name => available.includes(name));
-    }
-
-    const introduced = new Set(Object.values(data).flatMap(value => (typeof value === "string" ? getMessagePlaceholders(value) : [])));
-
-    return existing.filter(name => !introduced.has(name));
+    let existing = getMessagePlaceholders(message);
+
+    //  no potential placeholders were found in the reported message, so there's nothing further to check
+    if (existing.length === 0) {
+        return existing;
+    }
+
+    const original = getMessagePlaceholders(raw);
+
+    // filter out those that don't appear in the original template message as they're certainly not placeholders
+    existing = existing.filter(name => original.includes(name));
+
+    if (data) {
+        const provided = Object.keys(data);
+
+        // since the hydrated message texts are compared, we can ignore those for which the data is provided in the test
+        existing = existing.filter(name => !provided.includes(name));
+    }
+
+    return existing;
-    const existing = getMessagePlaceholders(message);
-
-    if (data === void 0) {
-        const available = getMessagePlaceholders(raw);
-
-        return existing.filter(name => available.includes(name));
-    }
-
-    const introduced = new Set(Object.values(data).flatMap(value => (typeof value === "string" ? getMessagePlaceholders(value) : [])));
-
-    return existing.filter(name => !introduced.has(name));
+    let existing = getMessagePlaceholders(message);
+
+    //  no potential placeholders were found in the reported message, so there's nothing further to check
+    if (existing.length === 0) {
+        return existing;
+    }
+
+    const original = getMessagePlaceholders(raw);
+
+    // filter out those that don't appear in the original template message as they're certainly not placeholders
+    existing = existing.filter(name => original.includes(name));
+
+    if (data) {
+        const provided = Object.keys(data);
+
+        // since the hydrated message texts are compared, we can ignore those for which the data is provided in the test
+        existing = existing.filter(name => !provided.includes(name));
+    }
+
+    return existing;
+}
+
 const metaSchemaDescription = `
 \t- If the rule has options, set \`meta.schema\` to an array or non-empty object to enable options validation.
 \t- If the rule doesn't have options, omit \`meta.schema\` to enforce that no options can be passed to the rule.
@@ -1010,7 +1033,7 @@ class RuleTester {
                                 `messageId '${message.messageId}' does not match expected messageId '${error.messageId}'.`
                             );
 
-                            const missingPlaceholders = getMessagePlaceholders(message.message);
+                            const missingPlaceholders = getMissingMessagePlaceholders(message.message, rule.meta.messages[message.messageId], error.data);
 
                             assert.ok(
                                 missingPlaceholders.length === 0,
@@ -1117,7 +1140,11 @@ class RuleTester {
                                                 `${suggestionPrefix} messageId should be '${expectedSuggestion.messageId}' but got '${actualSuggestion.messageId}' instead.`
                                             );
 
-                                            const missingPlaceholders = getMessagePlaceholders(actualSuggestion.desc);
+                                            const missingPlaceholders = getMissingMessagePlaceholders(
+                                                actualSuggestion.desc,
+                                                rule.meta.messages[expectedSuggestion.messageId],
+                                                expectedSuggestion.data
+                                            );
 
                                             assert.ok(
                                                 missingPlaceholders.length === 0,

@@ -98,3 +98,25 @@ module.exports.withPlaceholdersInData = {
         };
     }
 };
+
+module.exports.withSamePlaceholdersInData = {
+    meta: {
+        messages: {
+            avoidFoo: "Avoid using variables named '{{ name }}'.",
+            unused: "An unused key"
+        }
+    },
+    create(context) {
+        return {
+            Identifier(node) {
+                if (node.name === "foo") {
+                    context.report({
+                        node,
+                        messageId: "avoidFoo",
+                        data:      { name: '{{ name }}' },
+                    });
+                }
+            }
+        };
+    }
+};
@@ -1934,13 +1934,36 @@ describe("RuleTester", () => {
         }, "The reported message has the missing placeholders: 'type', 'name'. Please provide them via the 'data' property in the context.report() call.");
     });
 
-    it("should throw if the data in the message contains placeholders", () => {
+    it("should not throw if the data in the message contains placeholders not present in the raw message", () => {
+        ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withPlaceholdersInData, {
+            valid: [],
+            invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
+        });
+    });
+
+    it("should throw if the data in the message contains the same placeholder and data is not specified", () => {
         assert.throws(() => {
-            ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withPlaceholdersInData, {
+            ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withSamePlaceholdersInData, {
                 valid: [],
                 invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
             });
-        }, "The reported message has a missing placeholder 'placeholder'. Please provide them via the 'data' property in the context.report() call.");
+        }, "The reported message has a missing placeholder 'name'. Please provide them via the 'data' property in the context.report() call.");
+    });
+
+    it("should not throw if the data in the message contains the same placeholder and data is specified", () => {
+        ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withSamePlaceholdersInData, {
+            valid: [],
+            invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "{{ name }}" } }] }]
+        });
+    });
+
+    it("should throw an error for only specifying ignored non-string data property values", () => {
+        assert.throws(() => {
+            ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withSamePlaceholdersInData, {
+                valid: [],
+                invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: 0 } }] }]
+            });
+        }, "The reported message has a missing placeholder 'name'. Please provide them via the 'data' property in the context.report() call.");
     });
 
     // messageId/message misconfiguration cases