ruff
ruff copied to clipboard
Rename string AST nodes
Rename the string nodes for better understanding between the types. The following renames are performed:
Enum variants:
Expr::StringLiteral->Expr::StringExpr::BytesLiteral->Expr::BytesFStringPart::Literal->FStringPart::String- This includes the string literal part of an implictly concatenated f-string (e.g."foo" f"bar {x}")FStringExprValue::literals->FStringExprValue::strings
AST nodes:
ExprStringLiteral->ExprStringExprBytesLiteral->ExprBytes
To better reflect that the following are expressions:
StringValue->StringExprValueBytesValue->BytesExprValueFStringValue->FStringExprValue
Open ended renames
LiteralExpressionRef::StringLiteral->LiteralExpressionRef::String(enum variant)LiteralExpressionRef::BytesLiteral->LiteralExpressionRef::Bytes(enum variant)StringLike::StringLiteral->StringLike::String(enum variant)StringLike::BytesLiteral->StringLike::Bytes(enum variant)FStringElement/FStringPart->FStringComponent(enum type)- Expand
FStringtoFormattedStringeverywhere (struct, enum)
LibCST f-string names: https://github.com/Instagram/LibCST/blob/52bbff6dfc2dd7b3086710bc64896ccfaaed1a3d/native/libcst/src/nodes/expression.rs#L2430-L2443
FormattedStringFormattedStringContent(akaFStringElement)FormattedStringText(akaFStringLiteralElement)FormattedStringExpression(akaFStringExpressionElement)
I like the rename of FStringElement to FStringContent as "content" better describes the value between the quotes or a "string content".
Reference:
- https://github.com/astral-sh/ruff/pull/8835#discussion_r1414759829
- https://github.com/astral-sh/ruff/pull/9058#pullrequestreview-1778954720
I only looked at the changes in nodes.rs so far. Here a few notes (before you perform the entire rename):
is_literal_expr: I guess this is a downside of removing theLiteralsuffix fromExprStringandExprBytesbecause it may now be unexpected thatis_literal_exprreturnstruefor these two nodes. Maybe rename tois_constant()?- The
valuetypes: I find that they're adding cognitive overhead because it isn't a real concept that exists, but a optimization to avoid allocating in the common case of non-concatenated strings. Ideally, they would be hidden from the public API as discussed in the string formatting PR - You renamed the
literalsmethods tostrings. i think I likedliteralsbetter because I would expect thatstringsreturnsExprStrings and not literals. FString: I would rename it toFStringLiteralto be consistent withStringLiteralandBytesLiteralFStringPart::String: I think I would either preferFStringLiteral::StringandFStringLiteral::FString(although the name then conflicts with theFStringLiteral, maybeExprFStringLiteralorAnyFStringLiteral) orFStringPart::StringLiteralandFStringPart::FStringLiteral(clippy might complain).- FStrings with parts and elements always trip me over... I wonder if it's worth re-using
StringLiteralin FStrings or if we should normalize all of them toFStringLiteraland add ais_f_stringboolean flag which is true if the literal uses thefflag. It would remove the need for the entireFStringPartwhich adds a significant overhead in my view. Although it would require supporting more flags than justfbecause it's valid to mix f-strings with raw and unicode string literalsf"a" u"b"orf"a" r"b".
@dhruvmanila is this still relevant?
@dhruvmanila is this still relevant?
Yes, although I'll close this and take this up once the parser rewrite is finished to avoid rebasing and the information is already captured in PR description and Micha's review comment.
