Skip to content

feat(no-invalid-fetch-options): add no-invalid-fetch-options rule - #1530

Open
bartlomieju wants to merge 2 commits into
mainfrom
oxlint-port-no-invalid-fetch-options
Open

feat(no-invalid-fetch-options): add no-invalid-fetch-options rule#1530
bartlomieju wants to merge 2 commits into
mainfrom
oxlint-port-no-invalid-fetch-options

Conversation

@bartlomieju

@bartlomieju bartlomieju commented Jun 29, 2026

Copy link
Copy Markdown
Member

Ports the oxlint unicorn/no-invalid-fetch-options rule to deno_lint as a
native Rust rule.

The rule flags a body passed to fetch(url, { ... }) or
new Request(url, { ... }) when the method is GET or HEAD
(case-insensitive), since a body is not allowed for those methods and results
in a TypeError at runtime. An absent method defaults to GET, so a body
with no method is also flagged. A body of null or undefined is treated as
absent and not reported. Method values are read from string and single-element
template literals; values that cannot be read statically (variable references,
member expressions, enums, type annotations, conditionals, call results) are
treated as unknown and not flagged.

Both fetch(...) calls and new Request(...) are covered, matching oxc.

Reference:
https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/unicorn/no_invalid_fetch_options.rs

This port intentionally only inspects the method value when it is written as
an inline string or single-element template literal. It does not resolve method
values that come from another statement, a TypeScript enum member, or a typed
function parameter. As a result, six of oxc's fail fixtures are deliberately
not reproduced here (they would require cross-statement symbol resolution, enum
member resolution, or parameter type-annotation analysis):

  • const method = "head"; new Request(url, {method, body: "foo=bar"})
  • const method = "head"; fetch(url, {method, body: "foo=bar"})
  • const method = `head`; fetch(url, {method, body: "foo=bar"})
  • function foo(method: "HEAD" | "GET") { return new Request(url, {method, body: ""}); }
  • enum Method { Get = "GET" } ... fetch("/", { method: Method.Get, body: "" })
  • enum Method { Foo = "GET" } ... fetch("/", { method: Method.Foo, body: "" })

Because the rule treats an unresolved method as unknown (and therefore safe),
every one of oxc's pass fixtures is reproduced verbatim and still passes —
including the variable-, enum-, parameter-, as const-, and
member-expression-typed method cases. A less-capable rule simply flags fewer
things, so no genuine valid case regresses.

Tagged RECOMMENDED (on by default) — flagging for maintainer sign-off.

@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: LGTM — correct and conservative. Recommend merge after the RECOMMENDED sign-off you flagged + rebase. Not auto-merging (new rule).

Flags a body in fetch(url, {...}) / new Request(url, {...}) when method is GET/HEAD (or absent → GET). The intentional under-flagging (unresolved method values treated as unknown/safe) is the right call — it only reduces coverage, never introduces false positives, and every oxc pass fixture is reproduced.

Correctness — verified

  • Order-independent & last-wins: both method and body_range are accumulated across all props before the final check, so property order doesn't matter, and duplicate keys correctly take the last value ({method:"GET",method:"POST"} → POST → not flagged; {body:"x",body:null} → null → not flagged).
  • Absent method defaults to GET, so fetch(url, {body}) is flagged — correct (GET + body is a runtime TypeError).
  • is_null_or_undefined treats body: null / body: undefined as absent. Spread props bail out entirely (can't know method/body) — conservative and correct.
  • read_method reads inline string and single-element (tail) template literals, uppercased; templates with expressions and everything else → UNKNOWN → safe. as const, enum, variable, and typed-param method cases fall through to unknown (documented under-flagging).

Minor / faithful edges

  • Shorthand {body} counts as present even if the variable is undefined at runtime (statically unknown) — matches the reference; acceptable.
  • read_method uses quasi.raw(), so an escaped method template (`\x47ET`) wouldn't resolve to GET — absurd edge, non-issue.
  • Name-based fetch/Request (shadowed locals would match) — consistent with oxc.

Perf: trivial (per call/new, linear over object props).

Housekeeping: RECOMMENDED sign-off (you flagged it); mergeable: UNKNOWN → rebase; docs .md not in diff.

No code changes requested — the conservative design and edge handling are sound. Just tagging + rebase.

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