From 748d25e54159e35b030ff3f8755b7aa1d3969d07 Mon Sep 17 00:00:00 2001 From: Flavien Raynaud Date: Fri, 17 Nov 2023 14:07:04 +1000 Subject: [PATCH 1/2] [PSC-2029] Add lint rule to prevent trailing forward slashes In an attempt to increase consistency across endpoints, Airtasker wants to avoid having endpoints whose path ends with a trailing forward slash. This PR adds a lint rule that rejects contracts containing endpoints with a trailing forward slash. To maintain backwards compatibility, the default `LintConfig` has been updated to only `warn` on such issues. The rule can be enforced by running `spot lint --no-trailing-forward-slash=error` The rule can be fully disabled by running `spot lint --no-trailing-forward-slash=off`. --- cli/src/commands/lint.ts | 3 ++- lib/src/linting/rules.ts | 4 ++- .../no-trailing-forward-slash.ts | 17 +++++++++++++ .../trailing-forward-slash.ts | 17 +++++++++++++ .../rules/no-trailing-forward-slash.spec.ts | 25 +++++++++++++++++++ .../rules/no-trailing-forward-slash.ts | 25 +++++++++++++++++++ 6 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/no-trailing-forward-slash.ts create mode 100644 lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/trailing-forward-slash.ts create mode 100644 lib/src/linting/rules/no-trailing-forward-slash.spec.ts create mode 100644 lib/src/linting/rules/no-trailing-forward-slash.ts diff --git a/cli/src/commands/lint.ts b/cli/src/commands/lint.ts index 69a6a0183..1ccb62ffa 100644 --- a/cli/src/commands/lint.ts +++ b/cli/src/commands/lint.ts @@ -12,7 +12,8 @@ export interface LintConfig { const lintConfig: LintConfig = { rules: { - "no-omittable-fields-within-response-bodies": "warn" + "no-omittable-fields-within-response-bodies": "warn", + "no-trailing-forward-slash": "warn" } }; diff --git a/lib/src/linting/rules.ts b/lib/src/linting/rules.ts index cae102b0f..f86b6f482 100644 --- a/lib/src/linting/rules.ts +++ b/lib/src/linting/rules.ts @@ -7,6 +7,7 @@ import { noInlineObjectsWithinUnions } from "./rules/no-inline-objects-within-un import { noNullableArrays } from "./rules/no-nullable-arrays"; import { noNullableFieldsWithinRequestBodies } from "./rules/no-nullable-fields-within-request-bodies"; import { noOmittableFieldsWithinResponseBodies } from "./rules/no-omittable-fields-within-response-bodies"; +import { noTrailingForwardSlash } from "./rules/no-trailing-forward-slash"; export const availableRules: LintingRules = { "has-discriminator": hasDiscriminator, @@ -18,7 +19,8 @@ export const availableRules: LintingRules = { "no-nullable-fields-within-request-bodies": noNullableFieldsWithinRequestBodies, "no-omittable-fields-within-response-bodies": - noOmittableFieldsWithinResponseBodies + noOmittableFieldsWithinResponseBodies, + "no-trailing-forward-slash": noTrailingForwardSlash }; interface LintingRules { diff --git a/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/no-trailing-forward-slash.ts b/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/no-trailing-forward-slash.ts new file mode 100644 index 000000000..9c13768f2 --- /dev/null +++ b/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/no-trailing-forward-slash.ts @@ -0,0 +1,17 @@ +import { api, body, endpoint, response, String } from "@airtasker/spot"; + +@api({ name: "contract" }) +class Contract {} + +@endpoint({ + method: "GET", + path: "/users" +}) +class Endpoint { + @response({ status: 200 }) + successResponse(@body body: Body) {} +} + +interface Body { + body: String; +} diff --git a/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/trailing-forward-slash.ts b/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/trailing-forward-slash.ts new file mode 100644 index 000000000..387841ebf --- /dev/null +++ b/lib/src/linting/rules/__spec-examples__/no-trailing-forward-slash/trailing-forward-slash.ts @@ -0,0 +1,17 @@ +import { api, body, endpoint, response, String } from "@airtasker/spot"; + +@api({ name: "contract" }) +class Contract {} + +@endpoint({ + method: "GET", + path: "/users/" +}) +class Endpoint { + @response({ status: 200 }) + successResponse(@body body: Body) {} +} + +interface Body { + body: String; +} diff --git a/lib/src/linting/rules/no-trailing-forward-slash.spec.ts b/lib/src/linting/rules/no-trailing-forward-slash.spec.ts new file mode 100644 index 000000000..e3d4692e7 --- /dev/null +++ b/lib/src/linting/rules/no-trailing-forward-slash.spec.ts @@ -0,0 +1,25 @@ +import { parseContract } from "../../parsers/contract-parser"; +import { createProjectFromExistingSourceFile } from "../../spec-helpers/helper"; +import { noTrailingForwardSlash } from "./no-trailing-forward-slash"; + +describe("no-trailing-forward-slash linter rule", () => { + test("returns no violations for contract not containing a trailing forward slash", () => { + const file = createProjectFromExistingSourceFile( + `${__dirname}/__spec-examples__/no-trailing-forward-slash/no-trailing-forward-slash.ts` + ).file; + + const { contract } = parseContract(file).unwrapOrThrow(); + + expect(noTrailingForwardSlash(contract)).toHaveLength(0); + }); + + test("returns a violation for contract containing a trailing forward slash", () => { + const file = createProjectFromExistingSourceFile( + `${__dirname}/__spec-examples__/no-trailing-forward-slash/trailing-forward-slash.ts` + ).file; + + const { contract } = parseContract(file).unwrapOrThrow(); + + expect(noTrailingForwardSlash(contract)).toHaveLength(1); + }); +}); diff --git a/lib/src/linting/rules/no-trailing-forward-slash.ts b/lib/src/linting/rules/no-trailing-forward-slash.ts new file mode 100644 index 000000000..259bf8453 --- /dev/null +++ b/lib/src/linting/rules/no-trailing-forward-slash.ts @@ -0,0 +1,25 @@ +import { Contract } from "../../definitions"; +import { LintingRuleViolation } from "../rule"; + +/** + * Request types should always be object types + * + * @param contract a contract + */ +export function noTrailingForwardSlash( + contract: Contract +): LintingRuleViolation[] { + const violations: LintingRuleViolation[] = []; + + contract.endpoints.forEach(endpoint => { + const { path } = endpoint; + + if (path.match(/\/$/)) { + violations.push({ + message: `Endpoint (${endpoint.name} ${path}) contains a trailing forward slash` + }); + } + }); + + return violations; +} From e664cac0ae95b1e8fdb978393d515269857be239 Mon Sep 17 00:00:00 2001 From: Flavien Raynaud Date: Fri, 17 Nov 2023 14:13:41 +1000 Subject: [PATCH 2/2] Update function docstring --- lib/src/linting/rules/no-trailing-forward-slash.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/linting/rules/no-trailing-forward-slash.ts b/lib/src/linting/rules/no-trailing-forward-slash.ts index 259bf8453..cc1cdf267 100644 --- a/lib/src/linting/rules/no-trailing-forward-slash.ts +++ b/lib/src/linting/rules/no-trailing-forward-slash.ts @@ -2,7 +2,8 @@ import { Contract } from "../../definitions"; import { LintingRuleViolation } from "../rule"; /** - * Request types should always be object types + * Checks that no endpoint is defined with a path that contains a trailing + * forward slash. * * @param contract a contract */