Skip to content

feat: port bad-char-at-comparison, bad-object-literal-comparison, number-arg-out-of-range#1533

Open
bartlomieju wants to merge 2 commits into
mainfrom
oxlint-port-oxc-correctness-batch2
Open

feat: port bad-char-at-comparison, bad-object-literal-comparison, number-arg-out-of-range#1533
bartlomieju wants to merge 2 commits into
mainfrom
oxlint-port-oxc-correctness-batch2

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Second batch of oxc-correctness oxlint ports (the comparison family), following
erasing-op in #1521. All three are purely syntactic, with
tests ported from the upstream oxc fixtures.

  • bad-char-at-comparison — flags comparing the result of
    String.prototype.charAt() against a string whose length is not 1. charAt
    returns a single UTF-16 code unit, so the comparison can never be true.
  • bad-object-literal-comparison — flags x === {}, x !== [] and similar.
    An empty object/array literal is a fresh reference that is never equal to
    anything, so the comparison is always false (or always true for inequality).
    Only empty literals are flagged; x === { a: 1 } is left alone.
  • number-arg-out-of-range — flags out-of-range radix/precision arguments to
    toString (2–36), toFixed/toExponential (0–20), and toPrecision
    (1–21). Out-of-range values throw a RangeError at runtime. Handles both
    x.toString(64) and x["toExponential"](22).

Docs for these (and the rest of the bucket) are in denoland/docs#3378.

All three are tagged recommended, matching oxc's correctness classification.
As with #1521, this is the same on-by-default question — happy to drop the tag
and land them opt-in first if you prefer.

…er-arg-out-of-range

Second batch of oxc-correctness oxlint ports, all purely syntactic:

- bad-char-at-comparison: flags comparing the result of `String.prototype
  .charAt()` against a string whose length is not 1 (can never match).
- bad-object-literal-comparison: flags `x === {}` / `x !== []` and friends;
  an empty object/array literal is a fresh reference, so the comparison is
  always false (or always true for inequality).
- number-arg-out-of-range: flags out-of-range radix/precision arguments to
  `toString`/`toFixed`/`toExponential`/`toPrecision`, which throw at runtime.

Tests are ported from each oxc rule's fixtures. All three are tagged
recommended (oxc category: correctness); same on-by-default decision as the
rest of the batch. Docs added in denoland/docs#3378.
@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: needs changesnumber-arg-out-of-range uses outdated numeric limits and will false-positive on valid Deno code. Not merging (bug + CONFLICTING).

Correctness — blocking (number-arg-out-of-range)
The ranges for toFixed/toExponential/toPrecision are the pre-ES2018 limits. ECMAScript raised them (and V8/Deno implements the new ones):

  • toFixed(digits): 0–100 (rule uses 0–20)
  • toExponential(digits): 0–100 (rule uses 0–20)
  • toPrecision(precision): 1–100 (rule uses 1–21)
  • toString(radix): 2–36 ✓ (unchanged, correct)

So the rule flags valid Deno code. Your own "invalid" fixtures are actually valid in Deno:

  • x.toFixed(22) → runs fine ("1.00…"), not a RangeError
  • x.toExponential(22) → valid
  • x.toPrecision(100) → valid

Since this is tagged RECOMMENDED (on by default) and deno_lint targets Deno (V8), these are real false positives on correct code. Fix: use toFixed/toExponential(0, 100), toPrecision(1, 100), update the message/hint text, and move toFixed(22)/toExponential(22)/toPrecision(100) from invalid → valid (add toFixed(101) etc. as the new invalid boundary). (oxc apparently still uses the old limits upstream, but for Deno the V8 range is what matters.)

bad-char-at-comparison — minor (false negative)
is_bad_comparison_string uses value.chars().count() != 1 (Unicode scalar count), but charAt returns a single UTF-16 code unit. An astral char like "😀" is one code point but two UTF-16 units, so str.charAt(0) === "😀" is always false yet isn't flagged. Under-flagging (safe direction), but to match JS semantics use value.encode_utf16().count() != 1. The empty-string exemption (str.charAt(n) === "") is correctly preserved.

bad-object-literal-comparison — logic (flag only empty {}/[] literals in ===/!==) looks correct; no issues spotted.

Housekeeping: mergeable: CONFLICTING → rebase; RECOMMENDED sign-off (you flagged it); docs are in denoland/docs#3378 (noted).

Blocker: correct the toFixed/toExponential/toPrecision ranges to the V8/ES2018 limits (0–100 / 1–100) + fix the fixtures, then rebase. Consider the UTF-16 tweak for bad-char-at-comparison.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant