wrong scope for var declared in catch block with same name as catch argument

[kzc]

Rollup Version

rollup v2.53.1

Operating System (or Browser)

n/a

Node Version (if applicable)

n/a

Link To Reproduction

https://rollupjs.org/repl/?version=2.53.1&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMnZhciUyMGUlMjAlM0QlMjAlNUMlMjJGQUlMMWElNUMlMjIlMkMlMjB4JTIwJTNEJTIwMSUzQiU1Q24lNUNuY29uc29sZS5sb2coZnVuY3Rpb24oKSUyMCU3QiU1Q24lMjAlMjAlMjAlMjB0cnklMjAlN0IlNUNuJTIwJTIwJTIwJTIwJTIwJTIwJTIwJTIwJTJGJTJGJTIwZW1wdHklNUNuJTIwJTIwJTIwJTIwJTdEJTIwY2F0Y2glMjAoZSklMjAlN0IlNUNuJTIwJTIwJTIwJTIwJTIwJTIwJTIwJTIwdmFyJTIwZSUyMCUzRCUyMCU1QyUyMkZBSUwxYiU1QyUyMiUzQiU1Q24lMjAlMjAlMjAlMjAlN0QlNUNuJTVDbiUyMCUyMCUyMCUyMGlmJTIwKGUpJTVDbiUyMCUyMCUyMCUyMCUyMCUyMCUyMCUyMGNvbnNvbGUubG9nKCU1QyUyMkZBSUwxYyU1QyUyMiklM0IlNUNuJTIwJTIwJTIwJTIwZWxzZSU1Q24lMjAlMjAlMjAlMjAlMjAlMjAlMjAlMjBjb25zb2xlLmxvZyghZSUyMCUyNiUyNiUyMCU1QyUyMlBBU1MxJTVDJTIyKSUzQiU1Q24lNUNuJTIwJTIwJTIwJTIwcmV0dXJuJTIweCUyMCUzRiUyMCU1QyUyMkZBSUwyYSU1QyUyMiUyMCUzQSUyMHglMjAlM0QlMjAlNUMlMjJQQVNTMiU1QyUyMiUzQiU1Q24lNUNuJTIwJTIwJTIwJTIwdHJ5JTIwJTdCJTVDbiUyMCUyMCUyMCUyMCUyMCUyMCUyMCUyMG5vdF9yZWFjaGVkKCklM0IlNUNuJTIwJTIwJTIwJTIwJTdEJTIwY2F0Y2glMjAoeCklMjAlN0IlNUNuJTIwJTIwJTIwJTIwJTIwJTIwJTIwJTIwdmFyJTIweCUyMCUzRCUyMCU1QyUyMkZBSUwyYiU1QyUyMiUzQiU1Q24lMjAlMjAlMjAlMjAlN0QlNUNuJTdEKCkpJTNCJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyZXMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==

Expected Behaviour

Derived from example using rollup on minified code.

$ cat var-mirroring-catch-arg.js
var e = "FAIL1a", x = 1;

console.log(function() {
    try {
        // empty
    } catch (e) {
        var e = "FAIL1b";
    }

    if (e)
        console.log("FAIL1c");
    else
        console.log(!e && "PASS1");

    return x ? "FAIL2a" : x = "PASS2";

    try {
        not_reached();
    } catch (x) {
        var x = "FAIL2b";
    }
}());

$ cat var-mirroring-catch-arg.js | node
PASS1
PASS2

Actual Behaviour

$ cat var-mirroring-catch-arg.js | rollup --silent | node
FAIL1c
FAIL2a

$ cat var-mirroring-catch-arg.js | rollup --silent
console.log(function() {

    console.log("FAIL1c");

    return "FAIL2a" ;
}());

[lukastaegert]

I see the bug but I must admit I am just too stupid to understand what is going on here. So if I take this example

try {
  throw new Error();
} catch (e) {
  var e = 'value';
  console.log(e); // "value" of course
}
console.log(e); // undefined !?

It seems that this is equivalent to

try {
  throw new Error();
} catch (e2) {
  var e;
  e2 = 'value';
  console.log(e2);
}
console.log(e);

