diff --git a/README.md b/README.md index b9bae9d..e1f36de 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,14 @@ assertIdentical(foo.length, 2); assertArrayLength(foo, 2); ``` +Response status suggestions include the response description used on failure: + +```typescript +assertIdentical(response.status, 200); +// suggested improvement: +assertResponseStatus(response, 200, await describeResponse(response)); +``` + The same suggestions are available for both ESLint and Oxlint, generated from a single table of selectors, so the two report identically. diff --git a/docs/lint.md b/docs/lint.md index 922c70c..79e157e 100644 --- a/docs/lint.md +++ b/docs/lint.md @@ -67,4 +67,6 @@ own — `@typescript-eslint/only-throw-error` suppresses `typescript/only-throw- `@kensio/smartass/eslint` and `@kensio/smartass/oxlint` are generated from one table in [`src/lint/prefer-specific-assertions.ts`](../src/lint/prefer-specific-assertions.ts), so both -linters report identically. See the README for consumer setup. +linters report identically. Status checks against numeric literals suggest `assertResponseStatus` +with `describeResponse`. Failure messages include the response metadata and body. See the README +for consumer setup. diff --git a/src/assert/response-status/response-status.assert.ts b/src/assert/response-status/response-status.assert.ts index a36aa17..c054145 100644 --- a/src/assert/response-status/response-status.assert.ts +++ b/src/assert/response-status/response-status.assert.ts @@ -6,26 +6,22 @@ import { assertInstanceOf } from "../instance-of/instance-of.assert.js"; /** * Assert that a Response has a specific HTTP status code, with type narrowing. - * Optionally use describeResponse() for the third argument to get more detailed - * diagnostic information if the assertion fails. + * Pass describeResponse() as the third argument to include the response metadata + * and body in the failure message. * @example * ```ts - * import { assertResponseStatus } from "@kensio/smartass"; + * import { assertResponseStatus, describeResponse } from "@kensio/smartass"; * * const response = await fetch("https://example.com"); * - * assertResponseStatus(response, 200); + * assertResponseStatus( + * response, + * 200, + * await describeResponse(response), + * ); * * // response.status is now narrowed to 200 * ``` - * @example - * ```ts - * import { assertResponseStatus, describeResponse } from "@kensio/smartass"; - * - * const res = await fetch("https://example.com"); - * - * assertResponseStatus(res, 200, await describeResponse(res)); - * ``` */ export function assertResponseStatus( response: unknown, diff --git a/src/describe/response/describe-response.ts b/src/describe/response/describe-response.ts index 789fc6a..1a0538f 100644 --- a/src/describe/response/describe-response.ts +++ b/src/describe/response/describe-response.ts @@ -20,8 +20,8 @@ export interface ResponseDescription { * been consumed, or if cloning or reading the body fails, the returned * description still includes the synchronously available response metadata. * - * The result can be passed to assertResponseStatus() as the third argument to - * include body text and other captured details in assertion failure messages. + * Pass the result to assertResponseStatus() as the third argument. Its failure + * message then includes the body text and other captured details. * @example * ```ts * import { assertResponseStatus, describeResponse } from "@kensio/smartass"; diff --git a/src/eslint/smartass-eslint.config.test.ts b/src/eslint/smartass-eslint.config.test.ts index 2937fed..2bcd0a5 100644 --- a/src/eslint/smartass-eslint.config.test.ts +++ b/src/eslint/smartass-eslint.config.test.ts @@ -94,9 +94,73 @@ describe("smartassPreferSpecificAssertions", () => { expect(lint("assertStringLength(text, 0);")).toStrictEqual([]); }); + it("suggests a described response status assertion for a status property", () => { + // Given a broad assertion against a Response status property. + const code = "assertIdentical(response.status, 200);"; + + // When ESLint checks it with the published config. + const messages = lint(code); + + // Then it recommends the response assertion with failure diagnostics. + expect(messages).toStrictEqual([ + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertIdentical(response.status, expectedStatus).", + ]); + }); + + it("suggests a described response status assertion for a status comparison", () => { + // Given a boolean assertion around an exact Response status comparison. + const code = "assertTrue(response.status === 200);"; + + // When ESLint checks it with the published config. + const messages = lint(code); + + // Then it recommends the response assertion with failure diagnostics. + expect(messages).toStrictEqual([ + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status === expectedStatus).", + ]); + }); + + it("recognises loose and negated status comparisons", () => { + // Given other boolean forms that assert a Response has one status. + const code = [ + "assertTrue(response.status == 200);", + "assertFalse(response.status !== 200);", + "assertFalse(response.status != 200);", + ].join("\n"); + + // When ESLint checks them with the published config. + const messages = lint(code); + + // Then every form points to the described response status assertion. + expect(messages).toStrictEqual([ + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status == expectedStatus).", + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status !== expectedStatus).", + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status != expectedStatus).", + ]); + }); + + it("leaves string-valued application statuses alone", () => { + // Given status properties that cannot represent an HTTP status code. + const code = [ + "assertIdentical(job.status, 'active');", + "assertTrue(job.status === 'active');", + ].join("\n"); + + // When ESLint checks them with the published config. + const messages = lint(code); + + // Then it makes no Response-specific suggestion. + expect(messages).toStrictEqual([]); + }); + it("leaves assertions that are already specific alone", () => { expect(lint("assertArrayNotEmpty(values);")).toStrictEqual([]); expect(lint("assertIdentical(name, 'smartass');")).toStrictEqual([]); + expect( + lint( + "assertResponseStatus(response, 200, await describeResponse(response));", + ), + ).toStrictEqual([]); }); }); }); diff --git a/src/eslint/smartass-eslint.config.ts b/src/eslint/smartass-eslint.config.ts index 07f74c3..8861841 100644 --- a/src/eslint/smartass-eslint.config.ts +++ b/src/eslint/smartass-eslint.config.ts @@ -3,7 +3,9 @@ import { defineConfig } from "eslint/config"; import { preferSpecificAssertionRules } from "../lint/prefer-specific-assertions.js"; /** - * An ESLint flat config that nudges you from broad assertions towards the specific ones. + * An ESLint flat config that nudges you from broad assertions towards the specific ones. Response + * status suggestions include describeResponse(). Failure messages show the response metadata and + * body. * * The same suggestions are available to Oxlint users via `@kensio/smartass/oxlint`. */ diff --git a/src/lint/prefer-specific-assertions.ts b/src/lint/prefer-specific-assertions.ts index 4759d28..aabeaae 100644 --- a/src/lint/prefer-specific-assertions.ts +++ b/src/lint/prefer-specific-assertions.ts @@ -75,6 +75,15 @@ export const preferSpecificAssertionRules: readonly PreferSpecificAssertionRule[ message: "Use a more specific size assertion, such as assertSetSize(value, expectedSize) or assertMapSize(value, expectedSize), instead of assertIdentical(value.size, expectedSize).", }, + // Without type information, a `.status` property could belong to any object. A numeric literal + // in the expected position makes the Response interpretation specific enough to suggest the + // HTTP assertion while leaving string-valued application statuses alone. + { + selector: + "CallExpression[callee.name='assertIdentical']:has(> MemberExpression[property.name='status']:first-child):has(> Literal[value=type(number)]:nth-child(2))", + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertIdentical(response.status, expectedStatus).", + }, { selector: "CallExpression[callee.name='assertIdentical'] > BinaryExpression[operator='===']:first-child", @@ -188,6 +197,18 @@ export const preferSpecificAssertionRules: readonly PreferSpecificAssertionRule[ message: "Use a more specific size assertion, such as assertSetSize(value, expectedSize) or assertMapSize(value, expectedSize), instead of assertTrue(value.size === expectedSize).", }, + { + selector: + "CallExpression[callee.name='assertTrue'] > BinaryExpression[operator='===']:has(> MemberExpression[property.name='status']):has(> Literal[value=type(number)])", + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status === expectedStatus).", + }, + { + selector: + "CallExpression[callee.name='assertTrue'] > BinaryExpression[operator='==']:has(> MemberExpression[property.name='status']):has(> Literal[value=type(number)])", + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status == expectedStatus).", + }, { selector: "CallExpression[callee.name='assertTrue'] > CallExpression[callee.property.name='includes']", @@ -260,6 +281,18 @@ export const preferSpecificAssertionRules: readonly PreferSpecificAssertionRule[ message: "Use assertNonNullable(value) instead of assertFalse(value == null).", }, + { + selector: + "CallExpression[callee.name='assertFalse'] > BinaryExpression[operator='!==']:has(> MemberExpression[property.name='status']):has(> Literal[value=type(number)])", + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status !== expectedStatus).", + }, + { + selector: + "CallExpression[callee.name='assertFalse'] > BinaryExpression[operator='!=']:has(> MemberExpression[property.name='status']):has(> Literal[value=type(number)])", + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status != expectedStatus).", + }, // assertFalse(value.length === 0) is left alone for the same reason as the assertTrue length // comparisons above: the selector cannot tell a string from an array, and assertArrayNotEmpty // would throw on a string. diff --git a/src/oxlint/smartass-oxlint-plugin.test.ts b/src/oxlint/smartass-oxlint-plugin.test.ts index 84bdddb..5d07fff 100644 --- a/src/oxlint/smartass-oxlint-plugin.test.ts +++ b/src/oxlint/smartass-oxlint-plugin.test.ts @@ -87,6 +87,10 @@ describe("smartassOxlintPlugin", () => { { filename: "valid.ts", code: "assertStringLength(text, 0);" }, { filename: "valid.ts", code: "assertTrue(isReady);" }, { filename: "valid.ts", code: "assertIdentical(name, 'smartass');" }, + { + filename: "valid.ts", + code: "assertResponseStatus(response, 200, await describeResponse(response));", + }, // The selectors are anchored to the assertion name, so lookalikes are left alone. { filename: "valid.ts", code: "expect(values.length > 0);" }, // A string that happens to spell a boolean is not a boolean, and assertTrue would be the @@ -99,6 +103,14 @@ describe("smartassOxlintPlugin", () => { filename: "valid.ts", code: "assertIdentical(valueAttribute(false), 'false');", }, + { + filename: "valid.ts", + code: "assertIdentical(job.status, 'active');", + }, + { + filename: "valid.ts", + code: "assertTrue(job.status === 'active');", + }, // Same for a string spelling `null`. { filename: "valid.ts", code: "assertTrue(rawValue == 'null');" }, // `.length` comparisons are as much a string shape as an array one, and the array @@ -179,6 +191,56 @@ describe("smartassOxlintPlugin", () => { }, ], }, + { + filename: "invalid.ts", + code: "assertIdentical(response.status, 200);", + errors: [ + { + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertIdentical(response.status, expectedStatus).", + }, + ], + }, + { + filename: "invalid.ts", + code: "assertTrue(response.status === 200);", + errors: [ + { + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status === expectedStatus).", + }, + ], + }, + { + filename: "invalid.ts", + code: "assertTrue(response.status == 200);", + errors: [ + { + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertTrue(response.status == expectedStatus).", + }, + ], + }, + { + filename: "invalid.ts", + code: "assertFalse(response.status !== 200);", + errors: [ + { + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status !== expectedStatus).", + }, + ], + }, + { + filename: "invalid.ts", + code: "assertFalse(response.status != 200);", + errors: [ + { + message: + "Use assertResponseStatus(response, expectedStatus, await describeResponse(response)) instead of assertFalse(response.status != expectedStatus).", + }, + ], + }, { filename: "invalid.ts", code: "assertFalse(!existsSync(path));", diff --git a/src/oxlint/smartass-oxlint-plugin.ts b/src/oxlint/smartass-oxlint-plugin.ts index f3a7976..79e0e04 100644 --- a/src/oxlint/smartass-oxlint-plugin.ts +++ b/src/oxlint/smartass-oxlint-plugin.ts @@ -50,7 +50,8 @@ export interface OxlintPlugin { } /** - * Suggests a more specific assertion wherever a broad one is doing a specific job. + * Suggests a more specific assertion wherever a broad one is doing a specific job. Response status + * suggestions include describeResponse(). Failure messages show the response metadata and body. * * This is the Oxlint counterpart of the `no-restricted-syntax` entries in * `@kensio/smartass/eslint`; both are generated from the same selector table, so the two linters