Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hug multiline-strings preview style #9243

Merged
merged 1 commit into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions crates/ruff_formatter/src/printer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,11 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}

fn fits_text(&mut self, text: Text, args: PrintElementArgs) -> Fits {
fn exceeds_width(fits: &FitsMeasurer, args: PrintElementArgs) -> bool {
fits.state.line_width > fits.options().line_width.into()
&& !args.measure_mode().allows_text_overflow()
}

let indent = std::mem::take(&mut self.state.pending_indent);
self.state.line_width +=
u32::from(indent.level()) * self.options().indent_width() + u32::from(indent.align());
Expand All @@ -1493,7 +1498,13 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
return Fits::No;
}
match args.measure_mode() {
MeasureMode::FirstLine => return Fits::Yes,
MeasureMode::FirstLine => {
return if exceeds_width(self, args) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix in the printer. So far, it always returned Fits::Yes if a text contained a newline character regardless if the text before the newline exceeded the line width. This was incorrect.

Fits::No
} else {
Fits::Yes
};
}
MeasureMode::AllLines
| MeasureMode::AllLinesAllowTextOverflow => {
self.state.line_width = 0;
Expand All @@ -1511,9 +1522,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}
}

if self.state.line_width > self.options().line_width.into()
&& !args.measure_mode().allows_text_overflow()
{
if exceeds_width(self, args) {
return Fits::No;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# This file documents the deviations for formatting multiline strings with black.

# Black hugs the parentheses for `%` usages -> convert to fstring.
# Can get unreadable if the arguments split
# This could be solved by using `best_fitting` to try to format the arguments on a single
# line. Let's consider adding this later.
# ```python
# call(
# 3,
# "dogsay",
# textwrap.dedent(
# """dove
# coo""" % "cowabunga",
# more,
# and_more,
# "aaaaaaa",
# "bbbbbbbbb",
# "cccccccc",
# ),
# )
# ```
call(3, "dogsay", textwrap.dedent("""dove
coo""" % "cowabunga"))

# Black applies the hugging recursively. We don't (consistent with the hugging style).
path.write_text(textwrap.dedent("""\
A triple-quoted string
actually leveraging the textwrap.dedent functionality
that ends in a trailing newline,
representing e.g. file contents.
"""))



# Black avoids parenthesizing the following lambda. We could potentially support
# this by changing `Lambda::needs_parentheses` to return `BestFit` but it causes
# issues when the lambda has comments.
# Let's keep this as a known deviation for now.
generated_readme = lambda project_name: """
{}

<Add content here!>
""".strip().format(project_name)
8 changes: 4 additions & 4 deletions crates/ruff_python_formatter/src/expression/binary_like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,12 +394,12 @@ impl Format<PyFormatContext<'_>> for BinaryLike<'_> {
f,
[
operand.leading_binary_comments().map(leading_comments),
leading_comments(comments.leading(&string_constant)),
leading_comments(comments.leading(string_constant)),
// Call `FormatStringContinuation` directly to avoid formatting
// the implicitly concatenated string with the enclosing group
// because the group is added by the binary like formatting.
FormatStringContinuation::new(&string_constant),
trailing_comments(comments.trailing(&string_constant)),
trailing_comments(comments.trailing(string_constant)),
operand.trailing_binary_comments().map(trailing_comments),
line_suffix_boundary(),
]
Expand All @@ -413,12 +413,12 @@ impl Format<PyFormatContext<'_>> for BinaryLike<'_> {
write!(
f,
[
leading_comments(comments.leading(&string_constant)),
leading_comments(comments.leading(string_constant)),
// Call `FormatStringContinuation` directly to avoid formatting
// the implicitly concatenated string with the enclosing group
// because the group is added by the binary like formatting.
FormatStringContinuation::new(&string_constant),
trailing_comments(comments.trailing(&string_constant)),
trailing_comments(comments.trailing(string_constant)),
]
)?;
}
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_python_formatter/src/expression/expr_bin_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use ruff_python_ast::ExprBinOp;

use crate::comments::SourceComment;
use crate::expression::binary_like::BinaryLike;
use crate::expression::expr_string_literal::is_multiline_string;
use crate::expression::has_parentheses;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::string::AnyString;

#[derive(Default)]
pub struct FormatExprBinOp;
Expand Down Expand Up @@ -35,13 +35,13 @@ impl NeedsParentheses for ExprBinOp {
) -> OptionalParentheses {
if parent.is_expr_await() {
OptionalParentheses::Always
} else if let Some(literal_expr) = self.left.as_literal_expr() {
} else if let Some(string) = AnyString::from_expression(&self.left) {
// Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses
if !literal_expr.is_implicit_concatenated()
&& is_multiline_string(literal_expr.into(), context.source())
if !string.is_implicit_concatenated()
&& string.is_multiline(context.source())
&& has_parentheses(&self.right, context).is_some()
&& !context.comments().has_dangling(self)
&& !context.comments().has(literal_expr)
&& !context.comments().has(string)
&& !context.comments().has(self.right.as_ref())
{
OptionalParentheses::Never
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::ExprBytesLiteral;

use crate::comments::SourceComment;
use crate::expression::expr_string_literal::is_multiline_string;
use crate::expression::parentheses::{
in_parentheses_only_group, NeedsParentheses, OptionalParentheses,
};
Expand Down Expand Up @@ -41,7 +40,7 @@ impl NeedsParentheses for ExprBytesLiteral {
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
} else if is_multiline_string(self.into(), context.source()) {
} else if AnyString::Bytes(self).is_multiline(context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_python_formatter/src/expression/expr_compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use ruff_python_ast::{CmpOp, ExprCompare};

use crate::comments::SourceComment;
use crate::expression::binary_like::BinaryLike;
use crate::expression::expr_string_literal::is_multiline_string;
use crate::expression::has_parentheses;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::string::AnyString;

#[derive(Default)]
pub struct FormatExprCompare;
Expand Down Expand Up @@ -37,11 +37,11 @@ impl NeedsParentheses for ExprCompare {
) -> OptionalParentheses {
if parent.is_expr_await() {
OptionalParentheses::Always
} else if let Some(literal_expr) = self.left.as_literal_expr() {
} else if let Some(string) = AnyString::from_expression(&self.left) {
// Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses
if !literal_expr.is_implicit_concatenated()
&& is_multiline_string(literal_expr.into(), context.source())
&& !context.comments().has(literal_expr)
if !string.is_implicit_concatenated()
&& string.is_multiline(context.source())
&& !context.comments().has(string)
&& self.comparators.first().is_some_and(|right| {
has_parentheses(right, context).is_some() && !context.comments().has(right)
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use memchr::memchr2;

use ruff_python_ast::{AnyNodeRef, ExprFString};
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -50,10 +48,10 @@ impl NeedsParentheses for ExprFString {
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
} else if memchr2(b'\n', b'\r', context.source()[self.range].as_bytes()).is_none() {
OptionalParentheses::BestFit
} else {
} else if AnyString::FString(self).is_multiline(context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use ruff_formatter::FormatRuleWithOptions;
use ruff_python_ast::{AnyNodeRef, ExprStringLiteral};
use ruff_text_size::{Ranged, TextLen, TextRange};

use crate::comments::SourceComment;
use crate::expression::parentheses::{
in_parentheses_only_group, NeedsParentheses, OptionalParentheses,
};
use crate::other::string_literal::{FormatStringLiteral, StringLiteralKind};
use crate::prelude::*;
use crate::string::{AnyString, FormatStringContinuation, StringPrefix, StringQuotes};
use crate::string::{AnyString, FormatStringContinuation};

#[derive(Default)]
pub struct FormatExprStringLiteral {
Expand Down Expand Up @@ -80,24 +79,10 @@ impl NeedsParentheses for ExprStringLiteral {
) -> OptionalParentheses {
if self.value.is_implicit_concatenated() {
OptionalParentheses::Multiline
} else if is_multiline_string(self.into(), context.source()) {
} else if AnyString::String(self).is_multiline(context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
}
}
}

pub(super) fn is_multiline_string(expr: AnyNodeRef, source: &str) -> bool {
if expr.is_expr_string_literal() || expr.is_expr_bytes_literal() {
let contents = &source[expr.range()];
let prefix = StringPrefix::parse(contents);
let quotes =
StringQuotes::parse(&contents[TextRange::new(prefix.text_len(), contents.text_len())]);

quotes.is_some_and(StringQuotes::is_triple)
&& memchr::memchr2(b'\n', b'\r', contents.as_bytes()).is_some()
} else {
false
}
}
62 changes: 35 additions & 27 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@ use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::expr_generator_exp::is_generator_parenthesized;
use crate::expression::expr_tuple::is_tuple_parenthesized;
use crate::expression::parentheses::{
is_expression_parenthesized, optional_parentheses, parenthesized, NeedsParentheses,
OptionalParentheses, Parentheses, Parenthesize,
is_expression_parenthesized, optional_parentheses, parenthesized, HuggingStyle,
NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize,
};
use crate::prelude::*;
use crate::preview::is_hug_parens_with_braces_and_square_brackets_enabled;
use crate::preview::{
is_hug_parens_with_braces_and_square_brackets_enabled, is_multiline_string_handling_enabled,
};
use crate::string::AnyString;

mod binary_like;
pub(crate) mod expr_attribute;
Expand Down Expand Up @@ -126,7 +129,7 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
let node_comments = comments.leading_dangling_trailing(expression);
if !node_comments.has_leading() && !node_comments.has_trailing() {
parenthesized("(", &format_expr, ")")
.with_indent(!is_expression_huggable(expression, f.context()))
.with_hugging(is_expression_huggable(expression, f.context()))
.fmt(f)
} else {
format_with_parentheses_comments(expression, &node_comments, f)
Expand Down Expand Up @@ -444,7 +447,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
OptionalParentheses::Never => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.with_indent(!is_expression_huggable(expression, f.context()))
.with_indent(is_expression_huggable(expression, f.context()).is_none())
.fmt(f)
}

Expand Down Expand Up @@ -1084,7 +1087,7 @@ pub(crate) fn has_own_parentheses(
}

/// Returns `true` if the expression can hug directly to enclosing parentheses, as in Black's
/// `hug_parens_with_braces_and_square_brackets` preview style behavior.
/// `hug_parens_with_braces_and_square_brackets` or `multiline_string_handling` preview styles behavior.
///
/// For example, in preview style, given:
/// ```python
Expand All @@ -1110,30 +1113,25 @@ pub(crate) fn has_own_parentheses(
/// ]
/// )
/// ```
pub(crate) fn is_expression_huggable(expr: &Expr, context: &PyFormatContext) -> bool {
if !is_hug_parens_with_braces_and_square_brackets_enabled(context) {
return false;
}

pub(crate) fn is_expression_huggable(
expr: &Expr,
context: &PyFormatContext,
) -> Option<HuggingStyle> {
match expr {
Expr::Tuple(_)
| Expr::List(_)
| Expr::Set(_)
| Expr::Dict(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_) => true,

Expr::Starred(ast::ExprStarred { value, .. }) => matches!(
value.as_ref(),
Expr::Tuple(_)
| Expr::List(_)
| Expr::Set(_)
| Expr::Dict(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
),
| Expr::DictComp(_) => is_hug_parens_with_braces_and_square_brackets_enabled(context)
.then_some(HuggingStyle::Always),

Expr::Starred(ast::ExprStarred { value, .. }) => is_expression_huggable(value, context),

Expr::StringLiteral(string) => is_huggable_string(AnyString::String(string), context),
Expr::BytesLiteral(bytes) => is_huggable_string(AnyString::Bytes(bytes), context),
Expr::FString(fstring) => is_huggable_string(AnyString::FString(fstring), context),

Expr::BoolOp(_)
| Expr::NamedExpr(_)
Expand All @@ -1147,18 +1145,28 @@ pub(crate) fn is_expression_huggable(expr: &Expr, context: &PyFormatContext) ->
| Expr::YieldFrom(_)
| Expr::Compare(_)
| Expr::Call(_)
| Expr::FString(_)
| Expr::Attribute(_)
| Expr::Subscript(_)
| Expr::Name(_)
| Expr::Slice(_)
| Expr::IpyEscapeCommand(_)
| Expr::StringLiteral(_)
| Expr::BytesLiteral(_)
| Expr::NumberLiteral(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| Expr::EllipsisLiteral(_) => false,
| Expr::EllipsisLiteral(_) => None,
}
}

/// Returns `true` if `string` is a multiline string that is not implicitly concatenated.
fn is_huggable_string(string: AnyString, context: &PyFormatContext) -> Option<HuggingStyle> {
if !is_multiline_string_handling_enabled(context) {
return None;
}

if !string.is_implicit_concatenated() && string.is_multiline(context.source()) {
Some(HuggingStyle::IfFirstLineFits)
} else {
None
}
}

Expand Down
Loading
Loading