Is this really what is going on, i.e. the assignment to e in its own declaration only changes the parameter but not e?

[kzc]

Ha! I thought I understood the issue until I saw your examples. My description in the issue title is clearly wrong.

The ES spec would be the authoritative source, but if I had to guess, it appears that the scope of the var alias of the catch argument within the catch block is still the catch block but it will contribute an uninitialized var declaration to function scope similar to your second example.

Let's add a finally block to the mix to see what happens:

var x = 1;
try {
    throw 2;
} catch (x) {
    console.log("catch before var decl:", x);
    var x = 3;
    console.log("catch after var decl:", x);
} finally {
    console.log("finally before var decl:", x);
    var x = 4;
    console.log("finally after var decl:", x);
}
console.log("after:", x);

catch before var decl: 2
catch after var decl: 3
finally before var decl: 1
finally after var decl: 4
after: 4

@alexlamsl Can you help us out here with your take on how catch argument var aliasing works?

[kzc]

Correction:

it appears that the scope of the var alias of the catch argument within the catch block is still the catch block the catch parameter scope but it will contribute an uninitialized var declaration to function scope

[lukastaegert]

I asked around on Twitter and I believe the "correct" explanation is:

• The declaration of e is hoisted beyond the catch scope, as it should
• It is however shadowed within the catch scope by the parameter and the shadowing includes the assignment within the declaration!
  So at the core, the problem is that declaration and initializer assignment are two disconnected operations, and while the declaration is hoisted as usually, the assignment targets a different variable. https://twitter.com/lukastaegert/status/1414804266280235009

[lukastaegert]

But the effect is what you described.

[alexlamsl]

In ES6+ terms catch acts more like let except for allowing var declaration within the same block scope.

try { console.log(x) } catch (_) { console.log('not declared') }
(function() {
    try { console.log(x) } catch (_) { console.log('not declared') }
    x = 'start';
    try { console.log(x) } catch (_) { console.log('not declared') }
    try {
        try { console.log(x) } catch (_) { console.log('not declared') }
        x = 'try';
        try { console.log(x) } catch (_) { console.log('not declared') }
        throw 'throw';
    } catch (x) {
        try { console.log(x) } catch (_) { console.log('not declared') }
        var x = 'catch';
        try { console.log(x) } catch (_) { console.log('not declared') }
    } finally {
        try { console.log(x) } catch (_) { console.log('not declared') }
        x = 'finally';
        try { console.log(x) } catch (_) { console.log('not declared') }
    }
    try { console.log(x) } catch (_) { console.log('not declared') }
})();
try { console.log(x) } catch (_) { console.log('not declared') }

gives:

not declared
undefined
start
start
try
throw
catch
try
finally
finally
not declared

So it's equivalent to (minus all the console.log()'s):

(function() {
    var x; // hoisted from `catch`
    x = 'start';
    {
        x = 'try';
    }
    {
        let x = 'throw';
        x = 'catch';
    }
    {
        x = 'finally';
    }
})();

[kzc]

According to the ES5.1 specification:

The production Catch : catch ( Identifier ) Block is evaluated as follows:

    1. Let C be the parameter that has been passed to this production.
    2. Let oldEnv be the running execution context’s LexicalEnvironment.
    3. Let catchEnv be the result of calling NewDeclarativeEnvironment
       passing oldEnv as the argument.
    4. Call the CreateMutableBinding concrete method of catchEnv passing
       the Identifier String value as the argument.
    5. Call the SetMutableBinding concrete method of catchEnv passing the
       Identifier, C, and false as arguments. Note that the last argument
       is immaterial in this situation.
    6. Set the running execution context’s LexicalEnvironment to catchEnv.
    7. Let B be the result of evaluating Block.
    8. Set the running execution context’s LexicalEnvironment to oldEnv.
    9. Return B.

NOTE No matter how control leaves the Block the LexicalEnvironment is
     always restored to its former state.

NewDeclarativeEnvironment is also used by function and eval. So the catch parameter is in its own function-like temporary scope. The key difference being that the catch block's VariableEnvironment is not overridden and is shared with the parent environment of catch.

[lukastaegert]

Fix at #4178