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

feat(linter): configure by category in config files #6120

Conversation

DonIsaac
Copy link
Contributor

closes #5454

Adds a categories property to config files, where each key is a RuleCategory and each value is "allow"/"off", "warn", or "deny"/"error".

Note that this change won't come into effect until after #6088 is merged.

Copy link

graphite-app bot commented Sep 27, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@DonIsaac DonIsaac added the C-enhancement Category - New feature or request label Sep 27, 2024 — with Graphite App
Copy link
Contributor Author

DonIsaac commented Sep 27, 2024

@DonIsaac DonIsaac force-pushed the don/09-27-refactor_linter_add_schemars_and_serde_traits_to_allowwarndeny_and_rulecategories branch from 04aab0c to 5b9cb7e Compare September 27, 2024 17:21
@DonIsaac DonIsaac force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch from b1040fa to 055e407 Compare September 27, 2024 17:21
@github-actions github-actions bot added the A-linter Area - Linter label Sep 27, 2024
@DonIsaac DonIsaac force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch from 055e407 to c681bfe Compare September 27, 2024 17:29
@DonIsaac DonIsaac linked an issue Sep 27, 2024 that may be closed by this pull request
@DonIsaac DonIsaac force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch from c681bfe to 5555651 Compare September 27, 2024 17:33
@DonIsaac DonIsaac marked this pull request as ready for review September 27, 2024 17:34
Copy link

codspeed-hq bot commented Sep 27, 2024

CodSpeed Performance Report

Merging #6120 will not alter performance

Comparing don/09-27-feat_linter_configure_by_category_in_config_files (6e3224d) with main (c828b70)

Summary

✅ 29 untouched benchmarks

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

To be honest I haven't seen a single usage of these categories in the wild ... since it's a foreign concept from Rust.

Should we actually try and reduce complexity at this stage?

Or maybe it's just that there is no recommended usage documentation for these categories.

@DonIsaac
Copy link
Contributor Author

