diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index 340498d849b..ec25f675a67 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -876,6 +876,8 @@ In addition to the properties above, invalid test cases can also have the follow * `errors` (number or array, required): Asserts some properties of the errors that the rule is expected to produce when run on this code. If this is a number, asserts the number of errors produced. Otherwise, this should be a list of objects, each containing information about a single reported error. The following properties can be used for an error (all are optional): * `message` (string/regexp): The message for the error + * `messageId` (string): The Id for the error. See [testing errors with messageId](#testing-errors-with-messageid) for details + * `data` (object): Placeholder data which can be used in combination with `messageId` * `type` (string): The type of the reported AST node * `line` (number): The 1-based line number of the reported location * `column` (number): The 1-based column number of the reported location @@ -897,12 +899,36 @@ Any additional properties of a test case will be passed directly to the linter a If a valid test case only uses the `code` property, it can optionally be provided as a string containing the code, rather than an object with a `code` key. +#### Testing errors with `messageId` + +If the rule under test uses `messageId`s, you can use `messageId` property in a test case to assert reported error's `messageId` instead of its `message`. + +```js +{ + code: "let foo;", + errors: [{ messageId: "unexpected" }] +} +``` + +For messages with placeholders, a test case can also use `data` property to additionally assert reported error's `message`. + +```js +{ + code: "let foo;", + errors: [{ messageId: "unexpected", data: { name: "foo" } }] +} +``` + +Please note that `data` in a test case does not assert `data` passed to `context.report`. Instead, it is used to form the expected message text which is then compared with the received `message`. + #### Testing Suggestions Suggestions can be tested by defining a `suggestions` key on an errors object. The options to check for the suggestions are the following (all are optional): - * `desc` (string): The suggestion `desc` value - * `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s - * `output` (string): A code string representing the result of applying the suggestion fix to the input code + +* `desc` (string): The suggestion `desc` value +* `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s +* `data` (object): Placeholder data which can be used in combination with `messageId` +* `output` (string): A code string representing the result of applying the suggestion fix to the input code Example: @@ -914,7 +940,24 @@ ruleTester.run("my-rule-for-no-foo", rule, { errors: [{ suggestions: [{ desc: "Rename identifier 'foo' to 'bar'", + output: "var bar;" + }] + }] + }] +}) +``` + +`messageId` and `data` properties in suggestion test objects work the same way as in error test objects. See [testing errors with messageId](#testing-errors-with-messageid) for details. + +```js +ruleTester.run("my-rule-for-no-foo", rule, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ messageId: "renameFoo", + data: { newName: "bar" }, output: "var bar;" }] }] diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 5180f87a316..acfaf18325f 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -141,6 +141,7 @@ const friendlyErrorObjectParameterList = `[${[...errorObjectParameters].map(key const suggestionObjectParameters = new Set([ "desc", "messageId", + "data", "output" ]); const friendlySuggestionObjectParameterList = `[${[...suggestionObjectParameters].map(key => `'${key}'`).join(", ")}]`; @@ -571,10 +572,12 @@ class RuleTester { assert.ok(item.errors || item.errors === 0, `Did not specify errors for an invalid test of ${ruleName}`); + const ruleHasMetaMessages = hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages"); + const friendlyIDList = ruleHasMetaMessages ? `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]` : null; + const result = runRuleForItem(item); const messages = result.messages; - if (typeof item.errors === "number") { assert.strictEqual(messages.length, item.errors, util.format("Should have %d error%s but had %d: %s", item.errors, item.errors === 1 ? "" : "s", messages.length, util.inspect(messages))); @@ -620,12 +623,10 @@ class RuleTester { assertMessageMatches(message.message, error.message); } else if (hasOwnProperty(error, "messageId")) { assert.ok( - hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages"), + ruleHasMetaMessages, "Error can not use 'messageId' if rule under test doesn't define 'meta.messages'." ); if (!hasOwnProperty(rule.meta.messages, error.messageId)) { - const friendlyIDList = `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]`; - assert(false, `Invalid messageId '${error.messageId}'. Expected one of ${friendlyIDList}.`); } assert.strictEqual( @@ -700,19 +701,50 @@ class RuleTester { }); const actualSuggestion = message.suggestions[index]; + const suggestionPrefix = `Error Suggestion at index ${index} :`; + + if (hasOwnProperty(expectedSuggestion, "desc")) { + assert.ok( + !hasOwnProperty(expectedSuggestion, "data"), + `${suggestionPrefix} Test should not specify both 'desc' and 'data'.` + ); + assert.strictEqual( + actualSuggestion.desc, + expectedSuggestion.desc, + `${suggestionPrefix} desc should be "${expectedSuggestion.desc}" but got "${actualSuggestion.desc}" instead.` + ); + } - /** - * Tests equality of a suggestion key if that key is defined in the expected output. - * @param {string} key Key to validate from the suggestion object - * @returns {void} - */ - function assertSuggestionKeyEquals(key) { - if (hasOwnProperty(expectedSuggestion, key)) { - assert.deepStrictEqual(actualSuggestion[key], expectedSuggestion[key], `Error suggestion at index: ${index} should have desc of: "${actualSuggestion[key]}"`); + if (hasOwnProperty(expectedSuggestion, "messageId")) { + assert.ok( + ruleHasMetaMessages, + `${suggestionPrefix} Test can not use 'messageId' if rule under test doesn't define 'meta.messages'.` + ); + assert.ok( + hasOwnProperty(rule.meta.messages, expectedSuggestion.messageId), + `${suggestionPrefix} Test has invalid messageId '${expectedSuggestion.messageId}', the rule under test allows only one of ${friendlyIDList}.` + ); + assert.strictEqual( + actualSuggestion.messageId, + expectedSuggestion.messageId, + `${suggestionPrefix} messageId should be '${expectedSuggestion.messageId}' but got '${actualSuggestion.messageId}' instead.` + ); + if (hasOwnProperty(expectedSuggestion, "data")) { + const unformattedMetaMessage = rule.meta.messages[expectedSuggestion.messageId]; + const rehydratedDesc = interpolate(unformattedMetaMessage, expectedSuggestion.data); + + assert.strictEqual( + actualSuggestion.desc, + rehydratedDesc, + `${suggestionPrefix} Hydrated test desc "${rehydratedDesc}" does not match received desc "${actualSuggestion.desc}".` + ); } + } else { + assert.ok( + !hasOwnProperty(expectedSuggestion, "data"), + `${suggestionPrefix} Test must specify 'messageId' if 'data' is used.` + ); } - assertSuggestionKeyEquals("desc"); - assertSuggestionKeyEquals("messageId"); if (hasOwnProperty(expectedSuggestion, "output")) { const codeWithAppliedSuggestion = SourceCodeFixer.applyFixes(item.code, [actualSuggestion]).output; diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index a8bf356fed1..88d3e3dd6ea 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1007,7 +1007,7 @@ describe("RuleTester", () => { }); describe("suggestions", () => { - it("should pass with valid suggestions", () => { + it("should pass with valid suggestions (tested using desc)", () => { ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, { valid: [ "var boo;" @@ -1046,7 +1046,7 @@ describe("RuleTester", () => { }); }); - it("should pass with suggestions using messageIds", () => { + it("should pass with valid suggestions (tested using messageIds)", () => { ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { valid: [], invalid: [{ @@ -1055,6 +1055,62 @@ describe("RuleTester", () => { suggestions: [{ messageId: "renameFoo", output: "var bar;" + }, { + messageId: "renameFoo", + output: "var baz;" + }] + }] + }] + }); + }); + + it("should pass with valid suggestions (one tested using messageIds, the other using desc)", () => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + output: "var bar;" + }, { + desc: "Rename identifier 'foo' to 'baz'", + output: "var baz;" + }] + }] + }] + }); + }); + + it("should pass with valid suggestions (tested using both desc and messageIds for the same suggestion)", () => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + desc: "Rename identifier 'foo' to 'bar'", + messageId: "renameFoo", + output: "var bar;" + }, { + desc: "Rename identifier 'foo' to 'baz'", + messageId: "renameFoo", + output: "var baz;" + }] + }] + }] + }); + }); + + it("should pass with valid suggestions (tested using only desc on a rule that utilizes meta.messages)", () => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + desc: "Rename identifier 'foo' to 'bar'", + output: "var bar;" }, { desc: "Rename identifier 'foo' to 'baz'", output: "var baz;" @@ -1064,6 +1120,39 @@ describe("RuleTester", () => { }); }); + it("should pass with valid suggestions (tested using messageIds and data)", () => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + data: { newName: "bar" }, + output: "var bar;" + }, { + messageId: "renameFoo", + data: { newName: "baz" }, + output: "var baz;" + }] + }] + }] + }); + }); + + + it("should pass when tested using empty suggestion test objects if the array length is correct", () => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{}, {}] + }] + }] + }); + }); + it("should support explicitly expecting no suggestions", () => { [void 0, null, false, []].forEach(suggestions => { ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/no-eval"), { @@ -1130,7 +1219,7 @@ describe("RuleTester", () => { }, "Error should have 2 suggestions. Instead found 1 suggestions"); }); - it("should fail when the suggestion description doesn't match", () => { + it("should throw if the suggestion description doesn't match", () => { assert.throws(() => { ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, { valid: [], @@ -1144,10 +1233,199 @@ describe("RuleTester", () => { }] }] }); - }, "Error suggestion at index: 0 should have desc of: \"Rename identifier 'foo' to 'bar'\""); + }, "Error Suggestion at index 0 : desc should be \"not right\" but got \"Rename identifier 'foo' to 'bar'\" instead."); + }); + + it("should throw if the suggestion description doesn't match (although messageIds match)", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + desc: "Rename identifier 'foo' to 'bar'", + messageId: "renameFoo", + output: "var bar;" + }, { + desc: "Rename id 'foo' to 'baz'", + messageId: "renameFoo", + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 1 : desc should be \"Rename id 'foo' to 'baz'\" but got \"Rename identifier 'foo' to 'baz'\" instead."); + }); + + it("should throw if the suggestion messageId doesn't match", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "unused", + output: "var bar;" + }, { + messageId: "renameFoo", + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 0 : messageId should be 'unused' but got 'renameFoo' instead."); + }); + + it("should throw if the suggestion messageId doesn't match (although descriptions match)", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + desc: "Rename identifier 'foo' to 'bar'", + messageId: "renameFoo", + output: "var bar;" + }, { + desc: "Rename identifier 'foo' to 'baz'", + messageId: "avoidFoo", + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 1 : messageId should be 'avoidFoo' but got 'renameFoo' instead."); + }); + + it("should throw if test specifies messageId for a rule that doesn't have meta.messages", () => { + assert.throws(() => { + ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + output: "var bar;" + }] + }] + }] + }); + }, "Error Suggestion at index 0 : Test can not use 'messageId' if rule under test doesn't define 'meta.messages'."); + }); + + it("should throw if test specifies messageId that doesn't exist in the rule's meta.messages", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + output: "var bar;" + }, { + messageId: "removeFoo", + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 1 : Test has invalid messageId 'removeFoo', the rule under test allows only one of ['avoidFoo', 'unused', 'renameFoo']."); + }); + + it("should throw if hydrated desc doesn't match (wrong data value)", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + data: { newName: "car" }, + output: "var bar;" + }, { + messageId: "renameFoo", + data: { newName: "baz" }, + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 0 : Hydrated test desc \"Rename identifier 'foo' to 'car'\" does not match received desc \"Rename identifier 'foo' to 'bar'\"."); + }); + + it("should throw if hydrated desc doesn't match (wrong data key)", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + data: { newName: "bar" }, + output: "var bar;" + }, { + messageId: "renameFoo", + data: { name: "baz" }, + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 1 : Hydrated test desc \"Rename identifier 'foo' to '{{ newName }}'\" does not match received desc \"Rename identifier 'foo' to 'baz'\"."); + }); + + it("should throw if test specifies both desc and data", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + desc: "Rename identifier 'foo' to 'bar'", + messageId: "renameFoo", + data: { newName: "bar" }, + output: "var bar;" + }, { + messageId: "renameFoo", + data: { newName: "baz" }, + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 0 : Test should not specify both 'desc' and 'data'."); + }); + + it("should throw if test uses data but doesn't specify messageId", () => { + assert.throws(() => { + ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, { + valid: [], + invalid: [{ + code: "var foo;", + errors: [{ + suggestions: [{ + messageId: "renameFoo", + data: { newName: "bar" }, + output: "var bar;" + }, { + data: { newName: "baz" }, + output: "var baz;" + }] + }] + }] + }); + }, "Error Suggestion at index 1 : Test must specify 'messageId' if 'data' is used."); }); - it("should fail when the resulting suggestion output doesn't match", () => { + it("should throw if the resulting suggestion output doesn't match", () => { assert.throws(() => { ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, { valid: [],