Allow functions to redeclare vars and functions in function scopes

[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:

• resolves [!] (plugin commonjs--resolver) RollupError: Identifier "fn" has already been declared #5247

Description

Apparently, the semantics for duplicate function names differ between function scopes and other scopes: In function scopes, function can redeclare function and var while in other scopes it is forbidden

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
rollup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 13, 2023 8:10am

[github-actions]

Thank you for your contribution! ❤️

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

npm install rollup/rollup#gh-5247-function-redeclaration

Notice: Ensure you have installed Rust nightly. If you haven't installed it yet, please first see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust, then see https://rust-lang.github.io/rustup/concepts/channels.html to learn how to install Rust nightly.

or load it into the REPL:
https://rollup-4877i9lit-rollup-js.vercel.app/repl/?pr=5248

[codecov]

Codecov Report

Merging #5248 (ad51fad) into master (53d6360) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5248   +/-   ##
=======================================
  Coverage   98.89%   98.89%           
=======================================
  Files         231      232    +1     
  Lines        8926     8936   +10     
  Branches     2332     2333    +1     
=======================================
+ Hits         8827     8837   +10     
  Misses         40       40           
  Partials       59       59           

Files	Coverage Δ
src/ast/scopes/CatchBodyScope.ts	100.00% <ø> (ø)
src/ast/scopes/FunctionBodyScope.ts	100.00% <100.00%> (ø)
src/ast/scopes/ParameterScope.ts	100.00% <ø> (ø)
src/ast/scopes/Scope.ts	100.00% <100.00%> (ø)

[sapphi-red]

It seems cases like these are not allowed. This PR allows them.

{
  var fn
  function fn () {}
}

acorn playground
rollup repl for this PR

[lukastaegert]

Yes, please try again. So apparently the logic is still different from what I assumed: Redeclaring functions and stuff with functions is forbidden in nearly all scopes except function scopes, where apparently anything goes... Who made this up!??

[sapphi-red]

Great! The behavior looks good to me. Yeah, it's really complicated. 🫠

[github-actions]

This PR has been released as part of rollup@4.4.1. You can test it via npm install rollup.