feat(rule-tester): check for missing placeholder data in the message

packages/rule-tester/src/RuleTester.ts

@@ -38,7 +38,7 @@ import { satisfiesAllDependencyConstraints } from './utils/dependencyConstraints
 import { freezeDeeply } from './utils/freezeDeeply';
 import { getRuleOptionsSchema } from './utils/getRuleOptionsSchema';
 import { hasOwnProperty } from './utils/hasOwnProperty';
-import { interpolate } from './utils/interpolate';
+import { getPlaceholderMatcher, interpolate } from './utils/interpolate';
 import { isReadonlyArray } from './utils/isReadonlyArray';
 import * as SourceCodeFixer from './utils/SourceCodeFixer';
 import {
@@ -73,6 +73,45 @@ let defaultConfig = deepMerge(
   testerDefaultConfig,
 ) as TesterConfigWithDefaults;
 
+/**
+ * Extracts names of {{ placeholders }} from the reported message.
+ * @param message Reported message
+ * @returns Array of placeholder names
+ */
+function getMessagePlaceholders(message: string): string[] {
+  const matcher = getPlaceholderMatcher();
+
+  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 and not in the provided data.
+ * @param message The reported message
+ * @param raw The raw message specified in the rule meta.messages
+ * @param data The passed
+ * @returns Missing placeholder names
+ */
+function getUnsubstitutedMessagePlaceholders(
+  message: string,
+  raw: string,
+  data: Record<string, unknown> = {},
+): string[] {
+  const unsubstituted = getMessagePlaceholders(message);
+
+  if (unsubstituted.length === 0) {
+    return [];
+  }
+
+  // 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
+  const known = getMessagePlaceholders(raw);
+  const provided = Object.keys(data);
+
+  return unsubstituted.filter(
+    name => known.includes(name) && !provided.includes(name),
+  );
+}
+
 export class RuleTester extends TestFramework {
   readonly #testerConfig: TesterConfigWithDefaults;
   readonly #rules: Record<string, AnyRuleCreateFunction | AnyRuleModule> = {};
@@ -809,6 +848,19 @@ export class RuleTester extends TestFramework {
               error.messageId,
               `messageId '${message.messageId}' does not match expected messageId '${error.messageId}'.`,
             );
+
+            const unsubstitutedPlaceholders =
+              getUnsubstitutedMessagePlaceholders(
+                message.message,
+                rule.meta.messages[message.messageId],
+                error.data,
+              );
+
+            assert.ok(
+              unsubstitutedPlaceholders.length === 0,
+              `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.`,
+            );
+
             if (hasOwnProperty(error, 'data')) {
               /*
                *  if data was provided, then directly compare the returned message to a synthetic
@@ -954,6 +1006,19 @@ export class RuleTester extends TestFramework {
                     expectedSuggestion.messageId,
                     `${suggestionPrefix} messageId should be '${expectedSuggestion.messageId}' but got '${actualSuggestion.messageId}' instead.`,
                   );
+
+                  const unsubstitutedPlaceholders =
+                    getUnsubstitutedMessagePlaceholders(
+                      actualSuggestion.desc,
+                      rule.meta.messages[expectedSuggestion.messageId],
+                      expectedSuggestion.data,
+                    );
+
+                  assert.ok(
+                    unsubstitutedPlaceholders.length === 0,
+                    `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.`,
+                  );
+
                   if (hasOwnProperty(expectedSuggestion, 'data')) {
                     const unformattedMetaMessage =
                       rule.meta.messages[expectedSuggestion.messageId];

packages/rule-tester/src/utils/interpolate.ts

@@ -2,6 +2,13 @@
 
 import type { ReportDescriptorMessageData } from '@typescript-eslint/utils/ts-eslint';
 
+/**
+ * Returns a global expression matching placeholders in messages.
+ */
+export function getPlaceholderMatcher(): RegExp {
+  return /\{\{([^{}]+?)\}\}/gu;
+}
+
 export function interpolate(
   text: string,
   data: ReportDescriptorMessageData | undefined,
@@ -10,18 +17,17 @@ export function interpolate(
     return text;
   }
 
+  const matcher = getPlaceholderMatcher();
+
   // Substitution content for any {{ }} markers.
-  return text.replace(
-    /\{\{([^{}]+?)\}\}/gu,
-    (fullMatch, termWithWhitespace: string) => {
-      const term = termWithWhitespace.trim();
+  return text.replace(matcher, (fullMatch, termWithWhitespace: string) => {
+    const term = termWithWhitespace.trim();
 
-      if (term in data) {
-        return String(data[term]);
-      }
+    if (term in data) {
+      return String(data[term]);
+    }
 
-      // Preserve old behavior: If parameter name not provided, don't replace it.
-      return fullMatch;
-    },
-  );
+    // Preserve old behavior: If parameter name not provided, don't replace it.
+    return fullMatch;
+  });
 }

packages/rule-tester/tests/eslint-base/eslint-base.test.js

@@ -1686,6 +1686,63 @@ describe("RuleTester", () => {
         }, "Hydrated message \"Avoid using variables named 'notFoo'.\" does not match \"Avoid using variables named 'foo'.\"");
     });
 
+    it("should throw if the message has a single unsubstituted placeholder when data is not specified", () => {
+      assert.throws(() => {
+        ruleTester.run("foo", require("./fixtures/messageId").withMissingData, {
+          valid: [],
+          invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
+        });
+      }, "The reported message has an unsubstituted placeholder 'name'. Please provide the missing value via the 'data' property in the context.report() call.");
+    });
+
+    it("should throw if the message has a single unsubstituted placeholders when data is specified", () => {
+      assert.throws(() => {
+        ruleTester.run("foo", require("./fixtures/messageId").withMissingData, {
+          valid: [],
+          invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "name" } }] }]
+        });
+      }, "Hydrated message \"Avoid using variables named 'name'.\" does not match \"Avoid using variables named '{{ name }}'.");
+    });
+
+    it("should throw if the message has multiple unsubstituted placeholders when data is not specified", () => {
+      assert.throws(() => {
+        ruleTester.run("foo", require("./fixtures/messageId").withMultipleMissingDataProperties, {
+          valid: [],
+          invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
+        });
+      }, "The reported message has unsubstituted placeholders: 'type', 'name'. Please provide the missing values via the 'data' property in the context.report() call.");
+    });
+
+    it("should not throw if the data in the message contains placeholders not present in the raw message", () => {
+      ruleTester.run("foo", require("./fixtures/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/messageId").withSamePlaceholdersInData, {
+          valid: [],
+          invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
+        });
+      }, "The reported message has an unsubstituted placeholder 'name'. Please provide the missing value 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/messageId").withSamePlaceholdersInData, {
+        valid: [],
+        invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "{{ name }}" } }] }]
+      });
+    });
+
+    it("should not throw an error for specifying non-string data values", () => {
+      ruleTester.run("foo", require("./fixtures/messageId").withNonStringData, {
+        valid: [],
+        invalid: [{ code: "0", errors: [{ messageId: "avoid", data: { value: 0 } }] }]
+      });
+    });
+
     // messageId/message misconfiguration cases
     it("should throw if user tests for both message and messageId", () => {
         assert.throws(() => {
@@ -1854,6 +1911,61 @@ describe("RuleTester", () => {
             });
         });
 
+        it("should fail with a single missing data placeholder when data is not specified", () => {
+          assert.throws(() => {
+            ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMissingPlaceholderData, {
+              valid: [],
+              invalid: [{
+                code: "var foo;",
+                errors: [{
+                  messageId: "avoidFoo",
+                  suggestions: [{
+                    messageId: "renameFoo",
+                    output: "var bar;"
+                  }]
+                }]
+              }]
+            });
+          }, "The message of the suggestion has an unsubstituted placeholder 'newName'. Please provide the missing value via the 'data' property for the suggestion in the context.report() call.");
+        });
+
+        it("should fail with a single missing data placeholder when data is specified", () => {
+          assert.throws(() => {
+            ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMissingPlaceholderData, {
+              valid: [],
+              invalid: [{
+                code: "var foo;",
+                errors: [{
+                  messageId: "avoidFoo",
+                  suggestions: [{
+                    messageId: "renameFoo",
+                    data: { other: "name" },
+                    output: "var bar;"
+                  }]
+                }]
+              }]
+            });
+          }, "The message of the suggestion has an unsubstituted placeholder 'newName'. Please provide the missing value via the 'data' property for the suggestion in the context.report() call.");
+        });
+
+        it("should fail with multiple missing data placeholders when data is not specified", () => {
+          assert.throws(() => {
+            ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMultipleMissingPlaceholderDataProperties, {
+              valid: [],
+              invalid: [{
+                code: "var foo;",
+                errors: [{
+                  messageId: "avoidFoo",
+                  suggestions: [{
+                    messageId: "rename",
+                    output: "var bar;"
+                  }]
+                }]
+              }]
+            });
+          }, "The message of the suggestion has unsubstituted placeholders: 'currentName', 'newName'. Please provide the missing values via the 'data' property for the suggestion in the context.report() call.");
+        });
+
 
         it("should pass when tested using empty suggestion test objects if the array length is correct", () => {
             ruleTester.run("suggestions-messageIds", require("./fixtures/suggestions").withMessageIds, {

packages/rule-tester/tests/eslint-base/fixtures/messageId.js

@@ -37,3 +37,110 @@ module.exports.withMessageOnly = {
         };
     }
 };
+
+module.exports.withMissingData = {
+  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",
+          });
+        }
+      }
+    };
+  }
+};
+
+module.exports.withMultipleMissingDataProperties = {
+  meta: {
+    messages: {
+      avoidFoo: "Avoid using {{ type }} named '{{ name }}'.",
+      unused: "An unused key"
+    }
+  },
+  create(context) {
+    return {
+      Identifier(node) {
+        if (node.name === "foo") {
+          context.report({
+            node,
+            messageId: "avoidFoo",
+          });
+        }
+      }
+    };
+  }
+};
+
+module.exports.withPlaceholdersInData = {
+  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: '{{ placeholder }}' },
+          });
+        }
+      }
+    };
+  }
+};
+
+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 }}' },
+          });
+        }
+      }
+    };
+  }
+};
+
+module.exports.withNonStringData = {
+  meta: {
+    messages: {
+      avoid: "Avoid using the value '{{ value }}'.",
+    }
+  },
+  create(context) {
+    return {
+      Literal(node) {
+        if (node.value === 0) {
+          context.report({
+            node,
+            messageId: "avoid",
+            data:      { value: 0 },
+          });
+        }
+      }
+    };
+  }
+};