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

Change Request: Deprecate formatting rules and recommend using a source code formatter #17522

Closed
1 task
nzakas opened this issue Aug 31, 2023 · 49 comments · Fixed by #17696
Closed
1 task

Change Request: Deprecate formatting rules and recommend using a source code formatter #17522

nzakas opened this issue Aug 31, 2023 · 49 comments · Fixed by #17696
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features deprecation This change deprecates an existing feature enhancement This change enhances an existing feature of ESLint

Comments

@nzakas
Copy link
Member

nzakas commented Aug 31, 2023

ESLint version

HEAD

What problem do you want to solve?

ESLint has a lot of core rules that simply move whitespace around. After years of maintaining these rules, I personally believe two things:

  1. ESLint is not the right tool for source code formatting. Having each augmentation of whitespace as a separate rule just doesn't scale and forces us to continue to add exceptions and new rules whenever new syntax is added. This type of whitespace management is better handled holistically by source code formatters.
  2. It's getting harder to maintain these rules and people aren't as willing to do so. The indent rule, for example, currently has eight open issues, most of which have been open for over a year. The complexity of this rule alone takes away from the time we can spend on more impactful changes.

What do you think is the correct solution?

I'd like to deprecate all formatting rules (anything that is adjusting whitespace) in v9 and recommend that people use a source code formatter instead. Both Prettier and dprint are viable options that solve the same problems in different ways.

This would free us up from maintaining a large number of core rules and make it easy for us to drop these rules during the rewrite.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response

@nzakas nzakas added enhancement This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features breaking This change is backwards-incompatible labels Aug 31, 2023
@bradzacher
Copy link
Contributor

bradzacher commented Sep 5, 2023

I'm fully in favour of this! We on typescript-eslint have been big proponents of this for a long time and we have the same policy as ESLint around formatting rules - no brand new rules; no new options. We also mostly leave them in the community-maintained domain (as in the maintainers don't normally action bug reports for them).

We've been in this weird state for a while because there's a few formatting rules in core that break on TS syntax - so we need to have extension rules in our project to support the syntax. Around half of our extension rules are formatting rules!

So I think that ESLint core taking a hard stance will be a boon for the community at large, and will help us simplify things immensely ourselves 🎉

However I do know that a lot of people don't like formatters for various reasons. Whilst I clearly disagree with their reasons I think having the option out there is okay.
So I might suggest that all of the rules could be moved into the @eslint-community namespace as a separate plugin. This would allow the community to take control of them and maintain them (or let them languish as the community decides) and would also allow the support for other languages (eg the @typescript-eslint extension rule changes can be merged into such a fork).

This was actually something that @JoshuaKGoldberg and I had talked about in the past to try and "get rid of" formatting rules from @typescript-eslint given we never used them and were sick of the load from bug reports and PR reviews around them.

@JamesHenry
Copy link
Member

@nzakas
Copy link
Member Author

nzakas commented Sep 6, 2023

So I might suggest that all of the rules could be moved into the https://github.com/eslint-community namespace as a separate plugin. This would allow the community to take control of them and maintain them (or let them languish as the community decides) and would also allow the support for other languages (eg the https://github.com/typescript-eslint extension rule changes can be merged into such a fork).

In the short-term, the rules won't be removed from the core, we just won't keep updating them. In the long term, repackaging them into a separate plugin makes sense. My overall goal is to not move these over during the rewrite, so the final ruleset after the rewrite doesn't contain any formatting rules.

@mdjermanovic
Copy link
Member

I generally support this proposal, we just need to define a concrete list of rules we'd like to deprecate.

I'd like to deprecate all formatting rules (anything that is adjusting whitespace) in v9 and recommend that people use a source code formatter instead.

Would we keep non-whitespace rules that might classify as "formatting"? For example, semi and comma-dangle are handled by Prettier.

@bradzacher
Copy link
Contributor

For reference here is a list of rules that people disable when using prettier

https://github.com/prettier/eslint-config-prettier/blob/main/index.js

@nzakas
Copy link
Member Author

nzakas commented Sep 7, 2023

Would we keep non-whitespace rules that might classify as "formatting"? For example, semi and comma-dangle are handled by Prettier.

I was envisioning only rules that relate to whitespace, so we would keep things like semi and comma-dangle. But I'm open to arguments that we should include non-whitespace rules as well, I just think they're not as a big of a maintenance burden as the whitespace ones.

@nzakas
Copy link
Member Author

nzakas commented Sep 8, 2023

