feat(eslint-plugin): remove formatting/layout rules #8833

JoshuaKGoldberg · 2024-04-04T01:56:39Z

BREAKING CHANGE: removes many rules.

PR Checklist

Addresses an existing open issue: fixes 💀 Rule deletions: formatting (layout) rules #8074
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Deletes the rules marked as type 'layout', as well as handling for them in the config and website tooling.

I confirmed locally that https://github.com/eslint-stylistic/eslint-stylistic does rely on the 'layout' constituent for meta.type (#8695). But they only ever set type: 'layout' and later use rule as unknown as. So this can be worked around in userland.

💖

typescript-eslint · 2024-04-04T01:56:42Z

Thanks for the PR, @JoshuaKGoldberg!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

nx-cloud · 2024-04-04T01:58:09Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 94a530d. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 31 targets

Sent with 💌 from NxCloud.

codecov · 2024-04-04T02:25:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.42%. Comparing base (716b783) to head (5702dc2).
Report is 27 commits behind head on v8.

❗ Current head 5702dc2 differs from pull request most recent head 94a530d. Consider uploading reports for the commit 94a530d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##               v8    #8833      +/-   ##
==========================================
- Coverage   88.02%   86.42%   -1.61%     
==========================================
  Files         405      234     -171     
  Lines       14089    11423    -2666     
  Branches     4125     3575     -550     
==========================================
- Hits        12402     9872    -2530     
+ Misses       1382     1291      -91     
+ Partials      305      260      -45

