Change Request: Always check data for message in rule tester

[DMartens]

ESLint version

current

What problem do you want to solve?

Currently the rule tester only checks whether placeholders in messages are replaceable if the data property is set.
As a result there may be non-replaced placeholders even if this could be caught in the rule teser.
In other words the additional check can assure rule authors that all placeholders for their messages are available.
With the additional check the following can be caught:

// Rule
export default {
	messages: {
		id: "{{ expected }}"
	},
	create(context) {
		context.report({ messageId: "id", data: { actual: true } });
	},
}

// Rule Tester
ruleTester.run("Rule", rule, {
	valid: [],
	invalid: [{ messageId: "id" }]
});

This is especially helpful as with the upcoming stricter rule tests as message cannot be set with messageId to check whether the placeholders are correctly replaced.

What do you think is the correct solution?

Extend the logic for error / suggestion matcher to check whether all placeholders from the referenced message are available if there is no data property in the matcher.
This should also check that the data properties are not nullish.

Participation

• I am willing to submit a pull request for this change.

Additional comments

I proposed this as a part of stricter rule tester testing in #17654 .

[nzakas]

I think this is a fantastic idea that makes sense in the context of the other RuleTester changes we're making. 👍

[mdjermanovic]

Extend the logic for error / suggestion matcher to check whether all placeholders from the referenced message are available if there is no data property in the matcher.

Can you clarify what exactly will be checked? Would it be a check for the presence of a "{{ ... }}" substring in message.message (message text on the received lint problem)?

[nzakas]

Can you clarify what exactly will be checked? Would it be a check for the presence of a "{{ ... }}" substring in message.message (message text on the received lint problem)?

That's my understanding, yes. This would represent an un-interpolated placeholder string.

[DMartens]

The only extra check I can think of for this situation is additional (= unused in the message) data keys.
As the rule tester also does not check for this if data is set, I think this new check should not either.
I think for a clear error message a getPlaceholderNames(message) function is needed and as such we could just compare them with the keys in the message.data.
This is also the case when checking the message.message for unreplaced placeholders.
Concretely I was thought a rough implementation would be (maybe abstracted in a function so it can be reused for the suggestion case):

if ("data" in error) {
	// ...existing error handling
} else {
	const dataKeys = Object.keys(message.data);
    const placeholders = getPlaceholderNames(messages[messageId]);
	const missing = placeholders.filters((placeholder) => dataKeys.includes(placeholder));
	assert.strictEqual(missing, [], `The error message is missing data for the placeholders "${missing.join(', ')". Please either provide them in the respective context.report call or remove/rename the placeholders in the message.`)
}

The logic for the case with data could be updated to also provide a clearer error message in the case a data property is missing.

[mdjermanovic]

There's no message.data. RuleTester works with the final result, which is a LintMessage that has already been processed by Linter. RuleTester doesn't see the object passed to context.report().

But we could look for a {{ ... }} substring in message.message as an indicator that some expected data wasn't passed to context.report().

[sam3k]

Per 2024-02-08 tsc-meeting, we will target to release on beta.0 today but could possibly be move to beta.1.