Initial list of rules:

  • array-bracket-newline.js
  • array-bracket-spacing.js
  • array-element-newline.js
  • arrow-spacing.js
  • block-spacing.js
  • comma-spacing.js
  • computed-property-spacing.js
  • func-call-spacing.js
  • function-call-argument-newline.js
  • function-paren-newline.js
  • generator-star-spacing.js
  • implicit-arrow-linebreak.js
  • indent.js
  • indent-legacy.js
  • key-spacing.js
  • keyword-spacing.js
  • linebreak-style.js
  • line-comment-position.js
  • lines-around-comment.js
  • lines-around-directive.js
  • lines-between-class-members.js
  • multiline-comment-style.js
  • multiline-ternary.js
  • newline-after-var.js
  • newline-before-return.js
  • newline-per-chained-call.js
  • no-mixed-spaces-and-tabs.js
  • no-multiple-empty-lines.js
  • no-multi-spaces.js
  • no-spaced-func.js
  • no-trailing-spaces.js
  • no-whitespace-before-property.js
  • object-curly-newline.js
  • object-curly-spacing.js
  • object-property-newline.js
  • operator-linebreak.js
  • padded-blocks.js
  • padding-line-between-statements.js
  • rest-spread-spacing.js
  • semi-spacing.js
  • space-before-blocks.js
  • space-before-function-paren.js
  • spaced-comment.js
  • space-infix-ops.js
  • space-in-parens.js
  • space-unary-ops.js
  • switch-colon-spacing.js
  • template-curly-spacing.js
  • template-tag-spacing.js
  • yield-star-spacing.js

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Sep 8, 2023
@mdjermanovic
Copy link
Member

indent-legacy, lines-around-directive, newline-after-var, newline-before-return, no-spaced-func are already deprecated.

@mdjermanovic
Copy link
Member

multiline-comment-style is not about whitespace.

@mdjermanovic
Copy link
Member

I was envisioning only rules that relate to whitespace, so we would keep things like semi and comma-dangle.

I agree with deprecating only rules related to whitespace. While there are a few others also handled by prettier (and I believe dprint), there's probably no universal definition of what a JS source code formatter should be responsible for, but it's safe to say that any formatter should be responsible for whitespace.

@mdjermanovic
Copy link
Member

Few more whitespace-related rules that I think should be included:

  • brace-style
  • comma-style
  • dot-location
  • no-tabs
  • nonblock-statement-body-position
  • one-var-declaration-per-line
  • semi-style

@mdjermanovic
Copy link
Member

line-comment-position also isn't about whitespace, I believe.

@antfu
Copy link
Contributor

antfu commented Sep 11, 2023

As someone who has been proactively advocating using ESLint for formatting:

I am happy to see formatting rules being deprecated in the core if they can help save the maintenance efforts, as I stated in eslint/eslint.org#435 (comment).

To me, neither Prettier nor dprint is good enough to replace what we have right now in ESLint. So I think it's still hard for me to recommend them as alternatives. Instead of feeling bad about recommending ESLint to format and increasing the maintenance burden to the ESLint and typescript-eslint team, I think deprecating them and leaving the formatting/styling rules to the community is a good idea.

If that ends up happening, I'd be happy to initiate an org/community to fork and maintain those stylistic rules. So those efforts can be left to contributors we care about formatting and reducing the burden on the core teams 👍

Thanks a lot for what you have done for the community. ESLint really helped me a lot with maintaining clean and efficient codebases. 💚

@Shinigami92
Copy link
Contributor

With https://github.com/prettier/eslint-plugin-prettier we can also already use eslint to check if the style satisfies the repo's need.

Deprecating, removing and forking these stylistic rules into a community driven repo is a very good idea, because it makes the core smaller and the rules are specifically maintained by people who care.

I can just support this decision ❤️

@Mlocik97
Copy link

Mlocik97 commented Sep 11, 2023

@antfu There are multiple reasons why linters shouldn't care about formatting. Few already mentoyed here:

  • Doesn't scale
  • Error prone
  • Slower

But I agree that Prettier is not good formatter (even tho I use it). What is difference in those is that, ESLint care about syntax and semantics (and parses the code to meaningful chunks), processing with rules that checks about WHAT code is EXECUTED, not about HOW code LOOKS.

Formatters doesn't parse the code, doesn't care about syntax and semantics (excluding exceptions), it cares about white-spaces.

Actually ESLint could also include formatter, but that would need to run after Linting phase. Making it two separate things running in sequence. Pretty much what doing eslint && prettier does. But it can as well be separate package.

I guess we need alternative to Prettier that:

  • can work standalone (as formatters usualy do)
  • can co-work with Linters (specific API to borrow informations about code, would need to be implemented in both ESLint and the Formatter)
  • can have own rules about formatting, that could use informations from Linter.
  • can be as conigurable as possible, to the imagination of user.

For that I would propose to get rid of ESLint formatting rules, and instead implement API that allows formatters to simply "co-work" with ESLint. Giving high quality results.

@antfu
Copy link
Contributor

antfu commented Sep 11, 2023

