Experimental oxlint migration by AlbertSmit · Pull Request #13 · kaliberjs/eslint-plugin

AlbertSmit · 2025-10-22T15:30:52Z

Summary by CodeRabbit

New Features
- Added Oxlint integration as an alternative linting tool alongside ESLint.
Chores
- Added Oxlint configuration file for project-wide linting rules.
- Updated test scripts to support both ESLint and Oxlint: test:eslint and test:oxlint.
- Added Oxlint dependency.

…improved functionality

…uding linter environment detection and compatibility with Oxlint

… Oxlint, including enabling universal filename checks and adjusting rule severity levels.

socket-security · 2025-10-22T15:31:11Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@oxlint/linux-x64-musl@1.23.0
	@oxlint/win32-x64@1.23.0
	@oxlint/win32-arm64@1.23.0
	@oxlint/darwin-arm64@1.23.0
	@oxlint/darwin-x64@1.23.0
	@oxlint/linux-arm64-musl@1.23.0
	@oxlint/linux-arm64-gnu@1.23.0
	@oxlint/linux-x64-gnu@1.23.0
	oxlint@1.23.0

View full report

coderabbitai · 2025-10-22T15:31:14Z

Walkthrough

This pull request introduces Oxlint support alongside ESLint by adding Oxlint configuration, cross-linter compatibility utilities, and updating rule definitions to work with both linters through a new define-compatible-rule wrapper and defineRule API calls.

Changes

Cohort / File(s)	Summary
Oxlint Configuration & Documentation `.oxlintrc.json`, `README.md`	Added Oxlint configuration file with schema, environment presets, plugins, globals, and comprehensive ruleset; added usage instructions for oxlint experiment in README.
Core Package Configuration `package.json`, `index.js`	Added oxlint dependency (^1.23.0); split test script into test:eslint and test:oxlint; added meta property to exported configuration in index.js.
Cross-Linter Machinery `machinery/define-compatible-rule.js`, `machinery/oxlint-test-runner.js`	Introduced defineCompatibleRule utility that detects linter environment and transforms rules between Oxlint and ESLint formats; added test runner script for executing oxlint tests across rule directories.
AST Utilities `machinery/ast.js`	Expanded JSX utilities (getJSXElementName, getParentJSXElement, isRootJSXElement, hasParentsJSXElementsWithClassName, isInJSXBranch, getParentJSXElements, isInJSXFragment, getEnclosingFunctionNode); refactored functions to use safe fallbacks instead of throwing on unknowns; added ESLint scope utilities.
Filename Utilities `machinery/filename.js`	Refactored all functions to accept generic data object instead of ESLint context; added getFilename normalizer, isUniversal, and isRaw predicates; changed parameter from context-based to data-driven approach.
Rule Definitions `rules/component-properties/index.js`, `rules/jsx-key/index.js`, `rules/layout-class-name/index.js`, `rules/no-relative-parent-import/index.js`	Wrapped rule exports with defineRule from oxlint; renamed create to createOnce handler; added null-check guards in component-properties; added ExportAllDeclaration handler in no-relative-parent-import.
Complex Rule Updates `rules/naming-policy/index.js`, `rules/no-default-export/index.js`	Updated naming-policy to use filename-based context instead of ESLint context, added getValidRootElementClassNames helper; refactored no-default-export to use defineCompatibleRule with filename-aware checks for allowed export types (isApp, isTemplate, isUniversal, isRaw).

Sequence Diagram(s)

sequenceDiagram
    participant CLI as Linter CLI
    participant Compat as defineCompatibleRule
    participant Detect as Environment Detection
    participant Oxlint as Oxlint Runtime
    participant ESLint as ESLint Runtime
    
    CLI->>Compat: Import rule definition
    activate Compat
    Compat->>Detect: Load environment (module init)
    activate Detect
    alt Oxlint detected
        Detect->>Detect: require('oxlint') succeeds
        Detect-->>Compat: Oxlint mode
        Compat->>Oxlint: Transform rule (meta + messages + createOnce)
        Oxlint-->>Compat: defineRule result
    else ESLint active
        Detect->>Detect: require('oxlint') fails
        Detect-->>Compat: ESLint mode
        Compat->>ESLint: Return rule unchanged
        ESLint-->>Compat: Identity result
    end
    deactivate Detect
    Compat-->>CLI: Compatible rule
    deactivate Compat

