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

Tooling: Adopt or run once good lints from eslint unicorn #1449

Open
rubiesonthesky opened this issue Mar 24, 2024 · 3 comments
Open

Tooling: Adopt or run once good lints from eslint unicorn #1449

rubiesonthesky opened this issue Mar 24, 2024 · 3 comments
Labels
area: tooling Managing the repository's maintenance 🛠️ status: blocked Waiting for something else to be resolved 🙅

Comments

@rubiesonthesky
Copy link
Collaborator

🚀 Feature Request

This should be done only after #1435 (#1318) has been merged.

I see that some of the code in the repo is older and does not follow some practices that are used nowadays. Eslint unicorn has some helpful lints to find these.

Run at least once:

  • no-empty-file - there is one empty file src/mutators/builtIn/fixIncompleteTypes/fixIncompleteReactTypes/reactFiltering/reactUsageFiltering.ts. I think it could be just removed separately.
  • no-instanceof-array - I think this is no-brainer and should be fixed.
  • no-lonely-if - makes code easier to read
  • no-negated-condition - I think there were just few issues with this. But the code is easier to read after.
  • prefer-at - This has few nice fixes
  • prefer-node-protocol - a lot imports already use node:, this could unify them. This kind of rule will be in next major version of eslint-plugin-n too, I think.
  • prefer-string-replace-all - fixed some things, where regex was unnecessary actually.

On the fence:

  • no-array-reduce - there are some reduces in the code and I think they are fine. Or they need more thinking, how they could be removed, if wanted.
  • no-for-loop - I'm not sure, will this help in this code base. Since performance is pretty critical in tool like this, it should be checked will this affect performance in any meaningful way.
  • prefer-spread this is more stylistics thing.

If recommended set is adopted, these are the rules I had to disable locally while testing. Some of them are duplicate what ts-eslint has (e.g., prefer-module), some are just opposite what the repo uses (e.g., filename-case), and some of them can be dangerous (e.g., better-regex).

"unicorn/better-regex": "off",
"unicorn/explicit-length-check": "off",
"unicorn/filename-case": "off",
"unicorn/no-array-callback-reference": "off",
"unicorn/no-array-for-each": "off",
"unicorn/no-await-expression-member": "off",
"unicorn/no-nested-ternary": "off",
"unicorn/no-useless-undefined": "off",
"unicorn/prefer-module": "off",
"unicorn/prevent-abbreviations": "off",
"unicorn/switch-case-braces": "off",
@JoshuaKGoldberg JoshuaKGoldberg added status: in discussion Not yet ready for implementation or a pull request area: tooling Managing the repository's maintenance 🛠️ labels Mar 24, 2024
@JoshuaKGoldberg
Copy link
Owner

I like it! This is related to the upstream (in CTA) issue: JoshuaKGoldberg/create-typescript-app#1116.

@rubiesonthesky this could be a fun opportunity for you to contribute to a few libraries:

  1. Add the "unopinionated" config to eslint-plugin-unicorn
  2. Add usage of it to create-typescript-app
  3. Pull in those changes here

WDYT?

@rubiesonthesky
Copy link
Collaborator Author

Yeah that makes sense :)

@JoshuaKGoldberg JoshuaKGoldberg added status: blocked Waiting for something else to be resolved 🙅 and removed status: in discussion Not yet ready for implementation or a pull request labels Mar 25, 2024
@JoshuaKGoldberg
Copy link
Owner

Great! In that case marking as blocked on eslint-plugin-unicorn providing that "unopinionated" config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tooling Managing the repository's maintenance 🛠️ status: blocked Waiting for something else to be resolved 🙅
Projects
None yet
Development

No branches or pull requests

2 participants