From 0ce483931a5cdfe94d791ef23530deb9c9c4a445 Mon Sep 17 00:00:00 2001 From: azu Date: Mon, 26 Aug 2024 13:02:55 +0900 Subject: [PATCH 1/3] fix(valid-expect): allow `expect(value, "message")` --- docs/rules/valid-expect.md | 3 +- src/rules/valid-expect.ts | 8 +++++ tests/valid-expect.test.ts | 70 +++++++++++++++++++++++++++----------- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/docs/rules/valid-expect.md b/docs/rules/valid-expect.md index c79ba62..62d1023 100644 --- a/docs/rules/valid-expect.md +++ b/docs/rules/valid-expect.md @@ -68,13 +68,14 @@ This rule triggers a warning if `expect` is called with no argument or with more - Default: `1` - Enforce `expect` to be called with at most `maxArgs` arguments. + - Exception: `expect(value, "message")` is allowed. ```js // ✅ good expect(1).toBe(1) + expect(1, "expect value is one").toBe(1) // ❌ bad expect(1, 2, 3, 4).toBe(1) ``` - diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts index 9b08997..7064590 100644 --- a/src/rules/valid-expect.ts +++ b/src/rules/valid-expect.ts @@ -241,6 +241,14 @@ export default createEslintRule<[ } if (expect.arguments.length > maxArgs) { + // if expect(value, "message"), it is valid usage + // Note: 2nd argument should be string literal, not a variable in current implementation + if (expect.arguments.length === 2 + && expect.arguments[1].type === AST_NODE_TYPES.Literal + && typeof expect.arguments[1].value === 'string') { + return + } + const { start } = expect.arguments[maxArgs].loc const { end } = expect.arguments[expect.arguments.length - 1].loc diff --git a/tests/valid-expect.test.ts b/tests/valid-expect.test.ts index 10d75d6..ed4d7f1 100644 --- a/tests/valid-expect.test.ts +++ b/tests/valid-expect.test.ts @@ -83,6 +83,7 @@ ruleTester.run(RULE_NAME, rule, { } }); `, + 'expect(value, "message").toBe(1);', { code: 'expect(1).toBe(2);', options: [{ maxArgs: 2 }] @@ -150,20 +151,6 @@ ruleTester.run(RULE_NAME, rule, { } ] }, - { - code: 'expect("something", "else").toEqual("something");', - errors: [ - { - endColumn: 28, - column: 21, - messageId: 'tooManyArgs', - data: { - s: '', - amount: 1 - } - } - ] - }, { code: 'expect("something", "else", "entirely").toEqual("something");', options: [{ maxArgs: 2 }], @@ -236,18 +223,61 @@ ruleTester.run(RULE_NAME, rule, { s: 's', amount: 3 } - }, + } + ] + }, + // expect() 2nd argument to be a string literal + { + code: 'expect("something", true).toEqual("something");', + errors: [ { - endColumn: 28, + endColumn: 26, column: 21, messageId: 'tooManyArgs', - data: { - s: '', - amount: 1 - } } ] }, + { + code: 'expect("something", 12).toEqual("something");', + errors: [ + { + endColumn: 24, + column: 21, + messageId: 'tooManyArgs', + } + ] + }, + { + code: 'expect("something", {}).toEqual("something");', + errors: [ + { + endColumn: 24, + column: 21, + messageId: 'tooManyArgs', + } + ] + }, + { + code: 'expect("something", []).toEqual("something");', + errors: [ + { + endColumn: 24, + column: 21, + messageId: 'tooManyArgs', + } + ] + }, + // expect(value, "string literal") is valid, but expect(value, variable) is not + // because the AST does not have type information for variables + { + code: `const message = "message"; + expect(1, message).toBe(2);`, + errors: [{ + endColumn: 25, + column: 17, + messageId: 'tooManyArgs' + }] + }, { code: 'expect("something");', errors: [{ endColumn: 20, column: 1, messageId: 'matcherNotFound' }] From 4e207d928e2c632a484ace5b60cc10ddb301d6ca Mon Sep 17 00:00:00 2001 From: azu Date: Mon, 26 Aug 2024 13:09:24 +0900 Subject: [PATCH 2/3] docs: update example --- docs/rules/valid-expect.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/valid-expect.md b/docs/rules/valid-expect.md index 62d1023..5829db3 100644 --- a/docs/rules/valid-expect.md +++ b/docs/rules/valid-expect.md @@ -73,7 +73,7 @@ This rule triggers a warning if `expect` is called with no argument or with more ```js // ✅ good expect(1).toBe(1) - expect(1, "expect value is one").toBe(1) + expect(1, "expect value to be one").toBe(1) // ❌ bad From b492a7746a53b28ed21605815791a6aa8626b8ba Mon Sep 17 00:00:00 2001 From: azu Date: Mon, 26 Aug 2024 14:13:14 +0900 Subject: [PATCH 3/3] fix: support template literal --- docs/rules/valid-expect.md | 2 ++ src/rules/valid-expect.ts | 17 +++++++++++------ tests/valid-expect.test.ts | 2 ++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/rules/valid-expect.md b/docs/rules/valid-expect.md index 5829db3..83bea29 100644 --- a/docs/rules/valid-expect.md +++ b/docs/rules/valid-expect.md @@ -74,6 +74,8 @@ This rule triggers a warning if `expect` is called with no argument or with more // ✅ good expect(1).toBe(1) expect(1, "expect value to be one").toBe(1) + const message = "expect value to be one" + expect(1, `Error Message: ${message}`).toBe(1) // ❌ bad diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts index 7064590..25b5f1a 100644 --- a/src/rules/valid-expect.ts +++ b/src/rules/valid-expect.ts @@ -241,12 +241,17 @@ export default createEslintRule<[ } if (expect.arguments.length > maxArgs) { - // if expect(value, "message"), it is valid usage - // Note: 2nd argument should be string literal, not a variable in current implementation - if (expect.arguments.length === 2 - && expect.arguments[1].type === AST_NODE_TYPES.Literal - && typeof expect.arguments[1].value === 'string') { - return + // if expect(value, "message") and expect(value, `${message}`), it is valid usage + // Note: 2nd argument should be string, not a variable in current implementation + if (expect.arguments.length === 2) { + // expect(value, "string literal") + const isSecondArgString = expect.arguments[1].type === AST_NODE_TYPES.Literal && + typeof expect.arguments[1].value === 'string'; + // expect(value, `template literal`) + const isSecondArgTemplateLiteral = expect.arguments[1].type === AST_NODE_TYPES.TemplateLiteral; + if (isSecondArgString || isSecondArgTemplateLiteral) { + return; + } } const { start } = expect.arguments[maxArgs].loc diff --git a/tests/valid-expect.test.ts b/tests/valid-expect.test.ts index ed4d7f1..9bf6d1b 100644 --- a/tests/valid-expect.test.ts +++ b/tests/valid-expect.test.ts @@ -84,6 +84,8 @@ ruleTester.run(RULE_NAME, rule, { }); `, 'expect(value, "message").toBe(1);', + 'expect(value, `message`).toBe(1);', + 'const message = "message"; expect(value, `${message}`).toBe(1);', { code: 'expect(1).toBe(2);', options: [{ maxArgs: 2 }]