Skip to content

[@typescript-eslint/indent] SwitchCase default inconsistent with docs and ESLint #608

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.

Sign up for GitHub

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

Jump to bottom
Closed
benquarmby opened this issue Jun 11, 2019 · 4 comments
Closed

[@typescript-eslint/indent] SwitchCase default inconsistent with docs and ESLint #608

benquarmby opened this issue Jun 11, 2019 · 4 comments
Labels
bug Something isn't working documentation Documentation ("docs") that needs adding/updating package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Milestone
indent rewrite

Comments

@benquarmby
Copy link

The documentation states that the default indentation level for SwitchCase is zero:
https://github.com/typescript-eslint/typescript-eslint/blame/61e6385c65b147a5a8b4caa2f3241e5a92c9b1bf/packages/eslint-plugin/docs/rules/indent.md#L77

The default for ESLint is also zero:
https://eslint.org/docs/rules/indent#options

In practice, the level is actually 1. Other than the documentation being incorrect, I'd argue that turning ESLint's indent off and setting @typescript-eslint/indent to error should not produce new violations on code covered by both rules.

Repro

{
  "rules": {
    "indent": "off",
    "@typescript-eslint/indent": "error"
  }
}
switch (typeof true) {
case "string":
    throw new Error("never");
}

Expected Result
Does not raise a violation.

Actual Result
Raises a violation.

Additional Info
Can be worked around by explicitly setting the SwitchCase level. Unfortunately this requires being explicit about the indentation depth also:

{
  "rules": {
    "indent": "off",
    "@typescript-eslint/indent": ["error", 4, {
      "SwitchCase": 0
    }]
  }
}

Versions

package version
@typescript-eslint/eslint-plugin 1.10.2
@typescript-eslint/parser 1.10.2
TypeScript 3.3.1
ESLint 5.12.1
node 10.15.4
yarn 1.16.0
@benquarmby benquarmby added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jun 11, 2019
@bradzacher bradzacher added bug Something isn't working documentation Documentation ("docs") that needs adding/updating and removed triage Waiting for team members to take a look labels Jun 11, 2019
@bradzacher
Copy link
Member

Yeah the documentation is copy pasted from eslint core.
We don't do that any more for reasons like this, but haven't gone back to fix the older docs.

We set new defaults based off of typescript standard:

typescript-eslint/packages/eslint-plugin/src/rules/indent.ts

Lines 98 to 108 in 61e6385

defaultOptions: [
// typescript docs and playground use 4 space indent
4,
{
// typescript docs indent the case from the switch
// https://www.typescriptlang.org/docs/handbook/release-notes/typescript-1-8.html#example-4
SwitchCase: 1,
flatTernaryExpressions: false,
ignoredNodes: [],
},
],

So we need to update the docs to match.

@benquarmby
Copy link
Author

We set new defaults based off of typescript standard

Got it. Is that standard inferred from how TypeScript is used by the TS team and their documentation?

@bradzacher
Copy link
Member

In part, yes.

The recommended set and the defaults for rules were based off of three things: what the TS team does in the documentation, what community eslint configs setup when including the old eslint-plugin-typescript, and what I thought was the correct thing at the time.

For indentation it was very much the former - TS playground + TS documentation both use 4 space indentation with switch cases indented at 1.

@bradzacher bradzacher added this to the indent rewrite milestone Jul 27, 2019
@bradzacher
Copy link
Member

Merging into #1824

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working documentation Documentation ("docs") that needs adding/updating package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
indent rewrite
Development

No branches or pull requests
2 participants
@benquarmby @bradzacher