Skip to content

refactor(linter): support loose options for eslint/eqeqeq#8790

Merged
Boshen merged 1 commit intooxc-project:mainfrom
shulaoda:refactor/eslint-eqeqeq
Jan 31, 2025
Merged

refactor(linter): support loose options for eslint/eqeqeq#8790
Boshen merged 1 commit intooxc-project:mainfrom
shulaoda:refactor/eslint-eqeqeq

Conversation

@shulaoda
Copy link
Contributor

closes #8773

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 30, 2025

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.

@github-actions github-actions bot added A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Jan 30, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 30, 2025

CodSpeed Performance Report

Merging #8790 will not alter performance

Comparing shulaoda:refactor/eslint-eqeqeq (21d7e6d) with main (6c627df)

Summary

✅ 33 untouched benchmarks

@Sysix
Copy link
Member

Sysix commented Jan 30, 2025

I do not like that we support more then eslint. But I do not see why we should not do this => 👍

@Boshen Boshen merged commit 0aeaedd into oxc-project:main Jan 31, 2025
26 checks passed
@shulaoda shulaoda deleted the refactor/eslint-eqeqeq branch January 31, 2025 02:19
Boshen pushed a commit that referenced this pull request Jan 31, 2025
…8798)

Related to #8790 

For the configuration `"eqeqeq": ["warn", "alw", { "null": "ignore" }]`,
we should default `"alw"` to `"always"` and correctly handle the option
`{ "null": "ignore" }`.
@oxc-bot oxc-bot mentioned this pull request Feb 1, 2025
Boshen added a commit that referenced this pull request Feb 1, 2025
## [0.15.9] - 2025-02-01

### Features

