Correctly create outside variable when shadowed by catch parameter

[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 #4176

Description

Apparently, a var declared in a catch clause will always create a hoisted variable, even if the variable name matches the clause parameter. If that is the case, then the initializer goes to the parameter instead of the variable...
This handles this correctly by always creating a hoisted variable while deoptimizing the catch parameter if there is a conflict.

[github-actions]

Thank you for your contribution! ❤️

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

npm install rollup/rollup#gh-4176-wrong-catch-scope

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

[codecov]

Codecov Report

Merging #4178 (3098d55) into master (89b9370) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4178   +/-   ##
=======================================
  Coverage   98.33%   98.33%           
=======================================
  Files         202      202           
  Lines        7220     7221    +1     
  Branches     2111     2111           
=======================================
+ Hits         7100     7101    +1     
  Misses         58       58           
  Partials       62       62           

Impacted Files	Coverage Δ
src/ast/nodes/CatchClause.ts	83.33% <ø> (ø)
src/ast/scopes/CatchScope.ts	100.00% <100.00%> (ø)

---

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 89b9370...3098d55. Read the comment docs.

[kzc]

This parse issue may be beyond rollup's control...

Given:

$ cat catch-shadows-function.js 
"use strict";
function c() {
    console.log("PASS");
}
try {} catch (c) {
    var c;
}
c();

Expected output on all NodeJS versions I tried (agrees with Chrome and Firefox):

$ cat catch-shadows-function.js | node
PASS

Actual:

$ cat catch-shadows-function.js | rollup --silent
[!] Error: Identifier 'c' has already been declared (Note that you need plugins to import files that are not JavaScript)
- (6:8)

The error comes from acorn:

$ cat catch-shadows-function.js | node_modules/.bin/acorn --module --ecma2020
Identifier 'c' has already been declared (6:8)

$ grep version node_modules/acorn/package.json 
  "version": "8.4.0",

[kzc]

Never mind. Evidently if the file suffix is changed from js to mjs or node --input-type=module is used then recent versions of NodeJS will produce the same error as acorn and rollup. And sure enough, if <script type="module"> is used in a browser it will also produce the same error.

I take it that Rollup implicitly assumes all input is a module?

[lukastaegert]

Exactly, there is an option passed to acorn with that assumption