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

Preserve indent around multiline strings #9637

Merged
merged 1 commit into from Jan 26, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 10 additions & 31 deletions crates/ruff_python_formatter/src/expression/mod.rs
Expand Up @@ -16,14 +16,11 @@ use crate::comments::{leading_comments, trailing_comments, LeadingDanglingTraili
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::expr_generator_exp::is_generator_parenthesized;
use crate::expression::parentheses::{
is_expression_parenthesized, optional_parentheses, parenthesized, HuggingStyle,
NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize,
is_expression_parenthesized, optional_parentheses, parenthesized, NeedsParentheses,
OptionalParentheses, Parentheses, Parenthesize,
};
use crate::prelude::*;
use crate::preview::{
is_hug_parens_with_braces_and_square_brackets_enabled, is_multiline_string_handling_enabled,
};
use crate::string::AnyString;
use crate::preview::is_hug_parens_with_braces_and_square_brackets_enabled;

mod binary_like;
pub(crate) mod expr_attribute;
Expand Down Expand Up @@ -446,7 +443,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()).is_none())
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f)
}

Expand Down Expand Up @@ -1112,26 +1109,18 @@ pub(crate) fn has_own_parentheses(
/// ]
/// )
/// ```
pub(crate) fn is_expression_huggable(
expr: &Expr,
context: &PyFormatContext,
) -> Option<HuggingStyle> {
pub(crate) fn is_expression_huggable(expr: &Expr, context: &PyFormatContext) -> bool {
match expr {
Expr::Tuple(_)
| Expr::List(_)
| Expr::Set(_)
| Expr::Dict(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_) => is_hug_parens_with_braces_and_square_brackets_enabled(context)
.then_some(HuggingStyle::Always),
| Expr::DictComp(_) => is_hug_parens_with_braces_and_square_brackets_enabled(context),

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(_)
| Expr::BinOp(_)
Expand All @@ -1152,20 +1141,10 @@ pub(crate) fn is_expression_huggable(
| Expr::NumberLiteral(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| 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
| Expr::StringLiteral(_)
| Expr::BytesLiteral(_)
| Expr::FString(_)
| Expr::EllipsisLiteral(_) => false,
}
}

Expand Down
51 changes: 7 additions & 44 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Expand Up @@ -126,7 +126,7 @@ where
FormatParenthesized {
left,
comments: &[],
hug: None,
hug: false,
content: Argument::new(content),
right,
}
Expand All @@ -135,7 +135,7 @@ where
pub(crate) struct FormatParenthesized<'content, 'ast> {
left: &'static str,
comments: &'content [SourceComment],
hug: Option<HuggingStyle>,
hug: bool,
content: Argument<'content, PyFormatContext<'ast>>,
right: &'static str,
}
Expand All @@ -158,10 +158,7 @@ impl<'content, 'ast> FormatParenthesized<'content, 'ast> {
}

/// Whether to indent the content within the parentheses.
pub(crate) fn with_hugging(
self,
hug: Option<HuggingStyle>,
) -> FormatParenthesized<'content, 'ast> {
pub(crate) fn with_hugging(self, hug: bool) -> FormatParenthesized<'content, 'ast> {
FormatParenthesized { hug, ..self }
}
}
Expand All @@ -173,30 +170,10 @@ impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
let indented = format_with(|f| {
let content = Arguments::from(&self.content);
if self.comments.is_empty() {
match self.hug {
None => group(&soft_block_indent(&content)).fmt(f),
Some(HuggingStyle::Always) => content.fmt(f),
Some(HuggingStyle::IfFirstLineFits) => {
// It's not immediately obvious how the below IR works to only indent the content if the first line exceeds the configured line width.
// The trick is the first group that doesn't wrap `self.content`.
// * The group doesn't wrap `self.content` because we need to assume that `self.content`
// contains a hard line break and hard-line-breaks always expand the enclosing group.
// * The printer decides that a group fits if its content (in this case a `soft_line_break` that has a width of 0 and is guaranteed to fit)
// and the content coming after the group in expanded mode (`self.content`) fits on the line.
// The content coming after fits if the content up to the first soft or hard line break (or the end of the document) fits.
//
// This happens to be right what we want. The first group should add an indent and a soft line break if the content of `self.content`
// up to the first line break exceeds the configured line length, but not otherwise.
let indented = f.group_id("indented_content");
write!(
f,
[
group(&indent(&soft_line_break())).with_group_id(Some(indented)),
indent_if_group_breaks(&content, indented),
if_group_breaks(&soft_line_break()).with_group_id(Some(indented))
]
)
}
Copy link
Member

Choose a reason for hiding this comment

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

So this got much simpler because we no longer check if indenting would allow us to avoid line-length violations?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Breaking only if the first line doesn't fit required a more complicated IR. Now it's simpler, because we always or never indent.

if self.hug {
content.fmt(f)
} else {
group(&soft_block_indent(&content)).fmt(f)
}
} else {
group(&format_args![
Expand Down Expand Up @@ -228,20 +205,6 @@ impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
}
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub(crate) enum HuggingStyle {
/// Always hug the content (never indent).
Always,

/// Hug the content if the content up to the first line break fits into the configured line length. Otherwise indent the content.
///
/// This is different from [`HuggingStyle::Always`] in that it doesn't indent if the content contains a hard line break, and the content up to that hard line break fits into the configured line length.
///
/// This style is used for formatting multiline strings that, by definition, always break. The idea is to
/// only hug a multiline string if its content up to the first line breaks exceeds the configured line length.
IfFirstLineFits,
}

/// Wraps an expression in parentheses only if it still does not fit after expanding all expressions that start or end with
/// a parentheses (`()`, `[]`, `{}`).
pub(crate) fn optional_parentheses<'content, 'ast, Content>(
Expand Down
68 changes: 55 additions & 13 deletions crates/ruff_python_formatter/src/other/arguments.rs
@@ -1,16 +1,16 @@
use ruff_formatter::{write, FormatContext};
use ruff_python_ast::{ArgOrKeyword, Arguments, Expr};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_python_trivia::{PythonWhitespace, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use crate::comments::SourceComment;
use crate::expression::expr_generator_exp::GeneratorExpParentheses;
use crate::expression::is_expression_huggable;
use crate::expression::parentheses::{
empty_parenthesized, parenthesized, HuggingStyle, Parentheses,
};
use crate::expression::parentheses::{empty_parenthesized, parenthesized, Parentheses};
use crate::other::commas;
use crate::prelude::*;
use crate::preview::is_multiline_string_handling_enabled;
use crate::string::AnyString;

#[derive(Default)]
pub struct FormatArguments;
Expand Down Expand Up @@ -178,33 +178,75 @@ fn is_single_argument_parenthesized(argument: &Expr, call_end: TextSize, source:
///
/// Hugging should only be applied to single-argument collections, like lists, or starred versions
/// of those collections.
fn is_arguments_huggable(item: &Arguments, context: &PyFormatContext) -> Option<HuggingStyle> {
fn is_arguments_huggable(arguments: &Arguments, context: &PyFormatContext) -> bool {
// Find the lone argument or `**kwargs` keyword.
let arg = match (item.args.as_slice(), item.keywords.as_slice()) {
let arg = match (arguments.args.as_slice(), arguments.keywords.as_slice()) {
([arg], []) => arg,
([], [keyword]) if keyword.arg.is_none() && !context.comments().has(keyword) => {
&keyword.value
}
_ => return None,
_ => return false,
};

// If the expression itself isn't huggable, then we can't hug it.
let hugging_style = is_expression_huggable(arg, context)?;
if !(is_expression_huggable(arg, context)
|| AnyString::from_expression(arg)
.is_some_and(|string| is_huggable_string_argument(string, arguments, context)))
{
return false;
}

// If the expression has leading or trailing comments, then we can't hug it.
let comments = context.comments().leading_dangling_trailing(arg);
if comments.has_leading() || comments.has_trailing() {
return None;
return false;
}

let options = context.options();

// If the expression has a trailing comma, then we can't hug it.
if options.magic_trailing_comma().is_respect()
&& commas::has_magic_trailing_comma(TextRange::new(arg.end(), item.end()), options, context)
&& commas::has_magic_trailing_comma(
TextRange::new(arg.end(), arguments.end()),
options,
context,
)
{
return None;
return false;
}

true
}

/// Returns `true` if `string` is a multiline string that is not implicitly concatenated and there's no
/// newline between the opening parentheses of arguments and the quotes of the string:
///
/// ```python
/// # Hug this string
/// call("""test
/// multiline""")
///
/// # Don't hug because there's a newline between the opening parentheses and the quotes:
/// call(
/// """"
/// test
/// """"
/// )
/// ```
fn is_huggable_string_argument(
string: AnyString,
arguments: &Arguments,
context: &PyFormatContext,
) -> bool {
if !is_multiline_string_handling_enabled(context) {
return false;
}

if string.is_implicit_concatenated() || !string.is_multiline(context.source()) {
return false;
}

Some(hugging_style)
let between_parens_range = TextRange::new(arguments.start() + '('.text_len(), string.start());
let between_parens = &context.source()[between_parens_range];
!between_parens.trim_whitespace_end().ends_with(['\n', '\r'])
}
Expand Up @@ -301,7 +301,19 @@ this_will_also_become_one_line = ( # comment
# Another use case
data = yaml.load("""\
a: 1
@@ -85,11 +114,13 @@
@@ -77,19 +106,23 @@
b: 2
""",
)
-data = yaml.load("""\
+data = yaml.load(
+ """\
a: 1
b: 2
-""")
+"""
+)

MULTILINE = """
foo
""".replace("\n", "")
Expand All @@ -316,7 +328,7 @@ this_will_also_become_one_line = ( # comment
parser.usage += """
Custom extra help summary.

@@ -156,16 +187,24 @@
@@ -156,16 +189,24 @@
10 LOAD_CONST 0 (None)
12 RETURN_VALUE
""" % (_C.__init__.__code__.co_firstlineno + 1,)
Expand Down Expand Up @@ -347,7 +359,7 @@ this_will_also_become_one_line = ( # comment
[
"""cow
moos""",
@@ -198,7 +237,7 @@
@@ -198,7 +239,7 @@
`--global-option` is reserved to flags like `--verbose` or `--quiet`.
"""

Expand All @@ -356,7 +368,7 @@ this_will_also_become_one_line = ( # comment

this_will_stay_on_three_lines = (
"a" # comment
@@ -206,4 +245,6 @@
@@ -206,4 +247,6 @@
"c"
)

Expand Down Expand Up @@ -477,10 +489,12 @@ a: 1
b: 2
""",
)
data = yaml.load("""\
data = yaml.load(
"""\
a: 1
b: 2
""")
"""
)

MULTILINE = """
foo
Expand Down