To be honest I haven't seen a single usage of these categories in the wild ... since it's a foreign concept from Rust.

  1. 5 our of 8 non-skipped repos we test oxlint-ecosystem-ci use these categories..
  2. The way they are using these flags will break once feat(linter)!: support plugins in oxlint config files #6088 merges. This PR addresses the same user story that CLI flags (at least, the way these repos and many others are using them) used to solve.
  3. I've seen various issues and Discord messages asking for this feature. I think it's something people want.
  4. This would allow users who rely on the VSCode extension to perform broad-category rule configuration, something that is not currently possible (you can't configure CLI flags when running the LSP).

Should we actually try and reduce complexity at this stage?
Yes. I have two thoughts on this:

  1. This integrates directly with existing filters. It's mostly just a change to Oxlintrc
  2. Oxlintrc is meant to mimic eslintrc, so we can become backwards-compatible. In the future I see us having our own, more refined, config format.
  3. That being said, we should be careful what features we add to this config file

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Sep 28, 2024
Copy link

graphite-app bot commented Sep 28, 2024

Merge activity

  • Sep 27, 10:55 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Sep 29, 11:04 AM EDT: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.
  • Sep 29, 11:09 AM EDT: The Graphite merge queue couldn't merge this PR because it had conflicts with the trunk branch.
  • Sep 29, 11:10 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • Oct 8, 6:18 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Oct 8, 6:18 PM EDT: DonIsaac added this pull request to the Graphite merge queue.
  • Oct 8, 6:23 PM EDT: DonIsaac merged this pull request with the Graphite merge queue.

@DonIsaac DonIsaac force-pushed the don/09-27-refactor_linter_add_schemars_and_serde_traits_to_allowwarndeny_and_rulecategories branch 2 times, most recently from 06584b5 to 775b03a Compare September 29, 2024 14:58
@Boshen Boshen changed the base branch from don/09-27-refactor_linter_add_schemars_and_serde_traits_to_allowwarndeny_and_rulecategories to graphite-base/6120 September 29, 2024 15:03
@Boshen Boshen force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch from 5555651 to 8e58dea Compare September 29, 2024 15:04
@Boshen Boshen changed the base branch from graphite-base/6120 to main September 29, 2024 15:04
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Sep 29, 2024
@DonIsaac DonIsaac force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch 2 times, most recently from cf74faf to 1fc6e58 Compare September 30, 2024 14:13
@DonIsaac DonIsaac added the 0-merge Merge with Graphite Merge Queue label Oct 8, 2024
> closes #5454

Adds a `categories` property to config files, where each key is a `RuleCategory` and each value is `"allow"/"off"`, `"warn"`, or `"deny"/"error"`.

Note that this change won't come into effect until after #6088 is merged.
@DonIsaac DonIsaac force-pushed the don/09-27-feat_linter_configure_by_category_in_config_files branch from 1fc6e58 to 6e3224d Compare October 8, 2024 22:19
@graphite-app graphite-app bot merged commit 6e3224d into main Oct 8, 2024
26 checks passed
@graphite-app graphite-app bot deleted the don/09-27-feat_linter_configure_by_category_in_config_files branch October 8, 2024 22:23
DonIsaac added a commit that referenced this pull request Oct 19, 2024
## [0.10.0] - 2024-10-18

- 7f6b219 editor/vscode: [**BREAKING**] Unify configuration logic
(#6630) (DonIsaac)

- 782f0a7 codegen: [**BREAKING**] Rename `print_char` method to
`print_ascii_byte` (#6512) (overlookmotel)

- 7645e5c codegen: [**BREAKING**] Remove CommentOptions API (#6451)
(Boshen)

- 5200960 oxc: [**BREAKING**] Remove passing `Trivias` around (#6446)
(Boshen)

- 80266d8 linter: [**BREAKING**] Support plugins in oxlint config files
(#6088) (DonIsaac)

### Features

- 6f22538 ecmascript: Add `ToBoolean`, `ToNumber`, `ToString` (#6502)
(Boshen)
- 1e7fab3 linter: Implement `no-callback-in-promise` (#6157) (dalaoshu)
- c56343d linter: Promote `no_unsafe_optional_chaining` to correctness
(#6491) (Boshen)
- 454874a linter: Implement `react/iframe-missing-sandbox` (#6383) (Radu
Baston)
- c8174e2 linter: Add suggestions for `no-plusplus` (#6376) (camchenry)
- 6e3224d linter: Configure by category in config files (#6120)
(DonIsaac)
- c5e66e1 linter/no-unused-vars: Report own type references within
class, interface, and type alias declarations (#6557) (DonIsaac)
- 8c78f97 linter/node: Implement no-new-require (#6165) (Jelle van der
Waa)

### Bug Fixes

- cf92730 editor: Use human-readable output channel names (#6629)
(DonIsaac)
- d9159a2 editor: Misaligned command prefixes (#6628) (DonIsaac)
- b9c94bb editors/vscode: Temporarily solve oxc_language_server issue on
windows (#6384) (dalaoshu)
- e340424 linter: Support import type with namespaced import in
`import/no-duplicates` (#6650) (Dmitry Zakharov)
- a668397 linter: Panic in `no-else-return` (#6648) (dalaoshu)
- 41dc8e3 linter: Stack overflow in `oxc/no-async-endpoint-handlers`
(#6614) (DonIsaac)
- d07a9b0 linter: Panic in `no-zero-fractions` (#6607) (dalaoshu)
- d6a0d2e linter: Fix file name checking behavior of
`unicorn/filename-case` (#6463) (camchenry)
- 0784e74 linter: Error fixer of `switch-case-braces` (#6474) (dalaoshu)
- e811812 linter: Error diagnostic message based on parameter length of
valid-expect (#6455) (dalaoshu)
- f71c91e linter: Move `eslint/sort-keys` to `style` category (#6377)
(DonIsaac)
- 2b86de9 linter/no-control-regex: False negative for flags in template
literals (#6531) (DonIsaac)
- 685a590 linter/no-control-regex: Better diagnostic messages (#6530)
(DonIsaac)
- 6d5a9f2 linter/no-control-regex: Allow capture group references
(#6529) (DonIsaac)
- ba53bc9 linter/no-unused-vars: False positives in TS type assertions
(#6397) (DonIsaac)
- d3e59c6 linter/no-unused-vars: False positive in some default export
cases (#6395) (DonIsaac)
- e08f956 linter/no-unused-vars: False positive for functions and
classes in arrays (#6394) (DonIsaac)
- b9d7c5f no-unused-vars: Consider functions within conditional
expressions usable (#6553) (Brian Donovan)

### Performance

- 0cbd4d0 linter: Avoid megamorphism in `RuleFixer` methods (#6606)
(DonIsaac)
- 725f9f6 linter: Get fewer parent nodes in
`unicorn/prefer-dom-node-text-content` (#6467) (camchenry)
- c00f669 linter: Use NonZeroUsize for pending module cache entries
(#6439) (DonIsaac)
- a1a2721 linter: Replace `ToString::to_string` with `CompactStr` in
remaining rules (#6407) (camchenry)
- c5c69d6 linter: Use `CompactStr` in `valid-title` (#6406) (camchenry)
- d66e826 linter: Use `CompactStr` in `prefer-lowercase-title` (#6405)
(camchenry)
- 889400c linter: Use `CompactStr` for `get_node_name` in Jest rules
(#6403) (camchenry)
- 9906849 linter: Use `CompactStr` in `no-large-snapshots` (#6402)
(camchenry)
- c382ec4 linter: Use `CompactStr` in `no-hooks` (#6401) (camchenry)
- 24a5d9b linter: Use `CompactStr` in `expect-expect` (#6400)
(camchenry)
- 71dbdad linter: Use `CompactStr` in `no-console` (#6399) (camchenry)
- f5f00a1 linter: Use `CompactStr` in `no-bitwise` (#6398) (camchenry)
- 62afaa9 linter/jsx-no-comment-textnodes: Remove regex for checking
comment patterns (#6534) (camchenry)
- b3d0cce linter/no-unescaped-entities: Add fast path to check if char
should be replaced (#6594) (camchenry)
- ee73f56 linter/no-unused-vars: Do not construct `Regex` for default
ignore pattern (#6590) (camchenry)
- 77ddab8 linter/numeric-separators-style: Replace regex with number
parser (#6546) (camchenry)
- 8f47cd0 linter/react: Remove regex patterns in `no-unknown-property`
(#6536) (camchenry)

### Documentation

- 557f941 linter: Add docs to no-unused-vars and Tester (#6558)
(DonIsaac)

### Refactor

- ecce5c5 linter: Improve recursive argument handling and diagnostics
creation (#6513) (no-yan)
- f960e9e linter: Add suggested file names for `unicorn/filename-case`
(#6465) (camchenry)
- 7240ee2 linter: Make advertised fix kinds consistent (#6461)
(Alexander S.)
- b48c368 linter: `no_global_assign` rule: reduce name lookups (#6460)
(overlookmotel)
- 2566ce7 linter: Remove OxlintOptions (#6098) (DonIsaac)
- 002078a linter: Make Runtime's members private (#6440) (DonIsaac)
- 6a0a533 linter: Move module cache logic out of Runtime (#6438)
(DonIsaac)
- c18c6e9 linter: Split service code into separate modules (#6437)
(DonIsaac)
- 5ea9ef7 linter: Improve labels and help message for
`eslint/no-useless-constructor` (#6389) (DonIsaac)
- 2c32dac linter/no-control-regex: Remove duplicate code (#6527)
(DonIsaac)
- 435a89c oxc: Remove useless `allocator.alloc(program)` calls (#6571)
(Boshen)
- f70e93b oxc: Ban index methods on std::str::Chars (#6075) (dalaoshu)

### Testing

- a6cae98 linter: Make sure all auto-fixing rules have fixer test
(#6378) (DonIsaac)
- 06b09b2 linter/no-unused-vars: Enable now-passing tests (#6556)
(DonIsaac)
- badd11c linter/no-unused-vars: Ignored catch parameters (#6555)
(DonIsaac)
- 84aa2a2 linter/no-useless-constructor: Add cases for initializers in
subclass constructors (#6390) (DonIsaac)

---------

Co-authored-by: DonIsaac <22823424+DonIsaac@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-merge Merge with Graphite Merge Queue A-linter Area - Linter C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow setting cli flags via .oxlintrc.json
4 participants