sequenceDiagram
    participant Rule as Rule Code
    participant Filename as filename.js
    participant AST as ast.js
    
    Rule->>Filename: getFilename(data)
    activate Filename
    Filename->>Filename: Check if data has getFilename()
    alt Object with method
        Filename->>Filename: Call data.getFilename()
    else Plain string
        Filename->>Filename: Return data as-is
    end
    Filename-->>Rule: filename string
    deactivate Filename
    
    Rule->>Filename: getBaseFilename(filename)
    activate Filename
    Filename->>Filename: Extract base name (remove .universal, .raw, etc.)
    Filename-->>Rule: base filename
    deactivate Filename
    
    Rule->>AST: getEnclosingFunctionNode(node, context)
    activate AST
    AST->>AST: Traverse parents until FunctionExpression/FunctionDeclaration/ArrowFunction
    AST-->>Rule: function node
    deactivate AST

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

The changes introduce a substantial cross-linter compatibility layer with heterogeneous modifications: new utility functions with expanded scope (ast.js, define-compatible-rule.js), refactored function signatures shifting from context-based to data-driven patterns (filename.js), and systematic rule wrapping with defineRule across multiple files. While many rule updates follow a consistent pattern, the logic density in naming-policy and no-default-export requires separate reasoning. The machinery layer changes demand careful analysis of environment detection and rule transformation logic.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.68% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Experimental oxlint migration" accurately and specifically describes the primary change in this pull request. The changeset comprehensively introduces oxlint support, including a new oxlint configuration file, cross-linter compatibility utilities, updates to multiple rule files to use oxlint's API, the oxlint dependency, and a test runner for oxlint. The title clearly conveys the main objective to a developer scanning the repository history, and while it doesn't enumerate every detail, it appropriately summarizes the overarching purpose of the changeset.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch experimental-oxlint-migration

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

AlbertSmit · 2025-10-22T15:32:02Z

@coderabbitai review

coderabbitai · 2025-10-22T15:32:11Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

rules/component-properties/index.js (1)

20-20: Inconsistency: create vs createOnce across rules.

This rule uses create(context) while rules/layout-class-name/index.js uses createOnce(context). Ensure this difference is intentional and both are valid for the oxlint/ESLint compatibility layer being built.

rules/no-relative-parent-import/index.js (1)

1-31: This rule hard‑depends on Oxlint; ESLint will crash. Switch to the compatibility wrapper.

Importing oxlint directly breaks ESLint environments. Use the shared defineCompatibleRule and create (ESLint) with messageId/data so Oxlint mapping works.

Apply this diff:

-const { defineRule } = require("oxlint")
+const { defineCompatibleRule } = require("../../machinery/define-compatible-rule")

