Skip to content

fix(linter): Fix default behavior of the eslint/eqeqeq rule, and also the config option docs for it#16560

Merged
camchenry merged 6 commits intohandle-tuple-rules-betterfrom
more-tuple-docs
Dec 10, 2025
Merged

fix(linter): Fix default behavior of the eslint/eqeqeq rule, and also the config option docs for it#16560
camchenry merged 6 commits intohandle-tuple-rules-betterfrom
more-tuple-docs

Conversation

@connorshea
Copy link
Member

@connorshea connorshea commented Dec 6, 2025

This is part of #16023.

See the tests for the original rule.

We have a test in the eqeqeq.rs implementation like so:

// Issue: <https://github.com/oxc-project/oxc/issues/8773>
("href != null", Some(json!([{"null": "ignore"}]))),

The problem is that this test has an incorrect shape for the config object, see here:

// Should always be in one of these three formats, all three work in the original rule as well:
"eslint/eqeqeq": ["error", "always", { "null": "never" }],
"eslint/eqeqeq": ["error", "always"],
"eslint/eqeqeq": ["error"],

// But right now the tests have a case where the string arg is skipped, while the ESLint rule does not allow this:
"eslint/eqeqeq": ["error", { "null": "ignore" }],

The problem is that the code did previously handle this config array as invalid. However, because the implementation of from on NullType would fall back to ignore if it received bad data, it looked like it worked:

impl NullType {
    pub fn from(raw: &str) -> Self {
        match raw {
            "always" => Self::Always,
            "never" => Self::Never,
            _ => Self::Ignore,
        }
    }
}

Because always is marked as the default value (and is also the default value in the original ESLint rule), and so should be the default case. The test was just hitting the fallback value, so it looked like it worked, but really the fallback value was incorrect previously and did not match the docs or the ESLint behavior.

This fixes that issue by correcting the fallback value, and also fixes the auto-generated config shape/docs, so it correctly represents itself as taking a tuple.

Generated docs:

## 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` and `=== null`/`!== null`

@connorshea connorshea requested a review from camc314 as a code owner December 6, 2025 19:40
@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Dec 6, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 6, 2025

CodSpeed Performance Report

Merging #16560 will not alter performance

Comparing more-tuple-docs (2ba5292) with handle-tuple-rules-better (19b5d40)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

  1. 41 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.

@camc314 camc314 self-assigned this Dec 6, 2025
@connorshea connorshea force-pushed the handle-tuple-rules-better branch from ca59885 to 962bfe2 Compare December 6, 2025 22:52
The test only worked because the fallback logic was bad previously. This config object was always wrong, and never should have worked.

This is, however, technically a breaking change. If any users were reliant on the behavior of the rule defaulting to `ignore` if you gave it a bad value, this fix may break things?
@camchenry camchenry merged commit 48cda35 into handle-tuple-rules-better Dec 10, 2025
20 checks passed
@camchenry camchenry deleted the more-tuple-docs branch December 10, 2025 05:07
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-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants