Skip to content

feat(no-undef): support eslint-style /* global */ comments - #1498

Open
bartlomieju wants to merge 1 commit into
mainfrom
no-undef-global-comments
Open

feat(no-undef): support eslint-style /* global */ comments#1498
bartlomieju wants to merge 1 commit into
mainfrom
no-undef-global-comments

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

The no-undef rule only recognized identifiers from its built-in globals
list, so legitimate ambient globals (declared elsewhere, injected by a host,
or specific to a non-Deno environment) were always reported as undefined with
no way to allow them. ESLint solves this with /* global foo */ directive
comments, but deno_lint had no equivalent, leaving the comment-based escape
hatch unavailable.

This teaches no-undef to read eslint-compatible /* global foo */ and
/* globals foo, bar */ block comments and treat the listed names as defined.
Writability hints (foo:writable, bar:readonly, and the legacy baz:true)
are parsed and ignored, since no-undef only cares whether a name is defined.
Matching is deliberately strict: only block comments count, and the keyword
must be exactly global or globals followed by whitespace, so identifiers
like globalThis and line comments are never mistaken for a directive.

Closes #1287

@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: LGTM — correct and thoroughly tested. Recommend merge (rule-behavior change, so flagging rather than auto-merging; just needs green CI / a rebase).

Correctness — traced the parser against every test

  • trim_start().strip_prefix("global") → optional strip_prefix('s') → require the next char is whitespace: this correctly accepts global/globals and rejects globalThing/globalThis (the whitespace guard is the key), matching the negative test.
  • Comma-split with entry.split(':').next() correctly parses names and discards eslint writability hints (:writable/:readonly/legacy :true), including the spaced a: true form.
  • Multiline directives work (/* global\n a,\n b\n*/) because the split operates on the raw text and each entry is trimmed.
  • Line comments are ignored (CommentKind::Block guard), and undeclared names still error — both covered by tests.
  • declared_globals.contains(&*ident.sym) looks up &str against HashSet<String> via Borrow, so no per-identifier allocation. The pre-pass collect_declared_globals borrows comments before the mutable context borrow — ordering is correct.

Test coverage is excellent: positive cases (all forms + hints + multiline), and negatives for undeclared names, line comments, and the globalThing false-directive.

Minor (non-blocking)

  • Space-separated names like /* global a b */ become a single "a b" entry, which simply never matches any identifier (harmless). ESLint primarily splits on commas too, so this is close enough; if you want full parity you could also split entries on whitespace.

Perf: good — one pass over comments; O(1) set lookup per identifier. (The existing linear GLOBALS.iter().any() scan is unchanged and out of scope here.)

Housekeeping: mergeable: UNKNOWN → rebase; then it's ready. As a no-undef behavior change it's a rule-behavior tweak, but a well-scoped, opt-in-by-comment one.

No changes requested. Good to merge once CI is green.

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.

expect no-undef support comment declare like eslint

1 participant