mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 12:55:05 +00:00
cdac90ef68
Rebase of #6365 authored by @davidszotten. ## Summary This PR updates the AST structure for an f-string elements. The main **motivation** behind this change is to have a dedicated node for the string part of an f-string. Previously, the existing `ExprStringLiteral` node was used for this purpose which isn't exactly correct. The `ExprStringLiteral` node should include the quotes as well in the range but the f-string literal element doesn't include the quote as it's a specific part within an f-string. For example, ```python f"foo {x}" # ^^^^ # This is the literal part of an f-string ``` The introduction of `FStringElement` enum is helpful which represent either the literal part or the expression part of an f-string. ### Rule Updates This means that there'll be two nodes representing a string depending on the context. One for a normal string literal while the other is a string literal within an f-string. The AST checker is updated to accommodate this change. The rules which work on string literal are updated to check on the literal part of f-string as well. #### Notes 1. The `Expr::is_literal_expr` method would check for `ExprStringLiteral` and return true if so. But now that we don't represent the literal part of an f-string using that node, this improves the method's behavior and confines to the actual expression. We do have the `FStringElement::is_literal` method. 2. We avoid checking if we're in a f-string context before adding to `string_type_definitions` because the f-string literal is now a dedicated node and not part of `Expr`. 3. Annotations cannot use f-string so we avoid changing any rules which work on annotation and checks for `ExprStringLiteral`. ## Test Plan - All references of `Expr::StringLiteral` were checked to see if any of the rules require updating to account for the f-string literal element node. - New test cases are added for rules which check against the literal part of an f-string. - Check the ecosystem results and ensure it remains unchanged. ## Performance There's a performance penalty in the parser. The reason for this remains unknown as it seems that the generated assembly code is now different for the `__reduce154` function. The reduce function body is just popping the `ParenthesizedExpr` on top of the stack and pushing it with the new location. - The size of `FStringElement` enum is the same as `Expr` which is what it replaces in `FString::format_spec` - The size of `FStringExpressionElement` is the same as `ExprFormattedValue` which is what it replaces I tried reducing the `Expr` enum from 80 bytes to 72 bytes but it hardly resulted in any performance gain. The difference can be seen here: - Original profile: https://share.firefox.dev/3Taa7ES - Profile after boxing some node fields: https://share.firefox.dev/3GsNXpD ### Backtracking I tried backtracking the changes to see if any of the isolated change produced this regression. The problem here is that the overall change is so small that there's only a single checkpoint where I can backtrack and that checkpoint results in the same regression. This checkpoint is to revert using `Expr` to the `FString::format_spec` field. After this point, the change would revert back to the original implementation. ## Review process The review process is similar to #7927. The first set of commits update the node structure, parser, and related AST files. Then, further commits update the linter and formatter part to account for the AST change. --------- Co-authored-by: David Szotten <davidszotten@gmail.com>
124 lines
4.3 KiB
Rust
124 lines
4.3 KiB
Rust
use ruff_text_size::TextRange;
|
|
|
|
use crate::visitor::transformer::{walk_expr, walk_keyword, Transformer};
|
|
use crate::{nodes, Expr, Keyword};
|
|
|
|
/// Change an expression's location (recursively) to match a desired, fixed
|
|
/// range.
|
|
pub fn relocate_expr(expr: &mut Expr, range: TextRange) {
|
|
Relocator { range }.visit_expr(expr);
|
|
}
|
|
|
|
#[derive(Debug)]
|
|
struct Relocator {
|
|
range: TextRange,
|
|
}
|
|
|
|
impl Transformer for Relocator {
|
|
fn visit_expr(&self, expr: &mut Expr) {
|
|
match expr {
|
|
Expr::BoolOp(nodes::ExprBoolOp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::NamedExpr(nodes::ExprNamedExpr { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::BinOp(nodes::ExprBinOp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::UnaryOp(nodes::ExprUnaryOp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Lambda(nodes::ExprLambda { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::IfExp(nodes::ExprIfExp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Dict(nodes::ExprDict { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Set(nodes::ExprSet { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::ListComp(nodes::ExprListComp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::SetComp(nodes::ExprSetComp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::DictComp(nodes::ExprDictComp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::GeneratorExp(nodes::ExprGeneratorExp { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Await(nodes::ExprAwait { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Yield(nodes::ExprYield { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::YieldFrom(nodes::ExprYieldFrom { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Compare(nodes::ExprCompare { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Call(nodes::ExprCall { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::FString(nodes::ExprFString { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::StringLiteral(nodes::ExprStringLiteral { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::BytesLiteral(nodes::ExprBytesLiteral { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::NumberLiteral(nodes::ExprNumberLiteral { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::BooleanLiteral(nodes::ExprBooleanLiteral { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::NoneLiteral(nodes::ExprNoneLiteral { range }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::EllipsisLiteral(nodes::ExprEllipsisLiteral { range }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Attribute(nodes::ExprAttribute { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Subscript(nodes::ExprSubscript { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Starred(nodes::ExprStarred { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Name(nodes::ExprName { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::List(nodes::ExprList { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Tuple(nodes::ExprTuple { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::Slice(nodes::ExprSlice { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
Expr::IpyEscapeCommand(nodes::ExprIpyEscapeCommand { range, .. }) => {
|
|
*range = self.range;
|
|
}
|
|
}
|
|
walk_expr(self, expr);
|
|
}
|
|
|
|
fn visit_keyword(&self, keyword: &mut Keyword) {
|
|
keyword.range = self.range;
|
|
walk_keyword(self, keyword);
|
|
}
|
|
}
|