💅 lint/suspicious/noDuplicateTestHooks cannot handle test options argument

[tim-we]

Environment information

$ npx biome rage --linter
CLI:
  Version:                      2.3.7
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_PATH:               unset
  BIOME_LOG_PREFIX_NAME:        unset
  BIOME_CONFIG_PATH:            unset
  BIOME_THREADS:                unset
  NO_COLOR:                     unset
  TERM:                         xterm-256color
  JS_RUNTIME_VERSION:           v24.10.0
  JS_RUNTIME_NAME:              node
  NODE_PACKAGE_MANAGER:         npm/11.6.1

Biome Configuration:
  Status:                       Loaded successfully
  Path:                         /home/tim-we/biome-test/biome.json
  Formatter enabled:            true
  Linter enabled:               true
  Assist enabled:               true
  VCS enabled:                  false

Linter:
  JavaScript enabled:           unset
  JSON enabled:                 unset
  CSS enabled:                  unset
  GraphQL enabled:              unset
  Recommended:                  false
  Enabled rules:
    suspicious/noDuplicateTestHooks

Workspace:
  Open Documents:               0

Rule name

lint/suspicious/noDuplicateTestHooks

Playground link

https://biomejs.dev/playground/?lintRules=noDuplicateTestHooks&code=aQBtAHAAbwByAHQAIAB7AGIAZQBmAG8AcgBlAEUAYQBjAGgALAAgAGQAZQBzAGMAcgBpAGIAZQAsACAAZQB4AHAAZQBjAHQALAAgAHQAZQBzAHQAfQAgAGYAcgBvAG0AIAAiAHYAaQB0AGUAcwB0ACIAOwAKAAoAZABlAHMAYwByAGkAYgBlACgAIgBvAHUAdABlAHIAIgAsACAAKAApACAAPQA%2BACAAewAKACAAIABiAGUAZgBvAHIAZQBFAGEAYwBoACgAKAApACAAPQA%2BACAAewAKACAAIAAgACAAYwBvAG4AcwBvAGwAZQAuAGwAbwBnACgAIgBCAGUAZgBvAHIAZQAgAGUAdgBlAHIAeQAgAHQAZQBzAHQAIgApADsACgAgACAAfQApADsACgAKACAAIABkAGUAcwBjAHIAaQBiAGUAKAAiAGkAbgBuAGUAcgAgADEAIgAsACAAKAApACAAPQA%2BACAAewAKACAAIAAgACAAdABlAHMAdAAoACIAaQBuAG4AZQByACAAdABlAHMAdAAgADEAIgAsACAAKAApACAAPQA%2BACAAewAKACAAIAAgACAAIAAgAGUAeABwAGUAYwB0ACgAMQArADEAKQAuAHQAbwBCAGUAKAAyACkAOwAKACAAIAAgACAAfQApADsACgAgACAAfQApADsACgAKACAAIAAvAC8AIAB0AGUAcwB0ACAAbwBwAHQAaQBvAG4AcwAgAGEAcgBlACAAcgBlAHEAdQBpAHIAZQBkACAAdABvACAAcgBlAHAAcgBvAGQAdQBjAGUAIAB0AGgAZQAgAGIAdQBnAAoAIAAgAGQAZQBzAGMAcgBpAGIAZQAoACIAaQBuAG4AZQByACAAMgAiACwAIAB7AHIAZQB0AHIAeQA6ACAAMgB9ACwAIAAoACkAIAA9AD4AIAB7AAoAIAAgACAAIABiAGUAZgBvAHIAZQBFAGEAYwBoACgAKAApACAAPQA%2BACAAewAKACAAIAAgACAAIAAgAGMAbwBuAHMAbwBsAGUALgBsAG8AZwAoACIAYgBlAGYAbwByAGUAIABpAG4AbgBlAHIAIAAyACIAKQA7AAoAIAAgACAAIAB9ACkAOwAKAAoAIAAgACAAIAB0AGUAcwB0ACgAIgBpAG4AbgBlAHIAIAB0AGUAcwB0ACAAMgAiACwAIAAoACkAIAA9AD4AIAB7AAoAIAAgACAAIAAgACAAZQB4AHAAZQBjAHQAKAAxACsATQBhAHQAaAAuAHIAbwB1AG4AZAAoADIAIAAqACAATQBhAHQAaAAuAHIAYQBuAGQAbwBtACgAKQApACkALgB0AG8AQgBlACgAMwApADsACgAgACAAIAAgAH0AKQAKACAAIAB9ACkAOwAKAH0AKQA%3D

Expected result

Vitest allows you to nest describe(...) function calls and it sometimes makes sense to use a test hook in the outer and another one in the inner describe call. Biome seems to support this unless you use the test options argument. Then it reports a duplicate test hook.

This is valid vitest code, as you can see here: https://stackblitz.com/edit/vitest-dev-vitest-v3jy9zzt?file=biome.test.ts

If this gets acknowledged as a bug and you are willing to receive PRs for this I will look into it when I find the time.

Code of Conduct

• I agree to follow Biome's Code of Conduct

[ematipico]

Thank you @tim-we for wanting to fix the issue

You should take a look around here, that's where test calls are identified

https://github.com/biomejs/biome/blob/main/crates%2Fbiome_js_syntax%2Fsrc%2Fexpr_ext.rs#L943-L1063

One more thing, these functions are also used by the formatter, so there's a chance we fix the formatter too. If that's the case, we should add a changeset for that too, plus a test

[tim-we]

Thanks for the starting point. I think this is the function I will have to change:

biome/crates/biome_js_syntax/src/expr_ext.rs

Lines 1957 to 2059 in a226b28

	/// This is a specialized function that checks if the current [call expression]
	/// resembles a call expression usually used by a testing frameworks.
	///
	/// If the [call expression] matches the criteria, a different formatting is applied.
	///
	/// To evaluate the eligibility of a [call expression] to be a test framework like,
	/// we need to check its [callee] and its [arguments].
	///
	/// 1. The [callee] must contain a name or a chain of names that belongs to the
	/// test frameworks, for example: `test()`, `test.only()`, etc.
	/// 2. The [arguments] should be at the least 2
	/// 3. The first argument has to be a string literal
	/// 4. The third argument, if present, has to be a number literal
	/// 5. The second argument has to be an [arrow function expression] or [function expression]
	/// 6. Both function must have zero or one parameters
	///
	/// [call expression]: crate::JsCallExpression
	/// [callee]: crate::AnyJsExpression
	/// [arguments]: crate::JsCallArgumentList
	/// [arrow function expression]: crate::JsArrowFunctionExpression
	/// [function expression]: crate::JsCallArgumentList
	pub fn is_test_call_expression(&self) -> SyntaxResult<bool> {
	use AnyJsExpression::*;
	let callee = self.callee()?;
	let arguments = self.arguments()?;
	let mut args = arguments.args().iter();
	match (args.next(), args.next(), args.next()) {
	(Some(Ok(argument)), None, None) if arguments.args().len() == 1 => {
	if is_angular_test_wrapper(&self.clone().into())
	&& self
	.parent::<JsCallArgumentList>()
	.and_then(|arguments_list| arguments_list.parent::<JsCallArguments>())
	.and_then(|arguments| arguments.parent::<Self>())
	.is_some_and(|parent| parent.is_test_call_expression().unwrap_or(false))
	{
	return Ok(matches!(
	argument,
	AnyJsCallArgument::AnyJsExpression(
	JsArrowFunctionExpression(_) | JsFunctionExpression(_)
	)
	));
	}
	if is_unit_test_set_up_callee(&callee) {
	return Ok(argument
	.as_any_js_expression()
	.is_some_and(is_angular_test_wrapper));
	}
	Ok(false)
	}
	// it("description", ..)
	// it(Test.name, ..)
	(Some(Ok(AnyJsCallArgument::AnyJsExpression(_))), Some(Ok(second)), third)
	if arguments.args().len() <= 3
	&& (callee.contains_a_test_pattern()
	|| matches!(callee, AnyJsExpression::JsCallExpression(call_expression) if call_expression.callee()?.contains_a_test_each_pattern())) =>
	{
	// it('name', callback, duration)
	if !matches!(
	third,
	None | Some(Ok(AnyJsCallArgument::AnyJsExpression(
	AnyJsLiteralExpression(
	self::AnyJsLiteralExpression::JsNumberLiteralExpression(_)
	)
	)))
	) {
	return Ok(false);
	}
	if second
	.as_any_js_expression()
	.is_some_and(is_angular_test_wrapper)
	{
	return Ok(true);
	}
	let (parameters, has_block_body) = match second {
	AnyJsCallArgument::AnyJsExpression(JsFunctionExpression(function)) => (
	function
	.parameters()
	.map(AnyJsArrowFunctionParameters::from),
	true,
	),
	AnyJsCallArgument::AnyJsExpression(JsArrowFunctionExpression(arrow)) => (
	arrow.parameters(),
	arrow
	.body()
	.is_ok_and(|body| matches!(body, AnyJsFunctionBody::JsFunctionBody(_))),
	),
	_ => return Ok(false),
	};
	Ok(arguments.args().len() == 2 || (parameters?.len() <= 1 && has_block_body))
	}
	_ => Ok(false),
	}
	}
	}

[Bertie690]

Looking inot the history with #7287, yes that does look like the right function.