docs(linter): Update various rules to note that they can be disabled for TS code.#16819
docs(linter): Update various rules to note that they can be disabled for TS code.#16819graphite-app[bot] merged 1 commit intomainfrom
Conversation
github-actions
bot
added
A-linter
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
A few rules are documented as universally safe to disable under TypeScript, but at least no-undef is not reliably redundant across common TS configurations and mixed JS/TS setups. The no-unreachable doc is already conditional on allowUnreachableCode, but it could further clarify that the file must actually be type-checked by tsc. The no-import-assign doc does a good job of being nuanced; consider aligning other “can be disabled” notes to that same standard.
Additional notes (2)
-
Maintainability |
crates/oxc_linter/src/rules/eslint/no_undef.rs:30-39
The added guidance says this rule “can be disabled for TypeScript code” becausetscenforces it. That’s not reliably true in common configurations: -
In TS,
noImplicitAny: falseand/or JS-file checking disabled (allowJswithoutcheckJs) can allow undeclared identifiers to slip through. -
Even in
.ts, names can flow in viadeclare const ...in ambient.d.ts, global augmentations, or/// <reference ...>in ways that don’t correspond to runtime globals;tscmay be satisfied while runtime still fails.
Given this is linter-rule documentation, the doc should be more conditional (or caveated) to avoid pushing users to disable a rule that still catches real JS/TS runtime issues depending on project settings.
- Readability |
crates/oxc_linter/src/rules/eslint/no_unreachable.rs:24-31
The new note implies the TypeScript compiler enforces this check. This is only true whenallowUnreachableCodeis effectivelyfalse(the default) and the code is being typechecked bytsc. In many repos, linting can run on files or build steps that TS doesn’t typecheck (e.g., mixed JS/TS,transpileOnly, isolated module builds). You already added a config condition in the doc, which is good—but it might still overpromise that the compiler will always catch it.
Given you’re already mentioning allowUnreachableCode, consider also clarifying the scope: “when the file is type-checked by TypeScript” to prevent misinterpretation in mixed-language setups.
Summary of changes
Summary
This diff is documentation-focused and updates several ESLint-rule docs within crates/oxc_linter to clarify when rules are redundant under TypeScript.
What changed
- Added a consistent note to multiple rule docs stating they can be disabled for TypeScript because the TypeScript compiler enforces the same check:
constructor-super,no-class-assign,no-dupe-class-members,no-dupe-keys,no-func-assign,no-new-native-nonconstructor,no-obj-calls,no-setter-return,no-this-before-super,no-undef,no-unsafe-negation.
- Refined the
no-unreachablenote to be conditional ontsconfig.json(allowUnreachableCode: false). - Expanded
no-import-assigndocs to clarify TS usually catches this, but not always, so keeping the rule can still add value. - Standardized capitalization in the
no-import-assigndiagnostic text and updated the corresponding snapshot:crates/oxc_linter/src/snapshots/eslint_no_import_assign.snap. - Minor punctuation/wording tweaks (e.g., adding missing periods, formatting
ReferenceErroras code).
There was a problem hiding this comment.
Pull request overview
This PR updates documentation for several ESLint rules to note that they can be disabled when using TypeScript, since the TypeScript compiler already enforces these checks. Additionally, it standardizes diagnostic message capitalization to use sentence case (starting with capital letters).
- Updated 12 ESLint rules to document their TypeScript compiler equivalents
- Standardized diagnostic messages for
no-import-assignto use sentence case - Added conditional language for
no-unreachableregarding TypeScript configuration
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_linter/src/rules/eslint/constructor_super.rs | Added note that TypeScript compiler enforces this check |
| crates/oxc_linter/src/rules/eslint/no_class_assign.rs | Added note that TypeScript compiler enforces this check |
| crates/oxc_linter/src/rules/eslint/no_dupe_class_members.rs | Added note that TypeScript compiler enforces this check; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_dupe_keys.rs | Moved TypeScript note to "What it does" section from "Why is this bad"; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_func_assign.rs | Added note that TypeScript compiler enforces this check; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_import_assign.rs | Capitalized diagnostic messages; updated to note TypeScript generally enforces this with caveats; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_new_native_nonconstructor.rs | Added note that TypeScript compiler enforces this check; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_obj_calls.rs | Standardized TypeScript note wording; maintained period in "What it does" |
| crates/oxc_linter/src/rules/eslint/no_setter_return.rs | Added note that TypeScript compiler enforces this check |
| crates/oxc_linter/src/rules/eslint/no_this_before_super.rs | Added note that TypeScript compiler enforces this check; added backticks around ReferenceError |
| crates/oxc_linter/src/rules/eslint/no_undef.rs | Added note that TypeScript compiler enforces this check; line-wrapped "Why is this bad" |
| crates/oxc_linter/src/rules/eslint/no_unreachable.rs | Added conditional note about TypeScript compiler enforcement with config requirement; added period to "What it does" |
| crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs | Added note that TypeScript compiler enforces this check |
| crates/oxc_linter/src/snapshots/eslint_no_import_assign.snap | Capitalized all diagnostic warning and help messages to use sentence case |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #16819 will not alter performanceComparing Summary
Footnotes
|
Merge activity
|
…for TS code. (#16819) This is generally based on the `handled_by_typescript` flag in the original ESLint rule definitions. All of these are flagged in the original rules as being "handled by TypeScript", and so I deferred to that when determining which rules to note this on.
graphite-app
bot
force-pushed
the
ts-enforces-docs
branch
from
a47e6d4 to
ba8fe68
Compare
This is generally based on the
handled_by_typescriptflag in the original ESLint rule definitions. All of these are flagged in the original rules as being "handled by TypeScript", and so I deferred to that when determining which rules to note this on.