-const messages = {
-  'no relative parent import': found =>
-    `Unexpected relative parent import '${found}' - use a root slash import`,
-}
-
-module.exports = defineRule({
-  messages,
+module.exports = defineCompatibleRule({
   messages,
 
-  meta: { type: 'problem' },
+  meta: {
+    type: 'problem',
+    messages: {
+      noRelativeParentImport: "Unexpected relative parent import '{{found}}' - use a root slash import"
+    },
+    schema: []
+  },
 
-  createOnce(context) {
+  create(context) {
     return {
       'ImportDeclaration': reportRelativeImportInSource,
       'ExportNamedDeclaration': reportRelativeImportInSource,
       'ExportAllDeclaration': reportRelativeImportInSource,
     }
 
     function reportRelativeImportInSource({ source }) {
       if (!source) return
       const { value } = source
-      if (!value.includes('..')) return
+      // Only match dot-dot segments as full path components
+      if (!/(^|\/)\.\.(\/|$)/.test(value)) return
 
-      context.report({
-        message: messages['no relative parent import'](value),
-        node: source
-      })
+      context.report({
+        node: source,
+        messageId: 'noRelativeParentImport',
+        data: { found: value },
+      })
     }
   }
-})
+})

rules/naming-policy/index.js (2)

75-87: Align getBaseFilename usage with the new helpers.

Mixed signatures: here getBaseFilename(context) vs elsewhere getBaseFilename(filename). Use getFilename(context) first, then pass filename.

-      const name = getBaseFilename(context)
+      const filename = getFilename(context)
+      const name = getBaseFilename(filename)
       const expected = `./${name}.css`

Also applies to: 93-95

26-28: Fix typos in user-facing messages.

Minor but user-visible.

-  'invalid styles variable name': (found, expected) =>
-    `Unexptected css import name '${found}', expected '${expected}'`,
+  'invalid styles variable name': (found, expected) =>
+    `Unexpected css import name '${found}', expected '${expected}'`,
@@
-  'ref should end with Ref': (found, exptected) =>
-    `Unexpected ref name '${found}', expected '${exptected}'`,
+  'ref should end with Ref': (found, expected) =>
+    `Unexpected ref name '${found}', expected '${expected}'`,

Also applies to: 33-35

♻️ Duplicate comments (2)

rules/component-properties/index.js (1)

1-1: Critical: oxlint package doesn't export defineRule.

Same issue as in rules/layout-class-name/index.js - oxlint is a CLI tool and doesn't provide a Node.js defineRule export. This import will fail at runtime.

rules/jsx-key/index.js (1)

3-3: Critical: oxlint package doesn't export defineRule.

Same critical issue - this import from 'oxlint' will fail. All custom rules need to be updated to use the correct compatibility layer or mechanism for loading custom rules into oxlint.

🧹 Nitpick comments (2)

package.json (1)

24-24: Move oxlint to devDependencies.

oxlint is a development tool (linter) and should be in devDependencies rather than dependencies to avoid including it in production installations. Version 1.23.0 is current and valid on npm.
rules/naming-policy/index.js (1)
70-72: Verify selector for hooks returning refs.

Current selector may miss the common CallExpression form (e.g., const x = useDomRef()). Consider matching the callee Identifier under CallExpression.

Possible alternative:
// Match: const fooRef = useSomethingRef()
"VariableDeclarator > .init CallExpression > .callee Identifier[name=/^use.*Ref$/]"
Please confirm with tests that your selector hits intended cases and doesn’t fire on bare assignments like const x = useDomRef. Want a quick script to scan the repo for call sites?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 214dcc3 and d188fa5.

⛔ Files ignored due to path filters (1)

yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (14)

.oxlintrc.json (1 hunks)
README.md (1 hunks)
index.js (1 hunks)
machinery/ast.js (2 hunks)
machinery/define-compatible-rule.js (1 hunks)
machinery/filename.js (1 hunks)
machinery/oxlint-test-runner.js (1 hunks)
package.json (2 hunks)
rules/component-properties/index.js (3 hunks)
rules/jsx-key/index.js (3 hunks)
rules/layout-class-name/index.js (3 hunks)
rules/naming-policy/index.js (6 hunks)
rules/no-default-export/index.js (1 hunks)
rules/no-relative-parent-import/index.js (2 hunks)

🧰 Additional context used

🪛 Biome (2.1.2)

machinery/ast.js

[error] 22-22: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

.oxlintrc.json

[error] 1-25: JSON standard does not allow comments.

(parse)

[error] 28-30: Expected a property but instead found '// Environments → same semantics as ESLint'.

Expected a property here.

(parse)

[error] 25-28: End of file expected