@Mlocik97 I appreciate your input, but I am not sure if here is the right place to discuss what's in the scope of linter or formatter (as I probably can't convince you, either the other way around). I see ESLint more like a general AST-transformer/fixer with a very good rule/API design, despite there being a "Lint" in its name.

Solely for the reason of maintenance efforts, I would agree to deprecate them from the core to leave them to the community. In that case, I think we are on the same page :)

@eslint eslint locked as off-topic and limited conversation to collaborators Sep 11, 2023
@nzakas
Copy link
Member Author

nzakas commented Sep 11, 2023

Thanks for the input -- indeed, this isn't the right place to discuss linter vs. formatter. We're using this issue to track how we're moving forward with deprecating issues, so I'm locking it to ensure that we can focus on that. Please feel free to open a discussion if you'd like to free-form discuss things at a higher level.

@nzakas
Copy link
Member Author

nzakas commented Sep 28, 2023

I was talking with @antfu on Discord more about the plans for the stylistic plugin, and I think there's a good case to be made for also deprecating semi, quotes, and comma-dangle. My original concern with deprecating those was that they are pretty foundational to JavaScript and I didn't want to leave people without them. However, because @antfu is willing to maintain those rules as part of the plugin, I'm more comfortable with the idea of deprecating them. I'd say the same for rules that cover territory already covered by source code formatters like Prettier and dprint.

So, I'm unlocking this thread to continue that conversation. @bradzacher you've recommended against using certain rules in typescript-eslint, are there others besides semi, quotes and comma-dangle that overlap with ESLint?

@antfu
Copy link
Contributor

antfu commented Sep 28, 2023

Thanks for unlocking the issue and being so supportive of the stylistic plugin, Nicholas!

To add a bit of context here for people who are reading. To address the topic I mentioned in #17522 (comment), we created a GitHub organization https://github.com/eslint-stylistic/eslint-stylistic and started the work of porting stylistic/formatting-related rules from ESLint core and typescript-eslint to the dedicated repo. Rules will continue to be maintained by the community. Currently, we are focusing on 1:1 porting. The plugins themselves are already functional.

We are waiting for the final list of rules in order to ship the first v0.1 and start the maintenance work. We will work together with both teams to ensure the userland migration is straightforward.

If you have any ideas/suggestions for the project or rules, feel free to open issues at https://github.com/eslint-stylistic/eslint-stylistic, and avoid bloating this thread if possible. Thanks!

@bradzacher
Copy link
Contributor

@nzakas our stance is pretty well "if it can conflict with prettier then ditch it".
So just to be explicit and clear - this is the list (I've marked the ones not listed in yours/Milos's lists as (new)) that I think we can move to the stylistic community.
Note that some of these (eg padding-line-between-statements) are not covered by prettier/dprint - however they're purely stylistic whitespace rules so they fit the description regardless.

  • array-bracket-newline
  • array-bracket-spacing
  • array-element-newline
  • arrow-parens (new)
  • arrow-spacing
  • block-spacing
  • brace-style
  • comma-dangle (new)
  • comma-spacing
  • comma-style
  • computed-property-spacing
  • dot-location
  • eol-last (new)
  • func-call-spacing / no-spaced-func
  • function-call-argument-newline
  • function-paren-newline
  • generator-star-spacing
  • implicit-arrow-linebreak
  • indent / indent-legacy
  • jsx-quotes (new)
  • key-spacing
  • keyword-spacing
  • linebreak-style
  • lines-between-class-members
  • lines-around-comment
  • lines-around-directive
  • max-len (new)
  • max-statements-per-line (new)
  • multiline-ternary
  • new-parens (new)
  • newline-after-var
  • newline-before-return
  • newline-per-chained-call
  • no-confusing-arrow (new)
  • no-extra-parens (new)
  • no-extra-semi (new)
  • no-floating-decimal (new)
  • no-mixed-operators (new)
  • no-mixed-spaces-and-tabs
  • no-multi-spaces
  • no-multiple-empty-lines
  • no-tabs
  • no-trailing-spaces
  • no-unexpected-multiline (new)
  • no-whitespace-before-property
  • nonblock-statement-body-position
  • object-curly-newline
  • object-curly-spacing
  • object-property-newline
  • one-var-declaration-per-line
  • operator-linebreak
  • padded-blocks
  • padding-line-between-statements
  • quote-props (new)
  • quotes (new)
  • rest-spread-spacing
  • semi (new)
  • semi-spacing
  • semi-style
  • space-before-blocks
  • space-before-function-paren
  • space-in-parens
  • space-infix-ops
  • space-unary-ops
  • spaced-comment
  • switch-colon-spacing
  • template-curly-spacing
  • template-tag-spacing
  • wrap-iife (new)
  • wrap-regex (new)
  • yield-star-spacing
Notes on (new) rules

One final note: the prettier config disables both curly and no-unexpected-multiline. However these rules are not formatting rules:

  • curly is stylistic, but is not covered by prettier as (by-and-large) it does not change the AST when formatting. Dprint can be configured to add it.-,useBraces,-If%20braces%20should)
  • no-unexpected-multiline can warn against cases of unintentional multilines. Both formatters will always assume the AST is correct and will format code based on that. The rule can be used safely with formatters (and probably should be used!) for 99.99% of cases.

@nzakas
Copy link
Member Author

nzakas commented Sep 29, 2023

@bradzacher thanks for this exhaustive list and details!

I agree about no-unexpected-multline -- we added that as a way to find errors when people used no-semicolon style. And since we don't autofix this rule, it seems like a good candidate to stick around.

Similarly, curly is actually intended to help prevent errors caused by blockless if statements where indentation gets messed up and it's hard to see what statements will get executed.

All of the others labeled as "(new)" I'm open to also deprecating, pending consensus from the team.

@eyalroz
Copy link

eyalroz commented Oct 19, 2023

So, bottom line: As a (somewhat) newbie user who has so far used stylistic rules:

  • When, if ever, should I change my .eslintrc file?
  • What should I remove from it?
  • How do I convert my ESLint formatting rules into rules for "prettier"?
  • Where should I go to study the use of prettier instead of ESLint? Is their github repo sufficient?
  • What prettier plugins should I install, and from where?

(Perhaps a wiki page about this would be appropriate.)

@Shinigami92
Copy link
Contributor

So, bottom line: As a (somewhat) newbie user who has so far used stylistic rules:

  • When, if ever, should I change my .eslintrc file?
  • What should I remove from it?
  • How do I convert my ESLint formatting rules into rules for "prettier"?
  • Where should I go to study the use of prettier instead of ESLint? Is their github repo sufficient?
  • What prettier plugins should I install, and from where?

(Perhaps a wiki page about this would be appropriate.)

For some parts I'm with you.
But leave out prettier, because this has nothing to do with eslint.
The rules that will be deprecated in eslint will be ported over to https://github.com/eslint-stylistic/eslint-stylistic. So when you use some of these rules today, you will loose nothing.

@nzakas
Copy link
Member Author

nzakas commented Oct 20, 2023

@eyalroz We will have a blog post announcing the deprecation and the steps you should take. Stay tuned. In the meantime if you have specific questions, please open a discussion or stop by Discord rather than commenting here.

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Oct 23, 2023

Following up on rules such as curly and padding-line-between-statements (eslint-stylistic/eslint-stylistic#26 -> eslint-stylistic/eslint-stylistic#26 (comment)):

I'd be happy to have them here as long as ESLint team also agrees on that.

Should the discussion for whether to additionally move not-quite-covered-by-Prettier stylistic rules over be in this issue or a new one?

Edit: moved to #17681 per 👇

@nzakas
Copy link
Member Author

nzakas commented Oct 24, 2023

We've already finalized the list to deprecate above (and drafted the announcement: eslint/eslint.org#482).

If you'd like to discuss deprecating other rules, then let's start a new issue.

@nzakas
Copy link
Member Author

nzakas commented Oct 31, 2023

Working on this

@nzakas nzakas self-assigned this Oct 31, 2023
@mdjermanovic mdjermanovic removed the breaking This change is backwards-incompatible label Oct 31, 2023
@pubmikeb
Copy link

pubmikeb commented Nov 3, 2023

Following #17692, what about sort-keys and sort-vars rules?

@nzakas
Copy link
Member Author

nzakas commented Nov 3, 2023

@pubmikeb your question was answered on the discussion. If you'd like to suggest other rules to deprecate, please open a separate issue.

mdjermanovic pushed a commit that referenced this issue Nov 3, 2023
* feat: Deprecate formatting rules

Fixes #17522

* Add deprecation notice to rule files

* Fix tests

* Update eslint:all test

* Add deprecation notice to docs

* Clarify error message for rule check

* Update eslint:all test
@Alhadis

This comment was marked as off-topic.

@wJoenn
Copy link
Sponsor

wJoenn commented Nov 8, 2023

@Alhadis 8.53.0 does not disable these rules. It just mark them as deprecated
I don't even get a deprecation error when using them so it's basically like if nothing had changed so far.
Plus they announced that even if a deprecation warning is displayed, those rules would probably not be removed before 10.0.0

Finally if you wanna keep using them without deprecation warning just use Antfu's port https://eslint.style/
They're the exact same rules but another team's gonna handle their maintenance now, all you have to do is change their name in your config

@Alhadis

This comment was marked as off-topic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features deprecation This change deprecates an existing feature enhancement This change enhances an existing feature of ESLint
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

16 participants