Skip to content

Commit

Permalink
add DebugText for self-documenting f-strings
Browse files Browse the repository at this point in the history 
instead of modelling self-documenting f-strings (`f"{ foo= }"`) as a
(simplified)
`Constant("foo=")` followed by a `FormattedValue(Expr("x"))`, instead
model this case with a `DebugText(leading, trailing)` attribute on the
`FormattedValue` so that we don't have to synthesize nodes (which
results in siblings with overlapping ranges). We need to be able to
preserve the whitespace for self-documenting f-strings, as well as
reproduce the source (eg unparse, format).
  • Loading branch information
@davidszotten
davidszotten committed Jul 29, 2023
1 parent 3c99fbf commit 757dc4f
Show file tree
Hide file tree
Showing 26 changed files with 144 additions and 151 deletions.
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flynt/helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ruff_text_size::TextRange;
fn to_formatted_value_expr(inner: &Expr) -> Expr {
let node = ast::ExprFormattedValue {
value: Box::new(inner.clone()),
debug_text: None,
conversion: ConversionFlag::None,
format_spec: None,
range: TextRange::default(),
Expand Down
18 changes: 18 additions & 0 deletions crates/ruff_python_ast/src/comparable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,26 @@ pub struct ExprCall<'a> {
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ExprFormattedValue<'a> {
value: Box<ComparableExpr<'a>>,
debug_text: Option<DebugText<'a>>,
conversion: ast::ConversionFlag,
format_spec: Option<Box<ComparableExpr<'a>>>,
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct DebugText<'a> {
leading: &'a str,
trailing: &'a str,
}

impl<'a> From<&'a ast::DebugText> for DebugText<'a> {
fn from(debug_text: &'a ast::DebugText) -> Self {
Self {
leading: debug_text.leading.as_str(),
trailing: debug_text.trailing.as_str(),
}
}
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ExprJoinedStr<'a> {
values: Vec<ComparableExpr<'a>>,
Expand Down Expand Up @@ -849,11 +865,13 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
ast::Expr::FormattedValue(ast::ExprFormattedValue {
value,
conversion,
debug_text,
format_spec,
range: _range,
}) => Self::FormattedValue(ExprFormattedValue {
value: value.into(),
conversion: *conversion,
debug_text: debug_text.as_ref().map(Into::into),
format_spec: format_spec.as_ref().map(Into::into),
}),
ast::Expr::JoinedStr(ast::ExprJoinedStr {
Expand Down
9 changes: 9 additions & 0 deletions crates/ruff_python_ast/src/nodes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,7 @@ impl From<ExprCall> for Expr {
pub struct ExprFormattedValue {
pub range: TextRange,
pub value: Box<Expr>,
pub debug_text: Option<DebugText>,
pub conversion: ConversionFlag,
pub format_spec: Option<Box<Expr>>,
}
Expand Down Expand Up @@ -925,6 +926,14 @@ impl ConversionFlag {
}
}

#[derive(Clone, Debug, PartialEq)]
pub struct DebugText {
/// The text between the `{` and the expression node.
pub leading: String,
/// The text between the expression and the conversion, the format_spec, or the `}`, depending on what's present in the source
pub trailing: String,
}

/// See also [JoinedStr](https://docs.python.org/3/library/ast.html#ast.JoinedStr)
#[derive(Clone, Debug, PartialEq)]
pub struct ExprJoinedStr {
Expand Down
35 changes: 30 additions & 5 deletions crates/ruff_python_codegen/src/generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use std::ops::Deref;

use ruff_python_ast::{
self as ast, Alias, Arg, Arguments, BoolOp, CmpOp, Comprehension, Constant, ConversionFlag,
ExceptHandler, Expr, Identifier, MatchCase, Operator, Pattern, Stmt, Suite, TypeParam,
TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, WithItem,
DebugText, ExceptHandler, Expr, Identifier, MatchCase, Operator, Pattern, Stmt, Suite,
TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, WithItem,
};
use ruff_python_literal::escape::{AsciiEscape, Escape, UnicodeEscape};

Expand Down Expand Up @@ -1188,10 +1188,11 @@ impl<'a> Generator<'a> {
}
Expr::FormattedValue(ast::ExprFormattedValue {
value,
debug_text,
conversion,
format_spec,
range: _range,
}) => self.unparse_formatted(value, *conversion, format_spec.as_deref()),
}) => self.unparse_formatted(value, debug_text, *conversion, format_spec.as_deref()),
Expr::JoinedStr(ast::ExprJoinedStr {
values,
range: _range,
Expand Down Expand Up @@ -1381,7 +1382,13 @@ impl<'a> Generator<'a> {
}
}

fn unparse_formatted(&mut self, val: &Expr, conversion: ConversionFlag, spec: Option<&Expr>) {
fn unparse_formatted(
&mut self,
val: &Expr,
debug_text: &Option<DebugText>,
conversion: ConversionFlag,
spec: Option<&Expr>,
) {
let mut generator = Generator::new(self.indent, self.quote, self.line_ending);
generator.unparse_expr(val, precedence::FORMATTED_VALUE);
let brace = if generator.buffer.starts_with('{') {
Expand All @@ -1391,8 +1398,17 @@ impl<'a> Generator<'a> {
"{"
};
self.p(brace);

if let Some(debug_text) = debug_text {
self.buffer += debug_text.leading.as_str();
}

self.buffer += &generator.buffer;

if let Some(debug_text) = debug_text {
self.buffer += debug_text.trailing.as_str();
}

if !conversion.is_none() {
self.p("!");
#[allow(clippy::cast_possible_truncation)]
Expand Down Expand Up @@ -1424,10 +1440,11 @@ impl<'a> Generator<'a> {
}
Expr::FormattedValue(ast::ExprFormattedValue {
value,
debug_text,
conversion,
format_spec,
range: _range,
}) => self.unparse_formatted(value, *conversion, format_spec.as_deref()),
}) => self.unparse_formatted(value, debug_text, *conversion, format_spec.as_deref()),
_ => unreachable!(),
}
}
Expand Down Expand Up @@ -1740,6 +1757,14 @@ class Foo:
assert_eq!(round_trip(r#"f'abc{"def"}{1}'"#), r#"f"abc{'def'}{1}""#);
}

#[test]
fn self_documenting_f_string() {
assert_round_trip!(r#"f"{ chr(65) = }""#);
assert_round_trip!(r#"f"{ chr(65) = !s}""#);
assert_round_trip!(r#"f"{ chr(65) = !r}""#);
assert_round_trip!(r#"f"{ chr(65) = :#x}""#);
}

#[test]
fn indent() {
assert_eq!(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_python_parser/src/snapshots/ruff_python_parser__parser__tests__try.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ expression: parse_ast
keywords: [],
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down Expand Up @@ -199,6 +200,7 @@ expression: parse_ast
keywords: [],
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_python_parser/src/snapshots/ruff_python_parser__parser__tests__try_star.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ expression: parse_ast
keywords: [],
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down Expand Up @@ -249,6 +250,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down Expand Up @@ -336,6 +338,7 @@ expression: parse_ast
keywords: [],
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down Expand Up @@ -369,6 +372,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
2 changes: 2 additions & 0 deletions ...ython_parser/src/snapshots/ruff_python_parser__string__tests__fstring_constant_range.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand All @@ -52,6 +53,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
1 change: 1 addition & 0 deletions ...on_parser/src/snapshots/ruff_python_parser__string__tests__fstring_escaped_character.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
1 change: 1 addition & 0 deletions ...thon_parser/src/snapshots/ruff_python_parser__string__tests__fstring_escaped_newline.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
1 change: 1 addition & 0 deletions ...on_parser/src/snapshots/ruff_python_parser__string__tests__fstring_line_continuation.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load,
},
),
debug_text: None,
conversion: None,
format_spec: None,
},
Expand Down
26 changes: 7 additions & 19 deletions ...src/snapshots/ruff_python_parser__string__tests__fstring_parse_self_documenting_base.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,6 @@ source: crates/ruff_python_parser/src/string.rs
expression: parse_ast
---
[
Constant(
ExprConstant {
range: 2..9,
value: Str(
"user=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 2..9,
value: Str(
"",
),
kind: None,
},
),
FormattedValue(
ExprFormattedValue {
range: 2..9,
Expand All @@ -31,7 +13,13 @@ expression: parse_ast
ctx: Load,
},
),
conversion: Repr,
debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None,
},
),
Expand Down
52 changes: 14 additions & 38 deletions ...napshots/ruff_python_parser__string__tests__fstring_parse_self_documenting_base_more.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,6 @@ expression: parse_ast
kind: None,
},
),
Constant(
ExprConstant {
range: 6..13,
value: Str(
"user=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 6..13,
value: Str(
"",
),
kind: None,
},
),
FormattedValue(
ExprFormattedValue {
range: 6..13,
Expand All @@ -40,7 +22,13 @@ expression: parse_ast
ctx: Load,
},
),
conversion: Repr,
debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None,
},
),
Expand All @@ -53,24 +41,6 @@ expression: parse_ast
kind: None,
},
),
Constant(
ExprConstant {
range: 28..37,
value: Str(
"second=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 28..37,
value: Str(
"",
),
kind: None,
},
),
FormattedValue(
ExprFormattedValue {
range: 28..37,
Expand All @@ -81,7 +51,13 @@ expression: parse_ast
ctx: Load,
},
),
conversion: Repr,
debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None,
},
),
Expand Down
Loading

0 comments on commit 757dc4f

Please sign in to comment.