Flag	Coverage Δ
unittest	`86.42% <ø> (-1.61%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ckages/eslint-plugin/src/util/getESLintCoreRule.ts	`50.00% <ø> (ø)`
packages/utils/src/ts-eslint/ESLint.ts	`50.00% <ø> (ø)`

... and 150 files with indirect coverage changes

packages/utils/src/ts-eslint/Rule.ts

armano2 · 2024-04-08T22:08:19Z

seems good to me

…script-eslint#8815) * feat(typescript-estree): add defaultProject for project service * Thanks Jake, service.setCompilerOptionsForInferredProjects * Cleaned up a bit and added happy path testing * Update packages/typescript-estree/src/parser-options.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com> --------- Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

…onstructor` (typescript-eslint#8895) Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

bradzacher

Delete it all. Yes. Die.

The only suggestion I would have:
like camelcase - I think it would be best to leave tombstones behind to forward people to the new rules.

Why? Because people running on older versions will "click to open docs" in their IDE and it will open our site to https://typescript-eslint.io/rules/block-spacing/. They'll get a 404 page and be completely confused and probably file an issue.

Or they'll be using ESLint v8 and notice comma-dangle doesn't work as expected with TS constructs and they'll search our repo for a replacement (as is the standard when looking for extension rules) and find no existing rule -then request it cos they didn't read any other docs.

It'll cost us nothing to keep tombstones around but it'll help orient people and cut off useless support requests.

armano2 · 2024-04-16T14:52:03Z

Delete it all. Yes. Die.

The only suggestion I would have: like camelcase - I think it would be best to leave tombstones behind to forward people to the new rules.

Why? Because people running on older versions will "click to open docs" in their IDE and it will open our site to https://typescript-eslint.io/rules/block-spacing/. They'll get a 404 page and be completely confused and probably file an issue.

Or they'll be using ESLint v8 and notice comma-dangle doesn't work as expected with TS constructs and they'll search our repo for a replacement (as is the standard when looking for extension rules) and find no existing rule -then request it cos they didn't read any other docs.

It'll cost us nothing to keep tombstones around but it'll help orient people and cut off useless support requests.

what do you think about releasing old version of docs at https://v7.typescript-eslint.io/, that would let ppl who still use old version have access to it

bradzacher · 2024-04-16T21:24:21Z

@armano2 we could do that. The problem there is discoverability - the rules are setup to link directly to typescript-eslint.io/rules/rule-name so if someone uses the "open docs" command then they're not gonna see the legacy docs.

I'm personally not too miffed about older versions losing docs - it's going to be a small fraction of users that actively need the old docs.
Each time this has been brought up in the past our collective response has been "it's just not worth the effort".

* feat(eslint-plugin): remove formatting/layout rules * Remove "layout" * feat(typescript-estree): add defaultProject for project service (#8815) * feat(typescript-estree): add defaultProject for project service * Thanks Jake, service.setCompilerOptionsForInferredProjects * Cleaned up a bit and added happy path testing * Update packages/typescript-estree/src/parser-options.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com> --------- Co-authored-by: Brad Zacher <brad.zacher@gmail.com> * feat(eslint-plugin): replace `no-new-symbol` with `no-new-native-nonconstructor` (#8895) Co-authored-by: Brad Zacher <brad.zacher@gmail.com> * Add tombstones * 'deprecated' typo * Let's mention both issues * Fix unused variable issue --------- Co-authored-by: Brad Zacher <brad.zacher@gmail.com> Co-authored-by: Dave <reduckted@outlook.com>

JoshuaKGoldberg added this to the 8.0.0 milestone Apr 4, 2024

JoshuaKGoldberg mentioned this pull request Apr 4, 2024

Bug: RuleCreator leaks internal meta.docs, meta.type types #8695

Closed

4 tasks

feat(eslint-plugin): remove formatting/layout rules

5702dc2

JoshuaKGoldberg force-pushed the remove-formatting-rules branch from 5d779d9 to 5702dc2 Compare April 4, 2024 02:11

JoshuaKGoldberg marked this pull request as ready for review April 4, 2024 02:24

armano2 reviewed Apr 4, 2024

View reviewed changes

packages/utils/src/ts-eslint/Rule.ts Show resolved Hide resolved

Remove "layout"

559ec8d

JoshuaKGoldberg requested a review from armano2 April 8, 2024 17:12

armano2 approved these changes Apr 14, 2024

View reviewed changes

JoshuaKGoldberg mentioned this pull request Apr 14, 2024

fix(typescript-estree): add TSEnumBody node for TSEnumDeclaration body #8920

Merged

3 tasks

JoshuaKGoldberg and others added 2 commits April 15, 2024 09:39

feat(eslint-plugin): replace no-new-symbol with `no-new-native-nonc…

fda5704

…onstructor` (typescript-eslint#8895) Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

bradzacher reviewed Apr 15, 2024

View reviewed changes

bradzacher added the breaking change This change will require a new major version to be released label Apr 15, 2024

JoshuaKGoldberg added 9 commits April 15, 2024 10:51

Add tombstones

fd06a27

Merge branch 'main'

0614d7c

Merge branch 'v8' into remove-formatting-rules

5648157

Merge branch 'main' into v8

9b506c4

Merge branch 'v8' into remove-formatting-rules

da3c244

'deprecated' typo

872c5db

Let's mention both issues

e9f07a2

Merge branch 'v8'

2dd6434

Fix unused variable issue

637367e

JoshuaKGoldberg requested a review from bradzacher April 15, 2024 15:59

bradzacher approved these changes Apr 16, 2024

View reviewed changes

Merge branch 'v8'

94a530d

JoshuaKGoldberg merged commit 3f1cfda into typescript-eslint:v8 Apr 19, 2024
53 checks passed

JoshuaKGoldberg mentioned this pull request Apr 22, 2024

💀 Rule deletions: formatting (layout) rules #8074

Closed

github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): remove formatting/layout rules #8833

feat(eslint-plugin): remove formatting/layout rules #8833

JoshuaKGoldberg commented Apr 4, 2024 •

edited

Loading

typescript-eslint bot commented Apr 4, 2024

nx-cloud bot commented Apr 4, 2024 •

edited

Loading

codecov bot commented Apr 4, 2024 •

edited

Loading

armano2 commented Apr 8, 2024

bradzacher left a comment

armano2 commented Apr 16, 2024

bradzacher commented Apr 16, 2024

feat(eslint-plugin): remove formatting/layout rules #8833

feat(eslint-plugin): remove formatting/layout rules #8833

Conversation

JoshuaKGoldberg commented Apr 4, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Apr 4, 2024

nx-cloud bot commented Apr 4, 2024 • edited Loading

☁️ Nx Cloud Report

codecov bot commented Apr 4, 2024 • edited Loading

Codecov Report

armano2 commented Apr 8, 2024

bradzacher left a comment

Choose a reason for hiding this comment

armano2 commented Apr 16, 2024

bradzacher commented Apr 16, 2024

JoshuaKGoldberg commented Apr 4, 2024 •

edited

Loading

nx-cloud bot commented Apr 4, 2024 •

edited

Loading

codecov bot commented Apr 4, 2024 •

edited

Loading