Correctly handle catch declarations (#4108)

* Correctly handle catch declarations

* Improve coverage

src/ast/nodes/CatchClause.ts

@@ -17,20 +17,16 @@ export default class CatchClause extends NodeBase {
 		this.scope = new CatchScope(parentScope, this.context);
 	}
 
-	initialise(): void {
-		if (this.param) {
-			this.param.declare('parameter', UNKNOWN_EXPRESSION);
-		}
-	}
-
 	parseNode(esTreeNode: GenericEsTreeNode): void {
-		this.body = new this.context.nodeConstructors.BlockStatement(
-			esTreeNode.body,
-			this,
-			this.scope
-		) as BlockStatement;
+		// 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
+		const { param } = esTreeNode;
+		if (param) {
+			(this.param as GenericEsTreeNode) = new (this.context.nodeConstructors[param.type] ||
+				this.context.nodeConstructors.UnknownNode)(param, this, this.scope);
+			this.param!.declare('parameter', UNKNOWN_EXPRESSION);
+		}
 		super.parseNode(esTreeNode);
 	}
 }
-
-CatchClause.prototype.preventChildBlockScope = true;

src/ast/scopes/CatchScope.ts

@@ -11,10 +11,13 @@ export default class CatchScope extends ParameterScope {
 		init: ExpressionEntity | null,
 		isHoisted: boolean
 	): LocalVariable {
-		if (isHoisted) {
-			return this.parent.addDeclaration(identifier, context, init, isHoisted);
-		} else {
-			return super.addDeclaration(identifier, context, init, false);
+		const existingParameter = this.variables.get(identifier.name) as LocalVariable;
+		if (existingParameter) {
+			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-block-scope/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'uses correct scope in catch blocks'
+};

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

@@ -0,0 +1,17 @@
+try {
+	throw 'FAIL';
+} catch (t) {
+	var t = 'PASS';
+	assert.strictEqual(t, 'PASS');
+}
+
+let a = 1;
+let def = 'PASS2';
+try {
+	throw ['FAIL2', 'PASS1'];
+} catch ({ [a]: b, 3: d = def }) {
+	let a = 0,
+		def = 'FAIL3';
+	assert.strictEqual(b, 'PASS1');
+	assert.strictEqual(d, 'PASS2');
+}