[internal] ComparableExpr (f)strings and bytes made invariant under concatenation

[dylwil3]

Summary

This PR modifies the construction of ComparableExpr objects from Expr objects in order to ensure that the following equalities hold at the level of comparable AST nodes:

"a" "b" == "ab"
b"a" b"b" == b"ab"
"text" f"{foo} more" "text" f"{bar}" == f"text{foo} moretext{bar}"

• ExprStringLiteral
• ExprBytesLiteral
• ExprFString

Closes #13291

Details

• ExprStringLiteral: The comparable variant simply stores the concatenated string value using the to_str method
• ExprBytesLiteral: The comparable variant stores a Cow<'ast, [u8]> which is owned for concatenated bytes expressions and borrowed otherwise.
• ExprFString: The comparable variant stores the vector of FStringElement's obtained by joining together contiguous sequences of StringLiteral and FString elements. So, for example

"text" f"{foo} more" "text" f"{bar}"

would be interpreted as the vector with components"text", f"{foo}", " moretext", and f"{bar}".

Test Plan

• Unit tests with hand-crafted AST nodes
• Integration tests with parsed source code

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[MichaReiser]

This is great and nice work on f-strings. I've a few suggestions on how we can reduce the code for the f-string transformation and I think it would be great if we can reduce the test-boilerplate to improve readability.

[MichaReiser]

Nit: We can simplify this a bit

Suggested change

	ast::FStringPart::Literal(string_literal) => match &mut literal_accumulator {
	None => {
	literal_accumulator = Some(Cow::Borrowed(string_literal.as_str()));
	}
	Some(existing_literal) => match existing_literal {
	Cow::Borrowed(s1) => {
	let mut s = String::with_capacity(s1.len() + string_literal.len());
	s.push_str(s1);
	s.push_str(string_literal.as_str());
	*existing_literal = Cow::Owned(s);
	}
	Cow::Owned(ref mut s1) => {
	s1.push_str(string_literal.as_str());
	}
	},
	},
	ast::FStringPart::FString(fstring) => {
	ast::FStringPart::Literal(string_literal) => {
	literal_accumulator = Some(match literal_accumulator.take() {
	None => Cow::Borrowed(string_literal),
	Some(existing_literal) => {
	let mut s = existing_literal.into_owned();
	s.push_str(string_literal);
	s.into()
	}
	});
	},
	ast::FStringPart::FString(fstring) => {

[dylwil3]

Done, but superseded by Collector.

[MichaReiser]

Nit: This can be simplified to

Suggested change

	Some(existing_literal) => match existing_literal {
	Cow::Borrowed(s1) => {
	let mut s = String::with_capacity(s1.len() + literal.len());
	s.push_str(s1);
	s.push_str(literal);
	*existing_literal = Cow::Owned(s);
	}
	Cow::Owned(ref mut s1) => {
	s1.push_str(literal);
	}
	},
	Some(existing_literal) => existing_literal.to_mut().push_str(&literal.value),

[dylwil3]

Done (but now inside Collector)

[MichaReiser]

Suggested change

	elements: Vec<ComparableFStringElement<'a>>,
	elements: Box<[ComparableFStringElement<'a>]>,

[dylwil3]

Done.

[MichaReiser]

The word stack here is a bit confusing because the implementation doesn't make use of a stack. It uses a single "slot" storing the last string literal.

I think I'm good with the above comment because the most important informatin that I wasn't aware of is that adjacent string literals need to be concatenated.

[dylwil3]

Reworded a bit.

[MichaReiser]

Thanks for taking the time to write all those tests.

I don't have a great suggestion but I wonder if we should make them integration tests instead, because they're very verbose and are difficult to read. E.g. there's so much boilerplate that I find it difficult to pick up the important bits. I also assume that they were very annoying to write. What do you think?

[dylwil3]

The integration tests have the same content so I elected to remove these verbose unit tests entirely. They were helpful for development (and for my own understanding of the internals of f-string nodes), but keeping them around is probably more trouble than they're worth!

[MichaReiser]

This is great! Thank you!