Correctly create outside variable when shadowed by catch parameter (#…

…4178)

src/ast/nodes/CatchClause.ts

@@ -18,9 +18,9 @@ export default class CatchClause extends NodeBase {
 	}
 
 	parseNode(esTreeNode: GenericEsTreeNode): void {
-		// Parameters need to be declared first as the logic is that hoisted body
-		// variables are associated with outside vars unless there is a parameter,
-		// in which case they are associated with the parameter
+		// Parameters need to be declared first as the logic is that initializers
+		// of hoisted body variables are associated with parameters of the same
+		// name instead of the variable
 		const { param } = esTreeNode;
 		if (param) {
 			(this.param as GenericEsTreeNode) = new (this.context.nodeConstructors[param.type] ||

src/ast/scopes/CatchScope.ts

@@ -1,6 +1,7 @@
 import { AstContext } from '../../Module';
 import Identifier from '../nodes/Identifier';
 import { ExpressionEntity } from '../nodes/shared/Expression';
+import { UNDEFINED_EXPRESSION } from '../values';
 import LocalVariable from '../variables/LocalVariable';
 import ParameterScope from './ParameterScope';
 
@@ -13,10 +14,13 @@ export default class CatchScope extends ParameterScope {
 	): LocalVariable {
 		const existingParameter = this.variables.get(identifier.name) as LocalVariable;
 		if (existingParameter) {
+			// While we still create a hoisted declaration, the initializer goes to
+			// the parameter. Note that technically, the declaration now belongs to
+			// two variables, which is not correct but should not cause issues.
+			this.parent.addDeclaration(identifier, context, UNDEFINED_EXPRESSION, isHoisted);
 			existingParameter.addDeclaration(identifier, init);
 			return existingParameter;
 		}
-		// as parameters are handled differently, all remaining declarations are hoisted
 		return this.parent.addDeclaration(identifier, context, init, isHoisted);
 	}
 }

test/function/samples/catch-scope-shadowing/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'correctly associates shadowed variables in catch scopes'
+};

test/function/samples/catch-scope-shadowing/main.js

@@ -0,0 +1,23 @@
+var e = 'failed1',
+	x = 'value';
+
+(function () {
+	try {
+		// empty
+	} catch (e) {
+		var e = 'failed2';
+	}
+
+	assert.strictEqual(e, undefined);
+	assert.strictEqual(x, undefined);
+	x = 'failed3';
+	return;
+
+	try {
+		not_reached();
+	} catch (x) {
+		var x = 'failed4';
+	}
+})();
+
+assert.strictEqual(x, 'value');