refactor(rome_js_analyze): has_truthy_value

[ematipico]

Summary

This PR refactors some common logic among some rules; the new code is now implemented as API for the JsxAttribute.

This is a pattern that is popping up in our rules, it was good to have a specialized function that checks value of the attributes.

Test Plan

Current snapshots should not change.

Documentation

• The PR requires documentation
• I will create a new PR to update the documentation

[netlify]

✅ Deploy Preview for docs-rometools canceled.

Name	Link
🔨 Latest commit	b3e3bf6
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rometools/deploys/63a1b446db5b48000933a88d

[github-actions]

Parser conformance results on ubuntu-latest

js/262

Test result	main count	This PR count	Difference
Total	48647	48647	0
Passed	47582	47582	0
Failed	1065	1065	0
Panics	0	0	0
Coverage	97.81%	97.81%	0.00%

jsx/babel

Test result	main count	This PR count	Difference
Total	40	40	0
Passed	37	37	0
Failed	3	3	0
Panics	0	0	0
Coverage	92.50%	92.50%	0.00%

symbols/microsoft

Test result	main count	This PR count	Difference
Total	6093	6093	0
Passed	1754	1754	0
Failed	4339	4339	0
Panics	0	0	0
Coverage	28.79%	28.79%	0.00%

ts/babel

Test result	main count	This PR count	Difference
Total	639	639	0
Passed	565	565	0
Failed	74	74	0
Panics	0	0	0
Coverage	88.42%	88.42%	0.00%

ts/microsoft

Test result	main count	This PR count	Difference
Total	16740	16740	0
Passed	12817	12817	0
Failed	3923	3923	0
Panics	0	0	0
Coverage	76.57%	76.57%	0.00%

[MichaReiser]

It wasn't immediately clear to me what the closure is doing from reading the call sites only

What do you think of implementing this method as an extension method on SyntaxTokenText instead. Doing so would remove the need for the closure and the matching for many tokens could even be performed on the token kind rather than the text

[ematipico]

I had implemented the function without the use of the closure, but unfortunately, that degraded the DX, because I couldn't use the APIs I wanted.

On the other end, I didn't want to return SyntaxTokenText instead of bool because the function doesn't always return the text, but it actually checks if it contains a "truthy" value, so it felt weird.

[MichaReiser]

OK. I think I understand know what is happening. I wonder if it would be easier to understand if attribute has a function as_static_value() -> Option<StaticAttributeValue>

enum StaticAttributeValue {
	Token(JsSyntaxToken),
	// For strings. Or you can have `StringLiteral`, and `JsxString` variants if you don't want to duplicate the `inner_text` methods
	String(JsSyntaxToken),
}

impl StaticAttributeValue {
  fn is_truthy(&self) -> bool { ... }
  fn text(&self) -> &str { .. }
  fn is_falsy(&self) -> bool {}
}

It avoids the somewhat awkward combination of truthy with any other check that the caller wants to perform.

[MichaReiser]

I spent a bit more time thinking about this and it should be possible to even go one step further:

1. Change inner_string_text to return a SyntaxResult<QuotedString> where QuotedString is a thin wrapper around a token

struct QuotedString(JsSyntaxToken);

impl QuotedString {
  fn new(token: JsSyntaxToken) -> Self {
	assert!(matches!(token.kind(), STRING_LITERAL_TOKEN | JSX_STRING_TOKEN | ...));

	Self(token)
  }

  fn text(self) -> &str {
	self.0.text_trimmed().trim_start(['"', '\'']).trim_end(['"', '\''])
  }

  fn quoted_text(&self) -> &str { self.0.text_trimmed() }

 ...
}

impl Deref for QuotedString {
	type Target = str;

	fn deref(&self) -> &Self::Target { self.text() }
}

2. Introduce a new enum JsStaticValue

enum StaticValue {
	Boolean(JsSyntaxToken),
	Null(JsSyntaxToken),
	Undefined(JsSyntaxToken),
	Number(JsSyntaxToken),
	String(QuotedString),
	TemplateChunk(JsSyntaxToken),
}

impl StaticValue {
  fn is_truthy(&self) -> bool {... }
  fn text(&self) -> &str { ... }
}

3. Add a AnyJsExpression::as_static_value() method that returns Optionand do the same forAnyJsxAttribute`
4. Stretch: Replace the as_string_constant method with a method that returns Option<StringConstant> value where StringConstant is an enum. Finally, remove the with_string_constantmethod (you can useas_string_constant` directly.

enum StringConstant {
	TemplateChunk(JsSyntaxToken),
	String(QuotedString)
}

impl StringConstant  {
  fn text(&self) {
	match self {
		StringConstant::TemplateChunk(token) => token.text_trimmed(),
		StringConstant::String(quoted) => quoted.text()
	}
  }
}

impl Deref for StringConstant { ... }

[github-actions]

This PR is stale because it has been open 14 days with no activity.

[nissy-dev]

The continuos work is #4342, so I close this PR.