Skip to content

refactor(linter/plugins): enable TS strict option#19675

Merged
graphite-app[bot] merged 1 commit intomainfrom
02-24-chore_apps_oxlint_enable_typescript_strict_option
Mar 6, 2026
Merged

refactor(linter/plugins): enable TS strict option#19675
graphite-app[bot] merged 1 commit intomainfrom
02-24-chore_apps_oxlint_enable_typescript_strict_option

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Feb 24, 2026

Enable TypeScript strict option in apps/oxlint.

@github-actions github-actions bot added A-linter Area - Linter A-parser Area - Parser A-cli Area - CLI A-transformer Area - Transformer / Transpiler A-ast-tools Area - AST tools labels Feb 24, 2026
Copy link
Member Author

sapphi-red commented Feb 24, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-formatter Area - Formatter A-linter-plugins Area - Linter JS plugins C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Feb 24, 2026
@sapphi-red sapphi-red force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from 36ca452 to cf873b9 Compare February 24, 2026 11:25
@sapphi-red sapphi-red changed the title chore(apps/oxlint): enable TypeScript strict option fix(linter/plugins): enable TS strict option and fix type definitions for visitor Feb 24, 2026
@github-actions github-actions bot added the C-bug Category - Bug label Feb 24, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 24, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 46 skipped benchmarks1


Comparing 02-24-chore_apps_oxlint_enable_typescript_strict_option (36ca452) with main (0722721)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sapphi-red sapphi-red marked this pull request as ready for review February 24, 2026 11:32
Copilot AI review requested due to automatic review settings February 24, 2026 11:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes TypeScript type errors in the linter's JS plugin system by enabling strict mode and implementing a bivariance hack for visitor objects. The bivariance hack allows visitor functions with specific node types (e.g., CallExpression) to be assigned to properties expecting more generic node types, which is necessary for the plugin system to work correctly with TypeScript's strict type checking.

Changes:

  • Enabled TypeScript strict mode in apps/oxlint/tsconfig.json to catch type issues
  • Implemented bivariance hack pattern for VisitorObject type to fix variance issues with visitor function parameters
  • Added type assertions and nullish coalescing operators to satisfy strict null checks
  • Added type definition tests to verify visitor object compatibility

Reviewed changes

Copilot reviewed 13 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tasks/ast_tools/src/generators/raw_transfer.rs CRITICAL BUGS: Added incorrect import and type for NodeOrToken in generated deserialize type definitions
tasks/ast_tools/src/generators/estree_visit.rs Implemented bivariance hack pattern for visitor types to allow contravariant parameter types
apps/oxlint/src-js/generated/visitor.d.ts Generated visitor types with bivariance hack implementation
apps/oxlint/src-js/generated/deserialize.d.ts Generated with incorrect NodeOrToken type usage (from raw_transfer.rs bugs)
apps/oxlint/src-js/visitor.test-d.ts Added type-level tests to validate visitor object type compatibility
apps/oxlint/src-js/plugins/visitor.ts Added type assertions for strict mode compatibility
apps/oxlint/src-js/plugins/cfg.ts Added type assertions for strict mode compatibility
apps/oxlint/src-js/cli.ts Added undefined handling with nullish coalescing for strict null checks
apps/oxlint/src-js/package/rule_tester.ts Improved function signatures with proper this parameter typing and disabled unbound-method warnings
apps/oxlint/conformance/src/rule_tester.ts Added oxlint-disable comment for intentional unbound method usage
apps/oxlint/tsconfig.json Enabled strict mode and added node types
napi/parser/tsconfig.json Added node types configuration
napi/transform/tsconfig.json Added node types configuration
apps/oxfmt/tsconfig.json Added node types configuration
apps/oxlint/package.json Added @type-challenges/utils dependency for type tests
pnpm-lock.yaml Updated lock file with new dependency
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

@overlookmotel overlookmotel changed the base branch from main to graphite-base/19675 March 6, 2026 09:30
@overlookmotel overlookmotel force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from d15b14a to 40621dc Compare March 6, 2026 09:30
@overlookmotel overlookmotel changed the base branch from graphite-base/19675 to om/03-06-fix_linter_plugins_fix_type_definition_for_visitorobject_ March 6, 2026 09:30
@overlookmotel overlookmotel changed the title fix(linter/plugins): enable TS strict option and fix type definitions for visitor refactor(linter/plugins): enable TS strict option Mar 6, 2026
@overlookmotel overlookmotel marked this pull request as draft March 6, 2026 09:44
@overlookmotel overlookmotel marked this pull request as ready for review March 6, 2026 15:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

apps/oxlint/conformance/src/rule_tester.ts:112

  • test.before = function (this) { ... } relies on a this parameter without a type annotation. With TS strict (enables noImplicitThis), this form typically fails type-checking. Add an explicit this type (e.g. function (this: T) { ... } / unknown) and keep originalBefore typed with the same this so hooks that depend on their this context remain correctly typed.
    const originalBefore = test.before as () => void;
    test.before = function (this) {
      setCurrentTest(clonedTest);
      originalBefore!.call(this);
    };

@graphite-app graphite-app bot changed the base branch from om/03-06-fix_linter_plugins_fix_type_definition_for_visitorobject_ to graphite-base/19675 March 6, 2026 15:59
graphite-app bot pushed a commit that referenced this pull request Mar 6, 2026
Fixes #18154, #14745.

Use the bivariance hack to fix the type def for `VisitorObject`. The bivariance hack is taken from DefinitelyTyped/DefinitelyTyped#20219.

This was originally written by @sapphi-red in #19675. I've split it out into a separate PR. I can't claim to understand this witchcraft!
@graphite-app graphite-app bot force-pushed the graphite-base/19675 branch from ecef907 to 602daaa Compare March 6, 2026 16:08
@graphite-app graphite-app bot force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from 9904a4a to 9100c63 Compare March 6, 2026 16:08
@graphite-app graphite-app bot changed the base branch from graphite-base/19675 to main March 6, 2026 16:08
@graphite-app graphite-app bot force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from 9100c63 to cce88d5 Compare March 6, 2026 16:09
@overlookmotel overlookmotel force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from cce88d5 to a7ff967 Compare March 6, 2026 16:17
@overlookmotel overlookmotel removed C-bug Category - Bug A-parser Area - Parser A-transformer Area - Transformer / Transpiler A-ast-tools Area - AST tools A-formatter Area - Formatter labels Mar 6, 2026
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Mar 6, 2026
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 6, 2026

Merge activity

Enable TypeScript strict option in `apps/oxlint`.
@graphite-app graphite-app bot force-pushed the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch from a7ff967 to e255fcd Compare March 6, 2026 17:30
@github-actions github-actions bot added the A-ast-tools Area - AST tools label Mar 6, 2026
graphite-app bot pushed a commit that referenced this pull request Mar 6, 2026
Add a types test for #20065. This is a separate PR as it depends on `strict: true` TS config option added in #19675.
@graphite-app graphite-app bot merged commit e255fcd into main Mar 6, 2026
19 checks passed
@graphite-app graphite-app bot deleted the 02-24-chore_apps_oxlint_enable_typescript_strict_option branch March 6, 2026 17:37
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 6, 2026
graphite-app bot pushed a commit that referenced this pull request Mar 6, 2026
…e not `undefined` (#20070)

Follow-on after #19675.

The type for `lint` function that NAPI-RS produces is incorrect. `Option::None` on Rust side is translated to `null`, never `undefined`. Check this with a debug assertion, instead of handling `undefined` at runtime (which is pointless, because it never is `undefined`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast-tools Area - AST tools A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants