Correctly associate pure annotations and remove invalid ones

[lukastaegert]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

Description

This PR will improve the handling of pure annotations in several respects:

• annotations in front of conditional, logical, binary or sequence expressions will be associated with the first element unless the expression is surrounded by parentheses REPL
• annotations in front of call or new expressions will be associated with the expression even when there are parentheses
• annotations that cannot be associated with a call or new expression are always removed from the code REPL

I verified that the implemented behaviour seems to be in line with how terser behaves. This would also resolve discussions from #4068

[lukastaegert]

cc @kzc

[github-actions]

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#correctly-associate-annotations

or load it into the REPL:
https://rollupjs.org/repl/?pr=4095

[codecov]

Codecov Report

Merging #4095 (12d5b03) into master (2febefa) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4095   +/-   ##
=======================================
  Coverage   98.01%   98.02%           
=======================================
  Files         201      201           
  Lines        6994     7029   +35     
  Branches     2054     2060    +6     
=======================================
+ Hits         6855     6890   +35     
  Misses         66       66           
  Partials       73       73           

Impacted Files	Coverage Δ
src/Module.ts	100.00% <ø> (ø)
src/ast/nodes/CallExpression.ts	100.00% <ø> (ø)
src/ast/nodes/IfStatement.ts	100.00% <ø> (ø)
src/ast/nodes/NewExpression.ts	100.00% <ø> (ø)
src/Graph.ts	100.00% <100.00%> (ø)
src/ast/keys.ts	100.00% <100.00%> (ø)
src/ast/nodes/shared/Node.ts	100.00% <100.00%> (ø)
src/utils/pureComments.ts	100.00% <100.00%> (ø)
src/utils/sourceMappingURL.ts	100.00% <100.00%> (ø)
src/utils/treeshakeNode.ts	100.00% <100.00%> (ø)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2febefa...12d5b03. Read the comment docs.

[kzc]

Good stuff.

This is difficult to get right. I trust if the results were vetted with terser then it ought to be fine. Should there be pure annotation disagreements between minifiers, use the defacto behavior of uglify-js which happens to support ES2020+ these days.

With the recent PR on sequences it ought to be easy to support pure annotated calls with side effects in their parameters:

$ echo 'var foo = /*@__PURE__*/ bar(0, baz(), /*@__PURE__*/ boo(), 3);' | rollup --silent
/*@__PURE__*/ bar(0, baz(), /*@__PURE__*/ boo(), 3);

$ echo 'var foo = /*@__PURE__*/ bar(0, baz(), /*@__PURE__*/ boo(), 3);' | uglify-js --toplevel -mc
baz();

[kzc]

Side note... the surprising complexity of correctly associating comments with AST nodes and preserving their intent across various javascript tools is why I strongly discourage extending pure annotations beyond calls and new expressions. Anything can be wrapped in pure IIFEs in the worst case anyway.