Skip to content

test(minifier): extend legal-comment coverage for #19750 and #19748#21951

Merged
Dunqing merged 3 commits intooxc-project:fix/minifier-preserve-legal-comments-on-inlinefrom
gthb:tests/legal-comments-additional-coverage
Apr 30, 2026
Merged

test(minifier): extend legal-comment coverage for #19750 and #19748#21951
Dunqing merged 3 commits intooxc-project:fix/minifier-preserve-legal-comments-on-inlinefrom
gthb:tests/legal-comments-additional-coverage

Conversation

@gthb
Copy link
Copy Markdown
Contributor

@gthb gthb commented Apr 29, 2026

What

Three tests on top of #21575:

In dead_code_elimination.rs:

  • /* @license */ standalone — the only legal-comment marker the omnibus preserve_legal_comment_when_dce_removes_anchor doesn't pin.

In normalize.rs, covering the legal-comment subset of #19748 that #21575 also closes:

  • Banner above a removed program-level "use strict".
  • Inner-function "use strict" removal preserving its inner legal comment.

The placement quirk for the legal-comment subset of #19748 is pinned separately in #21943.

gthb added 2 commits April 29, 2026 18:00
…ect#19750

Pins three DCE-orphan scenarios beyond the omnibus
`preserve_legal_comment_when_dce_removes_anchor` test:

* `/* @license */` standalone (no `!`) — the only legal-comment marker
  the omnibus test doesn't already pin.
* Multi-declarator pruning: `b` removed, then `a` inlined, leaving no
  declarator. Pins that the legal comment orphans the same as the
  single-declarator case.
* Nested orphan in a surviving function whose own outer anchor is also
  DCE'd — pins that the inner orphan flushes at the inner block's next
  surviving statement, not at the outer scope.
…moval

The codegen-side fix in 6d697da (`print_legal_orphans_before` flushing
at every statement boundary) also resolves the legal-comment subset of
issue oxc-project#19748: a legal comment anchored to a removed program-level or
inner-function `"use strict"` directive is rescued at the next surviving
statement instead of being silently dropped.

Pin two cases — top-of-file banner above a removed program-level
directive (both `//!` and `/*!` forms), and an inner-function legal
comment when the redundant inner `"use strict"` is dropped under a
strict outer scope.

Documents (in a comment) that the non-legal subset of oxc-project#19748 is *not*
covered: `print_legal_orphans_before` is gated on `Comment::is_legal()`
by design, and closing the regular-comment case would need a separate
`Normalize`-side comment remap.
@gthb gthb force-pushed the tests/legal-comments-additional-coverage branch from 32d8855 to 2ca3a6b Compare April 29, 2026 18:00
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 29, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing gthb:tests/legal-comments-additional-coverage (33d1e66) with fix/minifier-preserve-legal-comments-on-inline (722ff71)

Open in CodSpeed

Footnotes

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

After the refactor of oxc-project#21575's omnibus
`preserve_legal_comment_when_dce_removes_anchor`, two of the additional
tests in this branch became mechanism-redundant:

* `preserve_legal_comment_when_dce_removes_one_of_multiple_declarators`
  exercised the multi-declarator AST-mutation path, but the codegen-side
  orphan flush is identical to the single-declarator case the parent
  already pins.
* `preserve_legal_comments_when_dce_removes_anchors_at_multiple_levels`
  asserted the inner orphan stays scoped when both inner and outer
  anchors are removed. The parent already pins the per-scope flush
  keeping the inner orphan inside the function with one orphan; adding
  an outer orphan exercises the same mechanism.

The remaining three tests (`/* @license */` standalone, and the two
`"use strict"`-removal tests for the legal-comment subset of oxc-project#19748)
each cover behaviour the parent omnibus doesn't reach.
Copy link
Copy Markdown
Member

@Dunqing Dunqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Dunqing Dunqing merged commit 5f0dcb8 into oxc-project:fix/minifier-preserve-legal-comments-on-inline Apr 30, 2026
35 checks passed
Dunqing pushed a commit that referenced this pull request Apr 30, 2026
…21951)

What
---

Three tests on top of #21575:

In `dead_code_elimination.rs`:

- `/* @license */` standalone — the only legal-comment marker the
omnibus `preserve_legal_comment_when_dce_removes_anchor` doesn't pin.

In `normalize.rs`, covering the legal-comment subset of #19748 that
#21575 also closes:

- Banner above a removed program-level `"use strict"`.
- Inner-function `"use strict"` removal preserving its inner legal
comment.

The placement quirk for the legal-comment subset of #19748 is pinned
separately in #21943.
@gthb gthb deleted the tests/legal-comments-additional-coverage branch April 30, 2026 07:42
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.

2 participants