add DebugText for self-documenting f-strings (#6167)

This commit is contained in:
David Szotten 2023-08-01 06:55:03 +01:00 committed by GitHub
parent 44a8d1c644
commit ba990b676f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 148 additions and 155 deletions

View file

@ -5,6 +5,7 @@ use ruff_text_size::TextRange;
fn to_formatted_value_expr(inner: &Expr) -> Expr { fn to_formatted_value_expr(inner: &Expr) -> Expr {
let node = ast::ExprFormattedValue { let node = ast::ExprFormattedValue {
value: Box::new(inner.clone()), value: Box::new(inner.clone()),
debug_text: None,
conversion: ConversionFlag::None, conversion: ConversionFlag::None,
format_spec: None, format_spec: None,
range: TextRange::default(), range: TextRange::default(),

View file

@ -592,6 +592,7 @@ pub struct ExprCall<'a> {
#[derive(Debug, PartialEq, Eq, Hash)] #[derive(Debug, PartialEq, Eq, Hash)]
pub struct ExprFormattedValue<'a> { pub struct ExprFormattedValue<'a> {
value: Box<ComparableExpr<'a>>, value: Box<ComparableExpr<'a>>,
debug_text: Option<&'a ast::DebugText>,
conversion: ast::ConversionFlag, conversion: ast::ConversionFlag,
format_spec: Option<Box<ComparableExpr<'a>>>, format_spec: Option<Box<ComparableExpr<'a>>>,
} }
@ -849,11 +850,13 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
ast::Expr::FormattedValue(ast::ExprFormattedValue { ast::Expr::FormattedValue(ast::ExprFormattedValue {
value, value,
conversion, conversion,
debug_text,
format_spec, format_spec,
range: _range, range: _range,
}) => Self::FormattedValue(ExprFormattedValue { }) => Self::FormattedValue(ExprFormattedValue {
value: value.into(), value: value.into(),
conversion: *conversion, conversion: *conversion,
debug_text: debug_text.as_ref(),
format_spec: format_spec.as_ref().map(Into::into), format_spec: format_spec.as_ref().map(Into::into),
}), }),
ast::Expr::JoinedStr(ast::ExprJoinedStr { ast::Expr::JoinedStr(ast::ExprJoinedStr {

View file

@ -888,6 +888,7 @@ impl From<ExprCall> for Expr {
pub struct ExprFormattedValue { pub struct ExprFormattedValue {
pub range: TextRange, pub range: TextRange,
pub value: Box<Expr>, pub value: Box<Expr>,
pub debug_text: Option<DebugText>,
pub conversion: ConversionFlag, pub conversion: ConversionFlag,
pub format_spec: Option<Box<Expr>>, pub format_spec: Option<Box<Expr>>,
} }
@ -925,6 +926,14 @@ impl ConversionFlag {
} }
} }
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
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) /// See also [JoinedStr](https://docs.python.org/3/library/ast.html#ast.JoinedStr)
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
pub struct ExprJoinedStr { pub struct ExprJoinedStr {

View file

@ -5,8 +5,8 @@ use std::ops::Deref;
use ruff_python_ast::{ use ruff_python_ast::{
self as ast, Alias, Arg, Arguments, BoolOp, CmpOp, Comprehension, Constant, ConversionFlag, self as ast, Alias, Arg, Arguments, BoolOp, CmpOp, Comprehension, Constant, ConversionFlag,
ExceptHandler, Expr, Identifier, MatchCase, Operator, Pattern, Stmt, Suite, TypeParam, DebugText, ExceptHandler, Expr, Identifier, MatchCase, Operator, Pattern, Stmt, Suite,
TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, WithItem, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, WithItem,
}; };
use ruff_python_literal::escape::{AsciiEscape, Escape, UnicodeEscape}; use ruff_python_literal::escape::{AsciiEscape, Escape, UnicodeEscape};
@ -1189,10 +1189,16 @@ impl<'a> Generator<'a> {
} }
Expr::FormattedValue(ast::ExprFormattedValue { Expr::FormattedValue(ast::ExprFormattedValue {
value, value,
debug_text,
conversion, conversion,
format_spec, format_spec,
range: _range, range: _range,
}) => self.unparse_formatted(value, *conversion, format_spec.as_deref()), }) => self.unparse_formatted(
value,
debug_text.as_ref(),
*conversion,
format_spec.as_deref(),
),
Expr::JoinedStr(ast::ExprJoinedStr { Expr::JoinedStr(ast::ExprJoinedStr {
values, values,
range: _range, range: _range,
@ -1382,7 +1388,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); let mut generator = Generator::new(self.indent, self.quote, self.line_ending);
generator.unparse_expr(val, precedence::FORMATTED_VALUE); generator.unparse_expr(val, precedence::FORMATTED_VALUE);
let brace = if generator.buffer.starts_with('{') { let brace = if generator.buffer.starts_with('{') {
@ -1392,8 +1404,17 @@ impl<'a> Generator<'a> {
"{" "{"
}; };
self.p(brace); self.p(brace);
if let Some(debug_text) = debug_text {
self.buffer += debug_text.leading.as_str();
}
self.buffer += &generator.buffer; self.buffer += &generator.buffer;
if let Some(debug_text) = debug_text {
self.buffer += debug_text.trailing.as_str();
}
if !conversion.is_none() { if !conversion.is_none() {
self.p("!"); self.p("!");
#[allow(clippy::cast_possible_truncation)] #[allow(clippy::cast_possible_truncation)]
@ -1425,10 +1446,16 @@ impl<'a> Generator<'a> {
} }
Expr::FormattedValue(ast::ExprFormattedValue { Expr::FormattedValue(ast::ExprFormattedValue {
value, value,
debug_text,
conversion, conversion,
format_spec, format_spec,
range: _range, range: _range,
}) => self.unparse_formatted(value, *conversion, format_spec.as_deref()), }) => self.unparse_formatted(
value,
debug_text.as_ref(),
*conversion,
format_spec.as_deref(),
),
_ => unreachable!(), _ => unreachable!(),
} }
} }
@ -1755,6 +1782,15 @@ class Foo:
assert_eq!(round_trip(r#"f'abc{"def"}{1}'"#), r#"f"abc{'def'}{1}""#); 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}""#);
assert_round_trip!(r#"f"{a=!r:0.05f}""#);
}
#[test] #[test]
fn indent() { fn indent() {
assert_eq!( assert_eq!(

View file

@ -112,6 +112,7 @@ expression: parse_ast
keywords: [], keywords: [],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -199,6 +200,7 @@ expression: parse_ast
keywords: [], keywords: [],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -216,6 +216,7 @@ expression: parse_ast
keywords: [], keywords: [],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -249,6 +250,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -336,6 +338,7 @@ expression: parse_ast
keywords: [], keywords: [],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -369,6 +372,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -52,6 +53,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -3,24 +3,6 @@ source: crates/ruff_python_parser/src/string.rs
expression: parse_ast expression: parse_ast
--- ---
[ [
Constant(
ExprConstant {
range: 2..9,
value: Str(
"user=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 2..9,
value: Str(
"",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 2..9, range: 2..9,
@ -31,7 +13,13 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
conversion: Repr, debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None, format_spec: None,
}, },
), ),

View file

@ -12,24 +12,6 @@ expression: parse_ast
kind: None, kind: None,
}, },
), ),
Constant(
ExprConstant {
range: 6..13,
value: Str(
"user=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 6..13,
value: Str(
"",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 6..13, range: 6..13,
@ -40,7 +22,13 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
conversion: Repr, debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None, format_spec: None,
}, },
), ),
@ -53,24 +41,6 @@ expression: parse_ast
kind: None, kind: None,
}, },
), ),
Constant(
ExprConstant {
range: 28..37,
value: Str(
"second=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 28..37,
value: Str(
"",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 28..37, range: 28..37,
@ -81,7 +51,13 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
conversion: Repr, debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None,
format_spec: None, format_spec: None,
}, },
), ),

View file

@ -3,24 +3,6 @@ source: crates/ruff_python_parser/src/string.rs
expression: parse_ast expression: parse_ast
--- ---
[ [
Constant(
ExprConstant {
range: 2..13,
value: Str(
"user=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 2..13,
value: Str(
"",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 2..13, range: 2..13,
@ -31,6 +13,12 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: Some(
DebugText {
leading: "",
trailing: "=",
},
),
conversion: None, conversion: None,
format_spec: Some( format_spec: Some(
JoinedStr( JoinedStr(

View file

@ -29,6 +29,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -31,6 +31,7 @@ expression: parse_ast
kind: None, kind: None,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -13,6 +13,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },
@ -27,6 +28,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -34,6 +34,7 @@ expression: parse_ast
], ],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -13,6 +13,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: Some( format_spec: Some(
JoinedStr( JoinedStr(
@ -29,6 +30,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -34,6 +34,7 @@ expression: parse_ast
], ],
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -13,6 +13,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: Some( format_spec: Some(
JoinedStr( JoinedStr(

View file

@ -3,24 +3,6 @@ source: crates/ruff_python_parser/src/string.rs
expression: parse_ast expression: parse_ast
--- ---
[ [
Constant(
ExprConstant {
range: 2..9,
value: Str(
"x =",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 2..9,
value: Str(
"",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 2..9, range: 2..9,
@ -31,7 +13,13 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
conversion: Repr, debug_text: Some(
DebugText {
leading: "",
trailing: " =",
},
),
conversion: None,
format_spec: None, format_spec: None,
}, },
), ),

View file

@ -3,24 +3,6 @@ source: crates/ruff_python_parser/src/string.rs
expression: parse_ast expression: parse_ast
--- ---
[ [
Constant(
ExprConstant {
range: 2..9,
value: Str(
"x=",
),
kind: None,
},
),
Constant(
ExprConstant {
range: 2..9,
value: Str(
" ",
),
kind: None,
},
),
FormattedValue( FormattedValue(
ExprFormattedValue { ExprFormattedValue {
range: 2..9, range: 2..9,
@ -31,7 +13,13 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
conversion: Repr, debug_text: Some(
DebugText {
leading: "",
trailing: "= ",
},
),
conversion: None,
format_spec: None, format_spec: None,
}, },
), ),

View file

@ -12,6 +12,7 @@ expression: parse_ast
value: None, value: None,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -20,6 +20,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -20,6 +20,7 @@ expression: parse_ast
ctx: Load, ctx: Load,
}, },
), ),
debug_text: None,
conversion: None, conversion: None,
format_spec: None, format_spec: None,
}, },

View file

@ -182,11 +182,15 @@ impl<'a> StringParser<'a> {
}; };
let mut expression = String::new(); let mut expression = String::new();
// for self-documenting strings we also store the `=` and any trailing space inside
// expression (because we want to combine it with any trailing spaces before the equal
// sign). the expression_length is the length of the actual expression part that we pass to
// `parse_fstring_expr`
let mut expression_length = 0;
let mut spec = None; let mut spec = None;
let mut delimiters = Vec::new(); let mut delimiters = Vec::new();
let mut conversion = ConversionFlag::None; let mut conversion = ConversionFlag::None;
let mut self_documenting = false; let mut self_documenting = false;
let mut trailing_seq = String::new();
let start_location = self.get_pos(); let start_location = self.get_pos();
assert_eq!(self.next_char(), Some('{')); assert_eq!(self.next_char(), Some('{'));
@ -230,6 +234,8 @@ impl<'a> StringParser<'a> {
// match a python 3.8 self documenting expression // match a python 3.8 self documenting expression
// format '{' PYTHON_EXPRESSION '=' FORMAT_SPECIFIER? '}' // format '{' PYTHON_EXPRESSION '=' FORMAT_SPECIFIER? '}'
'=' if self.peek() != Some('=') && delimiters.is_empty() => { '=' if self.peek() != Some('=') && delimiters.is_empty() => {
expression_length = expression.len();
expression.push(ch);
self_documenting = true; self_documenting = true;
} }
@ -304,40 +310,28 @@ impl<'a> StringParser<'a> {
} }
let ret = if self_documenting { let ret = if self_documenting {
// TODO: range is wrong but `self_documenting` needs revisiting beyond let value =
// ranges: https://github.com/astral-sh/ruff/issues/5970 parse_fstring_expr(&expression[..expression_length], start_location)
vec![ .map_err(|e| {
Expr::from(ast::ExprConstant { FStringError::new(
value: Constant::Str(expression.clone() + "="), InvalidExpression(Box::new(e.error)),
kind: None, start_location,
range: self.range(start_location), )
})?;
let leading =
&expression[..usize::from(value.range().start() - start_location) - 1];
let trailing =
&expression[usize::from(value.range().end() - start_location) - 1..];
vec![Expr::from(ast::ExprFormattedValue {
value: Box::new(value),
debug_text: Some(ast::DebugText {
leading: leading.to_string(),
trailing: trailing.to_string(),
}), }),
Expr::from(ast::ExprConstant { conversion,
value: trailing_seq.into(), format_spec: spec,
kind: None, range: self.range(start_location),
range: self.range(start_location), })]
}),
Expr::from(ast::ExprFormattedValue {
value: Box::new(
parse_fstring_expr(&expression, start_location).map_err(
|e| {
FStringError::new(
InvalidExpression(Box::new(e.error)),
start_location,
)
},
)?,
),
conversion: if conversion == ConversionFlag::None && spec.is_none()
{
ConversionFlag::Repr
} else {
conversion
},
format_spec: spec,
range: self.range(start_location),
}),
]
} else { } else {
vec![Expr::from(ast::ExprFormattedValue { vec![Expr::from(ast::ExprFormattedValue {
value: Box::new( value: Box::new(
@ -348,6 +342,7 @@ impl<'a> StringParser<'a> {
) )
})?, })?,
), ),
debug_text: None,
conversion, conversion,
format_spec: spec, format_spec: spec,
range: self.range(start_location), range: self.range(start_location),
@ -369,9 +364,7 @@ impl<'a> StringParser<'a> {
} }
} }
} }
' ' if self_documenting => { ' ' if self_documenting => expression.push(ch),
trailing_seq.push(ch);
}
'\\' => return Err(FStringError::new(UnterminatedString, self.get_pos()).into()), '\\' => return Err(FStringError::new(UnterminatedString, self.get_pos()).into()),
_ => { _ => {
if self_documenting { if self_documenting {