Skip to content

Commit

Permalink
Merge branch 'main' into v8
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaKGoldberg committed May 12, 2024
2 parents b765f7b + 7ce6acd commit 0d85360
Show file tree
Hide file tree
Showing 7 changed files with 382 additions and 13 deletions.
21 changes: 21 additions & 0 deletions docs/packages/Rule_Tester.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,27 @@ RuleTester.itOnly = vitest.it.only;
RuleTester.describe = vitest.describe;
```

#### Node built-in test runner

Consider setting up `RuleTester`'s static properties in a preloaded module using the [`--import`](https://nodejs.org/api/cli.html#--importmodule) or [`--require`](https://nodejs.org/api/cli.html#-r---require-module) flag:

```ts
// setup.js
import * as test from 'node:test';
import { RuleTester } from '@typescript-eslint/rule-tester';

RuleTester.afterAll = test.afterAll;
RuleTester.describe = test.describe;
RuleTester.it = test.it;
RuleTester.itOnly = test.it.only;
```

Tests can then be [run from the command line](https://nodejs.org/api/test.html#running-tests-from-the-command-line) like so:

```sh
node --import setup.js --test
```

## Options

### `RuleTester` constructor options
Expand Down
67 changes: 66 additions & 1 deletion packages/rule-tester/src/RuleTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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> = {};
Expand Down Expand Up @@ -812,6 +851,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
Expand Down Expand Up @@ -957,6 +1009,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];
Expand Down
28 changes: 17 additions & 11 deletions packages/rule-tester/src/utils/interpolate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
});
}
112 changes: 112 additions & 0 deletions packages/rule-tester/tests/eslint-base/eslint-base.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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, {
Expand Down

0 comments on commit 0d85360

Please sign in to comment.