Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PSC-2029] Add lint rule to prevent trailing forward slashes #2154

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

flavray
Copy link
Contributor

@flavray flavray commented Nov 17, 2023

Description, Motivation and Context

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 <api.rs> --no-trailing-forward-slash=error

The rule can be fully disabled by running
spot lint <api.ts> --no-trailing-forward-slash=off.

Example output:

$ spot lint api.ts
 ›   Warning: Endpoint (AAA /aaa/) contains a trailing forward slash
 ›   Warning: Endpoint (AAB /aab/) contains a trailing forward slash
 ›   Warning: Endpoint (ABC /abc/) contains a trailing forward slash
 ›   Warning: Endpoint (XXX /xxx/) contains a trailing forward slash
 ›   Warning: Endpoint (XYZ /xyz/) contains a trailing forward slash
Found 0 errors and 5 warnings


$ spot lint api.ts --no-trailing-forward-slash=error
 ›   Error: Endpoint (AAA /aaa/) contains a trailing forward slash
 ›   Error: Endpoint (AAB /aab/) contains a trailing forward slash
 ›   Error: Endpoint (ABC /abc/) contains a trailing forward slash
 ›   Error: Endpoint (XXX /xxx/) contains a trailing forward slash
 ›   Error: Endpoint (XYZ /xyz/) contains a trailing forward slash
Found 0 errors and 5 warnings


$ spot lint api.ts --no-trailing-forward-slash=off
Found 0 errors and 0 warnings

Checklist:

  • I've added/updated tests to cover my changes
  • I've created an issue associated with this PR

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 <api.rs> --no-trailing-forward-slash=error`

The rule can be fully disabled by running
`spot lint <api.ts> --no-trailing-forward-slash=off`.
@@ -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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default configuration used by the lint command. It can be overriden via command-line arguments since #2153

@flavray flavray marked this pull request as ready for review November 17, 2023 04:22
@flavray flavray requested a review from a team as a code owner November 17, 2023 04:22
Copy link
Contributor

@timdawborn timdawborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Nice one.

Copy link

@njday njday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

@flavray flavray merged commit 931adc9 into master Nov 20, 2023
6 checks passed
@flavray flavray deleted the PSC-2029-no-trailing-forward-slash branch November 20, 2023 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants