fix(ast/estree): Fix TSFunctionType and TSCallSignatureDeclaration

[therewillbecode]

Skips estree serialisation for the field named this_param for both TSFunctionType and TSCallSignatureDeclaration nodes.

PR is part of ongoing work to align our AST's ESTree output with that of TS-ESLint's. Relates to #9705.

[graphite-app]

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #9959 will not alter performance

_{Comparing therewillbecode:fix/skip-this-param-estree (35b3457) with main (0cdeedd)}

Summary

✅ 33 untouched benchmarks

[overlookmotel]

👍

I'd guess (but don't know) that the this_param field isn't ignored entirely. It probably gets added to the start of params, same as in Function:

oxc/crates/oxc_ast/src/ast/js.rs

Lines 1701 to 1705 in 0cdeedd

	/// Function parameters.
	///
	/// Does not include `this` parameters used by some TypeScript functions.
	#[estree(via = FunctionFormalParameters)]
	pub params: Box<'a, FormalParameters<'a>>,

#[estree(via = FunctionFormalParameters)] refers to this custom serializer (a bit tricky):

oxc/crates/oxc_ast/src/serialize.rs

Lines 384 to 421 in 0cdeedd

	/// Serializer for `params` field of `Function`.
	///
	/// In TS AST, this adds `this_param` to start of the array.
	#[ast_meta]
	#[estree(
	ts_type = "ParamPattern[]",
	raw_deser = "
	const params = DESER[Box<FormalParameters>](POS_OFFSET.params);
	/* IF_TS */
	const thisParam = DESER[Option<Box<TSThisParameter>>](POS_OFFSET.this_param)
	if (thisParam !== null) params.unshift(thisParam);
	/* END_IF_TS */
	params
	"
	)]
	pub struct FunctionFormalParameters<'a, 'b>(pub &'b Function<'a>);
	impl ESTree for FunctionFormalParameters<'_, '_> {
	fn serialize<S: Serializer>(&self, serializer: S) {
	let mut seq = serializer.serialize_sequence();
	if S::INCLUDE_TS_FIELDS {
	if let Some(this_param) = &self.0.this_param {
	seq.serialize_element(this_param);
	}
	}
	for item in &self.0.params.items {
	seq.serialize_element(item);
	}
	if let Some(rest) = &self.0.params.rest {
	seq.serialize_element(&FormalParametersRest(rest));
	}
	seq.end();
	}
	}

But maybe better to catch more of the "low hanging fruit" first, and come back to these more complex ones.

[overlookmotel]

Oh damn it! Merge conflicts (probably conflicts with your other PR which I just merged).

If you can rebase on main and run just test-estree again, that should sort it.

[overlookmotel]

I fixed the conflicts. Will merge as soon as CI goes green.

[therewillbecode]

👍

I'd guess (but don't know) that the this_param field isn't ignored entirely. It probably gets added to the start of params, same as in Function:

oxc/crates/oxc_ast/src/ast/js.rs

Lines 1701 to 1705 in 0cdeedd

	/// Function parameters.
	///
	/// Does not include `this` parameters used by some TypeScript functions.
	#[estree(via = FunctionFormalParameters)]
	pub params: Box<'a, FormalParameters<'a>>,

#[estree(via = FunctionFormalParameters)] refers to this custom serializer (a bit tricky):

oxc/crates/oxc_ast/src/serialize.rs

Lines 384 to 421 in 0cdeedd

	/// Serializer for `params` field of `Function`.
	///
	/// In TS AST, this adds `this_param` to start of the array.
	#[ast_meta]
	#[estree(
	ts_type = "ParamPattern[]",
	raw_deser = "
	const params = DESER[Box<FormalParameters>](POS_OFFSET.params);
	/* IF_TS */
	const thisParam = DESER[Option<Box<TSThisParameter>>](POS_OFFSET.this_param)
	if (thisParam !== null) params.unshift(thisParam);
	/* END_IF_TS */
	params
	"
	)]
	pub struct FunctionFormalParameters<'a, 'b>(pub &'b Function<'a>);
	impl ESTree for FunctionFormalParameters<'_, '_> {
	fn serialize<S: Serializer>(&self, serializer: S) {
	let mut seq = serializer.serialize_sequence();
	if S::INCLUDE_TS_FIELDS {
	if let Some(this_param) = &self.0.this_param {
	seq.serialize_element(this_param);
	}
	}
	for item in &self.0.params.items {
	seq.serialize_element(item);
	}
	if let Some(rest) = &self.0.params.rest {
	seq.serialize_element(&FormalParametersRest(rest));
	}
	seq.end();
	}
	}

But maybe better to catch more of the "low hanging fruit" first, and come back to these more complex ones.

Ah so that is what via does, it says use this type that wraps the AST type. Where the wrapper type implements the custom serialization logic. So I guess that is how we take case of dynamically serialising a field on a node or not depending on what its parent looks like.

[therewillbecode]

Thanks for the review and fixing the conflicts @overlookmotel !