fix(linter/no-extraneous-class): improve docs, reporting and code refactor#10797
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughThe changes enhance the Diagnostic helper functions were updated to accept additional context, enabling conditional help messages for empty classes with decorators and tailored suggestions for classes containing only static members or only a constructor. The diagnostic span for empty classes was refined to exclude decorator spans by adjusting the span start position, improving diagnostic accuracy. The rule logic was modified to skip classes with superclasses or decorated classes when the Test cases were reorganized, cleaned up, and expanded to cover new scenarios such as abstract classes and decorated classes with large decorators. The tests were updated to align with the new default configuration and improved formatting. No public trait methods or exported functions were removed or fundamentally changed. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (11)
🔇 Additional comments (15)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs (2)
12-19: Consider derivingSerialize/Deserializeand relying on Serde for config parsingNow that the struct carries both a
JsonSchemaderive and Serde container attributes, hand-rolling the JSON lookup logic below is unnecessary and brittle (every new field requires touching two places).
You could instead do:#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase", default)] pub struct NoExtraneousClass { pub allow_constructor_only: bool, pub allow_empty: bool, pub allow_static_only: bool, pub allow_with_decorator: bool, } impl Default for NoExtraneousClass { fn default() -> Self { Self { /* … */ } } }and then in
from_configurationsimply:serde_json::from_value(value.get(0).cloned().unwrap_or_default()) .unwrap_or_default()This removes repetitive look-ups and guarantees that the schema, default impl and runtime parsing stay in sync.
Also applies to: 21-30
131-132: Minor consistency nit:The hard-coded default
unwrap_or(true)mirrors theDefaultimpl, but once Serde-based parsing (see first comment) is adopted, this duplication disappears.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/oxc_linter/src/snapshots/typescript_no_extraneous_class.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs(13 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs
[error] 153-153: Clippy lint 'cast_possible_truncation': casting usize to u32 may truncate the value on 64-bit targets. Use #[allow(clippy::cast_possible_truncation)] to allow or use try_from and handle the error.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Benchmark linter (1)
- GitHub Check: Benchmark linter (0)
🔇 Additional comments (1)
crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs (1)
98-107: Nice usability improvement!Adding concrete “how to fix” help messages markedly improves DX.
No issues spotted here.
CodSpeed Instrumentation Performance ReportMerging #10797 will not alter performanceComparing Summary
|
DonIsaac
force-pushed
the
don/05-04-fix_linter/no-extraneous-class_allow_empty_decorated_classes_by_default
branch
3 times, most recently
from
a72b7c7 to
5a68c5e
Compare
|
I do not like that we are changing the default values. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Merge activity
|
camc314
changed the title
…actor (#10797) ## What This PR Does Deviates from `@typescript-eslint/no-extraneous-class` by allowing empty, decorated classes by default. This provides a better experience for NestJS user using Oxlint without a config file. In NestJS, [`Module`s are often empty but use decorators to declaratively state their contents](https://docs.nestjs.com/modules#feature-modules). Other similar frameworks (TSeD, maybe Angular) will also benefit. ### Other Changes - Fix span on empty class diagnostics to not cover decorators. - Provide help messages on all diagnostics - Add missing links and rich text stuff to docs
graphite-app
bot
force-pushed
the
don/05-04-fix_linter/no-extraneous-class_allow_empty_decorated_classes_by_default
branch
from
e320c29 to
82889ae
Compare
graphite-app
bot
deleted the
don/05-04-fix_linter/no-extraneous-class_allow_empty_decorated_classes_by_default
branch
## [0.16.11] - 2025-05-16 ### Features - 078bf0b language_server: Better fallback handling when passing invalid `Options` values (#10930) (Sysix) - be7f7e1 language_server/editor: Support multi workspace folders (#10875) (Sysix) - eef93b4 linter: Add import/no-unassigned-import (#10970) (yefan) - cc0112f linter: No-unused-vars add setting for `reportVarsOnlyUsedAsTypes` (#11009) (camc314) - 17e49c3 linter: Implement configuration and checking loops for `eslint/no_constant_condition` (#10949) (Ulrich Stark) - 21117ac linter: Implement react/forbid-elements (#10928) (Thomas BOCQUEZ) - 466c24a linter: Add gitlab reporter output format (#10927) (Connor Pearson) - a064082 linter: Add import/consistent-type-specifier-style rule (#10858) (yefan) - 4733b52 linter/no-extraneous-class: Add conditional fixer (#10798) (DonIsaac) ### Bug Fixes - 87bf2a8 editor: Send only `workspace/didChangeConfiguration` when some workspace configuration is effected (#11017) (Sysix) - ed5708d editor: Detect all workspaces config path changes (#11016) (Sysix) - 89cc21b language_server: Normalize oxlintrc config path (#10982) (Sysix) - c52a9ba linter: Fix plugins inside overrides not being applied (#11057) (camc314) - b12bd48 linter: Fix rule config not being correctly applied (#11055) (camc314) - 9a368be linter: False negative in no-restriced-imports with `patterns` and side effects (#11027) (camc314) - 8c2cfbc linter: False negative in no-restricted-imports (#11026) (camc314) - 8956870 linter: False positive in no-unused-vars (#11002) (camc314) - 33a60d2 linter: Skip eslint/no-redeclare when running on modules (#11004) (camc314) - 39063ce linter: Reword diagnostic message for no-control-regex (#10993) (camc314) - 9eedb58 linter: False positive with negative matches in no-restricted-imports (#10976) (camc314) - 10e77d7 linter: Improve diagnostics for no-control-regex (#10959) (camc314) - 0961296 linter: Add `gitlab` to linter `--help` docs (#10932) (camc314) - 82889ae linter/no-extraneous-class: Improve docs, reporting and code refactor (#10797) (DonIsaac) - 11c34e7 linter/no-img-element: Improve diagnostic and docs (#10908) (DonIsaac) - 584d8b9 napi: Enable mimalloc `no_opt_arch` feature on linux aarch64 (#11053) (Boshen) - 126ae75 semantic: Distinguish class private elements (#11044) (magic-akari) - 773d0de semantic: Correctly handle nested brackets in jsdoc parsing (#10922) (camc314) - b215b6c semantic: Dont parse `@` as jsdoc tags inside `[`/`]` (#10919) (camc314) ### Documentation - db6afb9 linter: Improve docs of no-debugger (#11033) (camc314) - 16541de linter: Improve docs of default-param-last (#11032) (camc314) - 2c2f3c4 linter: Improve docs of default-case-last (#11031) (camc314) - 56bb9ce linter: Improve docs of array-callback-return (#11030) (camc314) - 13dbcc6 linter: Correct docs for default config for no-redeclare (#10995) (camc314) - a86cbb3 linter: Fix incorrect backticks of fenced code blocks (#10947) (Ulrich Stark) ### Refactor - 3cc1466 language_server: New configuration structure for `initialize` and `workspace/didChangeConfiguration` (#10890) (Sysix) - bd2ef7d language_server: Use `Arc` for `diagnostic_report_map` (#10940) (Sysix) - bb999a3 language_server: Avoid cloning linter by taking reference in LintService (#10907) (Ulrich Stark) - d1b0c83 linter: Remove overrides index vec (#11058) (camc314) - 7ad6cf8 linter: Store severity separately, remove `RuleWithSeverity` (#11051) (camchenry) - e31c361 linter: Remove nested match statements in no-restricted-imports (#10975) (camc314) - 6ad9d4f linter: Tidy `eslint/func-names` (#10923) (camc314) - faf0a95 syntax: Rename `NameSpaceModule` to `NamespaceModule` (#10917) (Dunqing) ### Testing - 76b6b33 editor: Add tests for multi workspace folder setup (#10904) (Sysix) Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
3 participants
What This PR Does
Deviates from
@typescript-eslint/no-extraneous-classby allowing empty, decorated classes by default.This provides a better experience for NestJS user using Oxlint without a config file. In NestJS,
Modules are often empty but use decorators to declaratively state their contents. Other similar frameworks (TSeD, maybe Angular) will also benefit.Other Changes