fix(semantic): incorrect reference in parameters

[Dunqing]

close: #2499

We need to collect parameter bindings. If an IdentifierReference is used on a parameter, we need to check if the current parameter binding is already defined, if not then it can only access the parent scope's bindings.

Let's look at an example.

const foo = 0;
function a(b = foo, c = b) {
   const foo = 1;
   const c = 2;
}  

The parameter has a reference to foo, but there is no foo in the parameter binding, so it must access the parent scope. Let's look at c = b, which has a reference to b, and the first parameter already has a b binding defined, so in the current scope we can access b.

I apologize for adding two more properties to the SemanticBuilder, the current implementation can't avoid adding them. Please let me know if you have a better solution, I'm interested in a better solution!

[Dunqing]

• fix(semantic): incorrect reference in parameters #2644 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #2644 will not alter performance

_{Comparing 03-08-fix_semantic_Incorrect_reference_in_parameters (7550838) with main (c72675e)}

Summary

✅ 29 untouched benchmarks

[Dunqing]

@hyf0 cc

[hyf0]

Would it be easier to create a facade scope for the params part?

[Boshen]

My questions will always be: how does TypeScript do it?

Another questions, can we do everything inside FormalParameters instead of

oxc/crates/oxc_semantic/src/binder.rs

Lines 183 to 242 in c72675e

	impl<'a> Binder for BindingRestElement<'a> {
	// Binds the FormalParameters's rest of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else {
	return;
	};
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let excludes =
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes;
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}
	impl<'a> Binder for FormalParameter<'a> {
	// Binds the FormalParameter of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else { unreachable!() };
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let is_not_allowed_duplicate_parameters = matches!(
	parameters.kind,
	// ArrowFormalParameters: UniqueFormalParameters
	FormalParameterKind::ArrowFormalParameters |
	// UniqueFormalParameters : FormalParameters
	// * It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements.
	FormalParameterKind::UniqueFormalParameters
	) ||
	// Multiple occurrences of the same BindingIdentifier in a FormalParameterList is only allowed for functions which have simple parameter lists and which are not defined in strict mode code.
	builder.strict_mode() ||
	// FormalParameters : FormalParameterList
	// * It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements.
	!parameters.is_simple_parameter_list();
	let excludes = if is_not_allowed_duplicate_parameters {
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes
	} else {
	SymbolFlags::FunctionScopedVariableExcludes
	};
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}

[Dunqing]

My questions will always be: how does TypeScript do it?

I've taken a cursory look at typescript, but I don't see a specific way to handle it yet

Another questions, can we do everything inside FormalParameters instead of

oxc/crates/oxc_semantic/src/binder.rs

Lines 183 to 242 in c72675e

	impl<'a> Binder for BindingRestElement<'a> {
	// Binds the FormalParameters's rest of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else {
	return;
	};
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let excludes =
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes;
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}
	impl<'a> Binder for FormalParameter<'a> {
	// Binds the FormalParameter of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else { unreachable!() };
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let is_not_allowed_duplicate_parameters = matches!(
	parameters.kind,
	// ArrowFormalParameters: UniqueFormalParameters
	FormalParameterKind::ArrowFormalParameters |
	// UniqueFormalParameters : FormalParameters
	// * It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements.
	FormalParameterKind::UniqueFormalParameters
	) ||
	// Multiple occurrences of the same BindingIdentifier in a FormalParameterList is only allowed for functions which have simple parameter lists and which are not defined in strict mode code.
	builder.strict_mode() ||
	// FormalParameters : FormalParameterList
	// * It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements.
	!parameters.is_simple_parameter_list();
	let excludes = if is_not_allowed_duplicate_parameters {
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes
	} else {
	SymbolFlags::FunctionScopedVariableExcludes
	};
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}

CatchClause no FormalParameters

oxc/crates/oxc_ast/src/visit.rs

Lines 291 to 301 in 7550838

	fn visit_catch_clause(&mut self, clause: &CatchClause<'a>) {
	let kind = AstKind::CatchClause(self.alloc(clause));
	self.enter_scope(ScopeFlags::empty());
	self.enter_node(kind);
	if let Some(param) = &clause.param {
	self.visit_binding_pattern(param);
	}
	self.visit_statements(&clause.body.body);
	self.leave_node(kind);
	self.leave_scope();
	}

[Boshen]

I think catch clause is another special case ...

[Dunqing]

Another questions, can we do everything inside FormalParameters instead of

oxc/crates/oxc_semantic/src/binder.rs

Lines 183 to 242 in c72675e

	impl<'a> Binder for BindingRestElement<'a> {
	// Binds the FormalParameters's rest of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else {
	return;
	};
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let excludes =
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes;
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}
	impl<'a> Binder for FormalParameter<'a> {
	// Binds the FormalParameter of a function or method.
	fn bind(&self, builder: &mut SemanticBuilder) {
	let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
	let AstKind::FormalParameters(parameters) = parent_kind else { unreachable!() };
	if parameters.kind.is_signature() {
	return;
	}
	let includes = SymbolFlags::FunctionScopedVariable;
	let is_not_allowed_duplicate_parameters = matches!(
	parameters.kind,
	// ArrowFormalParameters: UniqueFormalParameters
	FormalParameterKind::ArrowFormalParameters |
	// UniqueFormalParameters : FormalParameters
	// * It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements.
	FormalParameterKind::UniqueFormalParameters
	) ||
	// Multiple occurrences of the same BindingIdentifier in a FormalParameterList is only allowed for functions which have simple parameter lists and which are not defined in strict mode code.
	builder.strict_mode() ||
	// FormalParameters : FormalParameterList
	// * It is a Syntax Error if IsSimpleParameterList of FormalParameterList is false and BoundNames of FormalParameterList contains any duplicate elements.
	!parameters.is_simple_parameter_list();
	let excludes = if is_not_allowed_duplicate_parameters {
	SymbolFlags::FunctionScopedVariable | SymbolFlags::FunctionScopedVariableExcludes
	} else {
	SymbolFlags::FunctionScopedVariableExcludes
	};
	self.bound_names(&mut |ident| {
	let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
	ident.symbol_id.set(Some(symbol_id));
	});
	}
	}

Initially, we did everything in FormalParameters, but that would result in the node query by node id not being specific enough
#2114

[Boshen]

Ahh ... I'll take a look at this next week :-(