- 1a41181 linter: Implement `eslint/prefer-object-spread` (#8216)
(tbashiyy)
- adb8ebd linter: Implement no-useless-call rule (#8789) (keita hino)
- 3790933 linter: Add vitest/prefer-lowercase-title rule (#8152) (Tyler
Earls)
- e8e6917 linter: Unicorn/switch-cases-braces support options (#8704)
(1zumii)

### Bug Fixes

- 8ce21d1 linter: Can't disable `no-nested-ternary` rule anymore (#8600)
(dalaoshu)
- e929f26 linter: Output `LintCommandInfo` for
`CliRunResult::LintNoFilesFound` (#8714) (Sysix)
- 4f30a17 linter: Unicorn/switch-case-braces mangles code when applying
fix (#8758) (Tyler Earls)
- 9cc9d5f linter: `ignorePatterns` does not work when files are provided
as command arguments (#8590) (dalaoshu)
- 1de6f85 linter: No-lone-blocks erroring on block statements containing
comments (#8720) (Tyler Earls)
- 77ef61a linter: Fix diagnostic spans for `oxc/no-async-await` (#8721)
(camchenry)
- f15bdce linter: Catch `Promise` in `typescript/array-type` rule
(#8702) (Rintaro Itokawa)
- 5041cb3 vscode: Fix commands by reverting commit `259a47b` (#8819)
(Alexander S.)

### Performance

- d318238 linter: Remove sorting of rules in cache (#8718) (camchenry)

### Documentation

- 57b7ca8 ast: Add documentation for all remaining JS AST methods
(#8820) (Cam McHenry)

### Refactor

- c2fdfc4 linter: Correctly handle loose options for `eslint/eqeqeq`
(#8798) (dalaoshu)
- 0aeaedd linter: Support loose options for `eslint/eqeqeq` (#8790)
(dalaoshu)
- 194a5ff linter: Remove `LintResult` (#8712) (Sysix)
- 4a2f2a9 linter: Move default `all_rules` output to trait (#8710)
(Sysix)
- 741fb40 linter: Move stdout outside LintRunner (#8694) (Sysix)
- 10e5920 linter: Move finishing default diagnostic message to
`GraphicalReporter` (#8683) (Sysix)
- 9731c56 oxlint: Move output from `CliRunResult::InvalidOption` to
outside and use more Enums for different invalid options (#8778) (Sysix)
- fe45bee oxlint: Create different `CliRunResult` instead of passing
`ExitCode` to it (#8777) (Sysix)
- 2378fef oxlint: Move ConfigFileInit output outside CliRunResult, exit
code 1 when it fails (#8776) (Sysix)
- f4cecb5 oxlint: Remove unused `CliRunResult::PathNotFound` (#8775)
(Sysix)

### Testing

- ad35e82 linter: Use snapshot testing instead of LintResult (#8711)
(Sysix)
- bf895eb linter: Add diagnostic format test snapshots (#8696)
(Alexander S.)
- 34d3d72 linter: Add snapshot tester for cli (#8695) (Sysix)
- 0bf2bcf oxlint: Test two real rules with same name but from different
plugins (#8821) (dalaoshu)
- 2b83b71 oxlint: Improve disabling "no-nested-ternary" tests (#8814)
(Alexander S.)
- 45648e7 oxlint: Fix InvalidOptionTsConfig tests for windows (#8791)
(Alexander S.)
- 48bfed9 oxlint: Ignore windows path mismatch (Boshen)
- 6f4a023 oxlint: Remove "--print-config" test (#8792) (Sysix)
- 55c2025 oxlint: Add `CliRunResult` to snapshot (#8780) (Sysix)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
graphite-app bot pushed a commit that referenced this pull request Jan 22, 2026
…uple config options. (#18372)

This was built out of the ashes of #16555. Implements necessary work for #16023.

This introduces a new TupleRuleConfig. Rather than forcing DefaultRuleConfig to bend to our will as attempted previously, this PR adds a distinct rule config setup that is built to handle tuple-based rule configs.

This PR updates 3 rules to use TupleRuleConfig:

- `eslint/eqeqeq`
- `eslint/sort_keys`
- `eslint/yoda`

It also adds snapshot validation tests to ensure that they error as-expected when given an invalid config option.

The only real change here is in `eslint/eqeqeq`. We previously allowed passing _only_ an object to the rule config (due to #8790), but that is not valid in the original rule, and also makes implementing proper config option validation quite a bit more difficult. I had removed support for this in #16560, but that was part of the previous, doomed attempt at using DefaultRuleConfig with tuples. So it never got merged, and I'm redoing it here.

This is a necessary part of the change for this PR, as the alternative is to make the config options parsing/validation implementation _considerably_ more complex. This also brings us back in line with ESLint's behavior for this rule.

---

AI Disclosure: I used the code and tests from #16555 as the basis for the changes in `rule.rs`, then let Claude Opus 4.5 run with it and guided it toward the TupleRuleConfig concept. After I got a satisfactory TupleRuleConfig implementation, I then manually implemented the rest of the changes in this PR by copying the files over from #16555, applying the TupleRuleConfig rename, and then carefully going through the diffs to make sure nothing was deleted that shouldn't have been.

I have added tests to ensure that the config validation for tuple rules works, and have entirely avoided changing the unit tests in any of the 3 modified rules (other than the one change for eqeqeq I noted above).

---

<details>
<summary>Updated docs for the changed rules</summary>

Note that the sort-keys docs are entirely unchanged as they were correct prior to this, so I've excluded them.

yoda:
```md
## Configuration

### The 1st option

type: `"never" | "always"`

#### `"never"`

The default `"never"` option can have exception options in an object literal, via `exceptRange` and `onlyEquality`.

#### `"always"`

The `"always"` option requires that literal values must always come first in comparisons.

### The 2nd option

This option is an object with the following properties:

#### exceptRange

type: `boolean`

default: `false`

If the `"exceptRange"` property is `true`, the rule _allows_ yoda conditions
in range comparisons which are wrapped directly in parentheses, including the
parentheses of an `if` or `while` condition.
A _range_ comparison tests whether a variable is inside or outside the range
between two literal values.

#### onlyEquality

type: `boolean`

default: `false`

If the `"onlyEquality"` property is `true`, the rule reports yoda
conditions _only_ for the equality operators `==` and `===`. The `onlyEquality`
option allows a superset of the exceptions which `exceptRange` allows, thus
both options are not useful together.
```

eqeqeq:

```md
## Configuration

### The 1st option

type: `"always" | "smart"`

#### `"always"`

Always require triple-equal comparisons, `===`/`!==`.
This is the default.

#### `"smart"`

Allow certain safe comparisons to use `==`/`!=` (`typeof`, literals, nullish).

### The 2nd option

This option is an object with the following properties:

#### null

type: `"always" | "never" | "ignore"`

##### `"always"`

Always require triple-equals when comparing with null, `=== null`/`!== null`.
This is the default.

##### `"never"`

Never require triple-equals when comparing with null, always use `== null`/`!= null`.

##### `"ignore"`

Ignore null comparisons, allow either `== null`/`!= null` or `=== null`/`!== null`.
```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter 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.

linter: eqeqeq null ignore is not working

3 participants