From 41f0aad7b3e163436d870dd936e0cdd05cf4b80b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 8 Sep 2023 11:48:57 +0200 Subject: [PATCH] Add FString support to binary like formatting ## Summary This is the last part of the string - binary like formatting. It adds support for handling fstrings the same as "regular" strings. ## Test Plan I added a test for both binary and comparison. Small improvements across several projects **This PR** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | **transformers** | 0.99929 | 2587 | 454 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99978 | 3496 | 2173 | | **warehouse** | 0.99825 | 648 | 22 | | **zulip** | 0.99950 | 1437 | 27 | **Base** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | transformers | 0.99928 | 2587 | 454 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99978 | 3496 | 2173 | | warehouse | 0.99824 | 648 | 22 | | zulip | 0.99948 | 1437 | 28 | --- .../ruff/expression/binary_implicit_string.py | 9 +++ .../test/fixtures/ruff/expression/compare.py | 9 +++ .../src/expression/binary_like.rs | 61 +++++++------------ .../src/expression/string.rs | 34 +++++++++-- ...expression__binary_implicit_string.py.snap | 15 +++++ .../format@expression__compare.py.snap | 15 +++++ 6 files changed, 100 insertions(+), 43 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py index 29e1ea4521..993bdca659 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_implicit_string.py @@ -84,6 +84,15 @@ self._assert_skipping( + x ) +( + b + c + d + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + f"bbbbbb{z}bbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" + % aaaaaaaaaaaa + + x +) + ( b < c > d < "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/compare.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/compare.py index e266b6b37a..99dd8f4b14 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/compare.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/compare.py @@ -131,6 +131,15 @@ assert ( in caplog.messages ) +( + b < c > d < + f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" + % aaaaaaaaaaaa + > x +) + c = (a > # test leading binary comment "a" "b" * b diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index e3b58a59ae..09cfbc2604 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/crates/ruff_python_formatter/src/expression/binary_like.rs @@ -5,18 +5,16 @@ use smallvec::SmallVec; use ruff_formatter::write; use ruff_python_ast::{ - BytesConstant, Constant, Expr, ExprAttribute, ExprBinOp, ExprCompare, ExprConstant, - ExprUnaryOp, StringConstant, UnaryOp, + Constant, Expr, ExprAttribute, ExprBinOp, ExprCompare, ExprConstant, ExprUnaryOp, UnaryOp, }; use crate::comments::{leading_comments, trailing_comments, Comments, SourceComment}; -use crate::expression::expr_constant::ExprConstantLayout; use crate::expression::parentheses::{ in_parentheses_only_group, in_parentheses_only_soft_line_break, in_parentheses_only_soft_line_break_or_space, is_expression_parenthesized, write_in_parentheses_only_group_end_tag, write_in_parentheses_only_group_start_tag, }; -use crate::expression::string::StringLayout; +use crate::expression::string::{AnyString, FormatString, StringLayout}; use crate::expression::OperatorPrecedence; use crate::prelude::*; @@ -197,29 +195,12 @@ impl Format> for BinaryLike<'_> { let mut string_operands = flat_binary .operands() .filter_map(|(index, operand)| { - if let Expr::Constant( - constant @ ExprConstant { - value: - Constant::Str(StringConstant { - implicit_concatenated: true, - .. - }) - | Constant::Bytes(BytesConstant { - implicit_concatenated: true, - .. - }), - .. - }, - ) = operand.expression() - { - if is_expression_parenthesized(constant.into(), source) { - None - } else { - Some((index, constant, operand)) - } - } else { - None - } + AnyString::from_expression(operand.expression()) + .filter(|string| { + string.is_implicit_concatenated() + && !is_expression_parenthesized(string.into(), source) + }) + .map(|string| (index, string, operand)) }) .peekable(); @@ -296,11 +277,11 @@ impl Format> for BinaryLike<'_> { f, [ operand.leading_binary_comments().map(leading_comments), - string_constant - .format() - .with_options(ExprConstantLayout::String( - StringLayout::ImplicitConcatenatedStringInBinaryLike, - )), + leading_comments(comments.leading(&string_constant)), + FormatString::new(&string_constant).with_layout( + StringLayout::ImplicitConcatenatedStringInBinaryLike, + ), + trailing_comments(comments.trailing(&string_constant)), operand.trailing_binary_comments().map(trailing_comments), line_suffix_boundary(), ] @@ -311,12 +292,16 @@ impl Format> for BinaryLike<'_> { // "a" "b" + c // ^^^^^^^-- format the first operand of a binary expression // ``` - string_constant - .format() - .with_options(ExprConstantLayout::String( - StringLayout::ImplicitConcatenatedStringInBinaryLike, - )) - .fmt(f)?; + write!( + f, + [ + leading_comments(comments.leading(&string_constant)), + FormatString::new(&string_constant).with_layout( + StringLayout::ImplicitConcatenatedStringInBinaryLike + ), + trailing_comments(comments.trailing(&string_constant)), + ] + )?; } // Write the right operator and start the group for the right side (if any) diff --git a/crates/ruff_python_formatter/src/expression/string.rs b/crates/ruff_python_formatter/src/expression/string.rs index f1ac10751b..063eb350d5 100644 --- a/crates/ruff_python_formatter/src/expression/string.rs +++ b/crates/ruff_python_formatter/src/expression/string.rs @@ -4,7 +4,7 @@ use bitflags::bitflags; use ruff_formatter::{format_args, write, FormatError, FormatOptions, TabWidth}; use ruff_python_ast::node::AnyNodeRef; -use ruff_python_ast::{self as ast, ExprConstant, ExprFString}; +use ruff_python_ast::{self as ast, Constant, ExprConstant, ExprFString, ExpressionRef}; use ruff_python_parser::lexer::{lex_starts_at, LexicalError, LexicalErrorType}; use ruff_python_parser::{Mode, Tok}; use ruff_source_file::Locator; @@ -24,12 +24,26 @@ enum Quoting { Preserve, } +#[derive(Clone, Debug)] pub(super) enum AnyString<'a> { Constant(&'a ExprConstant), FString(&'a ExprFString), } impl<'a> AnyString<'a> { + pub(crate) fn from_expression(expression: &'a Expr) -> Option> { + match expression { + Expr::Constant( + constant @ ExprConstant { + value: Constant::Str(_) | Constant::Bytes(_), + .. + }, + ) => Some(AnyString::Constant(constant)), + Expr::FString(fstring) => Some(AnyString::FString(fstring)), + _ => None, + } + } + fn quoting(&self, locator: &Locator) -> Quoting { match self { Self::Constant(_) => Quoting::CanChange, @@ -50,7 +64,7 @@ impl<'a> AnyString<'a> { } /// Returns `true` if the string is implicitly concatenated. - fn implicit_concatenated(&self) -> bool { + pub(super) fn is_implicit_concatenated(&self) -> bool { match self { Self::Constant(ExprConstant { value, .. }) => value.is_implicit_concatenated(), Self::FString(ExprFString { @@ -79,6 +93,15 @@ impl<'a> From<&AnyString<'a>> for AnyNodeRef<'a> { } } +impl<'a> From<&AnyString<'a>> for ExpressionRef<'a> { + fn from(value: &AnyString<'a>) -> Self { + match value { + AnyString::Constant(expr) => ExpressionRef::Constant(expr), + AnyString::FString(expr) => ExpressionRef::FString(expr), + } + } +} + pub(super) struct FormatString<'a> { string: &'a AnyString<'a>, layout: StringLayout, @@ -97,7 +120,7 @@ pub enum StringLayout { } impl<'a> FormatString<'a> { - pub(super) fn new(string: &'a AnyString) -> Self { + pub(super) fn new(string: &'a AnyString<'a>) -> Self { if let AnyString::Constant(constant) = string { debug_assert!(constant.value.is_str() || constant.value.is_bytes()); } @@ -117,7 +140,7 @@ impl<'a> Format> for FormatString<'a> { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { match self.layout { StringLayout::Default => { - if self.string.implicit_concatenated() { + if self.string.is_implicit_concatenated() { in_parentheses_only_group(&FormatStringContinuation::new(self.string)).fmt(f) } else { FormatStringPart::new( @@ -972,9 +995,10 @@ fn format_docstring_line( #[cfg(test)] mod tests { - use crate::expression::string::count_indentation_like_black; use ruff_formatter::TabWidth; + use crate::expression::string::count_indentation_like_black; + #[test] fn test_indentation_like_black() { let tab_width = TabWidth::try_from(8).unwrap(); diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap index 7ce094c667..50fb1a7285 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap @@ -90,6 +90,15 @@ self._assert_skipping( + x ) +( + b + c + d + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + f"bbbbbb{z}bbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" + % aaaaaaaaaaaa + + x +) + ( b < c > d < "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" @@ -266,6 +275,12 @@ self._assert_skipping( "cccccccccccccccccccccccccc" % aaaaaaaaaaaa + x ) +( + b + c + d + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + f"bbbbbb{z}bbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" % aaaaaaaaaaaa + x +) + ( b < c > d < "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__compare.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__compare.py.snap index 817c44b908..288046cbef 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__compare.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__compare.py.snap @@ -137,6 +137,15 @@ assert ( in caplog.messages ) +( + b < c > d < + f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" + % aaaaaaaaaaaa + > x +) + c = (a > # test leading binary comment "a" "b" * b @@ -356,6 +365,12 @@ assert ( "be silently ignored by the index" in caplog.messages ) +( + b < c > d < f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + "cccccccccccccccccccccccccc" % aaaaaaaaaaaa > x +) + c = ( a > # test leading binary comment