Skip to content

feat(no-empty-file): add no-empty-file rule - #1520

Open
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-empty-file
Open

feat(no-empty-file): add no-empty-file rule#1520
bartlomieju wants to merge 1 commit into
mainfrom
oxlint-port-no-empty-file

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Ports oxlint's unicorn/no-empty-file rule to deno_lint as a native Rust
rule. It flags a file whose top-level body contains no real code — only
whitespace, comments, directive prologues (e.g. "use strict";), empty
statements (;), or empty blocks ({}).

Top-level string-literal statements are treated as directives and counted
as empty, matching oxc; a string literal nested inside a block is a real
expression statement and keeps the file non-empty. Files consisting solely
of a triple-slash reference directive are exempt. The reported span is
capped at 100 characters so comment-heavy empty files don't produce a huge
diagnostic.

Reference rule:
https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/unicorn/no_empty_file.rs
(test fixtures ported from there; eslint-disable-specific fixtures were
dropped as they don't translate to deno_lint's ignore mechanism).

Note for maintainer review: this rule is tagged RECOMMENDED (on by
default for every user), consistent with it being a correctness rule.
Because of that, an empty file now produces a diagnostic by default, which
required updating the existing empty_file and empty_file_with_ast
framework tests in src/lib.rs to expect the new no-empty-file
diagnostic. Please confirm the on-by-default tagging is desired.

@bartlomieju

Copy link
Copy Markdown
Member Author

Verdict: LGTM on the logic — needs maintainer sign-off (RECOMMENDED/on-by-default) + one perf reorder. Not auto-merging (new rule).

Faithful port of oxlint's unicorn/no-empty-file.

Correctness — verified

  • Top-level directive strings ("use strict";) count as empty; a string nested in a block is real code — the is_empty_top_level_stmt vs is_empty_nested_stmt split handles this correctly, matching oxc.
  • Stmt::Expr only treats Expr::Lit(Lit::Str) as a directive, so a top-level template literal `x`; is (correctly) real code, not a directive. Nice distinction.
  • ModuleDecl (imports/exports) → non-empty; vacuous .all() on an empty body → flagged. Correct.
  • The 100-char span cap uses char_indices().nth(100) → a byte offset at a char boundary, so start + byte_offset is UTF-8 safe. Good.

Performance — worth reordering

  • The triple-slash exemption context.all_comments().any(is_triple_slash_reference) runs first, for every file, scanning all comments and regex-matching each line comment — even for obviously non-empty files. Since body-emptiness is cheap (just iterate top-level items) and almost always false, check emptiness first and only do the comment scan when the file otherwise looks empty:
    let is_empty = match program { ... };
    if !is_empty { return; }
    if context.all_comments().any(is_triple_slash_reference) { return; }
    That moves the per-file comment scan off the hot path.

Minor

  • The triple-slash regex char class ["|'] includes a literal | as an alternative quote char (should be ["']). Harmless for real /// <reference> directives, just a smell.

Product decision (you flagged it)

  • Tagged RECOMMENDED, so empty files now warn by default for everyone (hence the src/lib.rs framework-test updates). Reasonable for a correctness rule, but it's the call to confirm — some users intentionally keep empty mod.ts/placeholder files.

Housekeeping: mergeable: UNKNOWN → rebase; docs .md? (not in the diff — new rules conventionally ship one).

Logic is solid and well-tested. Blockers: confirm the RECOMMENDED tagging + the perf reorder; then good to land.

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