Skip to content

Commit

Permalink
Use LineSuffix.reserved_width in ruff_python_formatter
Browse files Browse the repository at this point in the history
  • Loading branch information
cnpryer committed Aug 27, 2023
1 parent 15b73bd commit 4e3783f
Show file tree
Hide file tree
Showing 17 changed files with 459 additions and 50 deletions.
59 changes: 47 additions & 12 deletions crates/ruff_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,46 +434,81 @@ fn debug_assert_no_newlines(text: &str) {
debug_assert!(!text.contains('\r'), "The content '{text}' contains an unsupported '\\r' line terminator character but text must only use line feeds '\\n' as line separator. Use '\\n' instead of '\\r' and '\\r\\n' to insert a line break in strings.");
}

/// Pushes some content to the end of the current line
/// Pushes some content to the end of the current line.
///
/// ## Examples
///
/// ```
/// use ruff_formatter::{format};
/// ```rust
/// use ruff_formatter::format;
/// use ruff_formatter::prelude::*;
///
/// fn main() -> FormatResult<()> {
/// # fn main() -> FormatResult<()> {
/// let elements = format!(SimpleFormatContext::default(), [
/// text("a"),
/// line_suffix(&text("c")),
/// line_suffix(&text("c"), 0),
/// text("b")
/// ])?;
///
/// assert_eq!(
/// "abc",
/// elements.print()?.as_code()
/// );
/// assert_eq!("abc", elements.print()?.as_code());
/// # Ok(())
/// # }
/// ```
///
/// Provide reserved width for the line suffix to include it during measurement. This
/// will push the line suffix to the next line if it exceeds the configured line width.
/// ```rust
/// use ruff_formatter::{format, format_args, LineWidth, SimpleFormatContext, SimpleFormatOptions};
/// use ruff_formatter::prelude::*;
///
/// # fn main() -> FormatResult<()> {
/// let context = SimpleFormatContext::new(SimpleFormatOptions {
/// line_width: LineWidth::try_from(2).unwrap(),
/// ..SimpleFormatOptions::default()
/// });
///
/// let elements = format!(context, [
/// group(&format_args![
/// &format_with(|f| {
/// f.fill()
/// .entry(
/// &soft_line_break(),
/// &format_args![text("a"), text("b")],
/// )
/// .entry(
/// &soft_line_break(),
/// &line_suffix(&text("c"), 1),
/// )
/// .finish()
/// }),
/// ]),
/// ])?;
///
/// assert_eq!("ab\nc", elements.print()?.as_code());
/// # Ok(())
/// # }
/// ```
#[inline]
pub fn line_suffix<Content, Context>(inner: &Content) -> LineSuffix<Context>
pub fn line_suffix<Content, Context>(inner: &Content, reserved_width: u32) -> LineSuffix<Context>
where
Content: Format<Context>,
{
LineSuffix {
content: Argument::new(inner),
reserved_width,
}
}

#[derive(Copy, Clone)]
pub struct LineSuffix<'a, Context> {
content: Argument<'a, Context>,
reserved_width: u32,
}

impl<Context> Format<Context> for LineSuffix<'_, Context> {
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
f.write_element(FormatElement::Tag(StartLineSuffix));
f.write_element(FormatElement::Tag(StartLineSuffix {
reserved_width: self.reserved_width,
}));
Arguments::from(&self.content).fmt(f)?;
f.write_element(FormatElement::Tag(EndLineSuffix));

Expand All @@ -500,7 +535,7 @@ impl<Context> std::fmt::Debug for LineSuffix<'_, Context> {
/// # fn main() -> FormatResult<()> {
/// let elements = format!(SimpleFormatContext::default(), [
/// text("a"),
/// line_suffix(&text("c")),
/// line_suffix(&text("c"), 0),
/// text("b"),
/// line_suffix_boundary(),
/// text("d")
Expand Down
16 changes: 13 additions & 3 deletions crates/ruff_formatter/src/format_element/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,16 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
)?;
}

StartLineSuffix => {
write!(f, [text("line_suffix(")])?;
StartLineSuffix { reserved_width } => {
write!(
f,
[
text("line_suffix("),
dynamic_text(&std::format!("{reserved_width:?}"), None),
text(","),
space(),
]
)?;
}

StartVerbatim(_) => {
Expand Down Expand Up @@ -671,7 +679,9 @@ impl FormatElements for [FormatElement] {
match element {
// Line suffix
// Ignore if any of its content breaks
FormatElement::Tag(Tag::StartLineSuffix | Tag::StartFitsExpanded(_)) => {
FormatElement::Tag(
Tag::StartLineSuffix { reserved_width: _ } | Tag::StartFitsExpanded(_),
) => {
ignore_depth += 1;
}
FormatElement::Tag(Tag::EndLineSuffix | Tag::EndFitsExpanded) => {
Expand Down
11 changes: 7 additions & 4 deletions crates/ruff_formatter/src/format_element/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ pub enum Tag {
StartEntry,
EndEntry,

/// Delay the printing of its content until the next line break
StartLineSuffix,
/// Delay the printing of its content until the next line break. Using reserved width will include
/// the associated line suffix during measurement.
StartLineSuffix {
reserved_width: u32,
},
EndLineSuffix,

/// A token that tracks tokens/nodes that are printed as verbatim.
Expand Down Expand Up @@ -96,7 +99,7 @@ impl Tag {
| Tag::StartIndentIfGroupBreaks(_)
| Tag::StartFill
| Tag::StartEntry
| Tag::StartLineSuffix
| Tag::StartLineSuffix { reserved_width: _ }
| Tag::StartVerbatim(_)
| Tag::StartLabelled(_)
| Tag::StartFitsExpanded(_)
Expand All @@ -122,7 +125,7 @@ impl Tag {
StartIndentIfGroupBreaks(_) | EndIndentIfGroupBreaks => TagKind::IndentIfGroupBreaks,
StartFill | EndFill => TagKind::Fill,
StartEntry | EndEntry => TagKind::Entry,
StartLineSuffix | EndLineSuffix => TagKind::LineSuffix,
StartLineSuffix { reserved_width: _ } | EndLineSuffix => TagKind::LineSuffix,
StartVerbatim(_) | EndVerbatim => TagKind::Verbatim,
StartLabelled(_) | EndLabelled => TagKind::Labelled,
StartFitsExpanded { .. } | EndFitsExpanded => TagKind::FitsExpanded,
Expand Down
41 changes: 38 additions & 3 deletions crates/ruff_formatter/src/printer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ impl<'a> Printer<'a> {
stack.push(TagKind::IndentIfGroupBreaks, args);
}

FormatElement::Tag(StartLineSuffix) => {
FormatElement::Tag(StartLineSuffix { reserved_width }) => {
self.state.line_width += reserved_width;
self.state
.line_suffixes
.extend(args, queue.iter_content(TagKind::LineSuffix));
Expand Down Expand Up @@ -1184,7 +1185,11 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}
}

FormatElement::Tag(StartLineSuffix) => {
FormatElement::Tag(StartLineSuffix { reserved_width }) => {
self.state.line_width += reserved_width;
if self.state.line_width > self.options().print_width.into() {
return Ok(Fits::No);
}
self.queue.skip_content(TagKind::LineSuffix);
self.state.has_line_suffix = true;
}
Expand Down Expand Up @@ -1720,12 +1725,42 @@ two lines`,
text("]")
]),
text(";"),
line_suffix(&format_args![space(), text("// trailing")])
line_suffix(&format_args![space(), text("// trailing")], 0)
]);

assert_eq!(printed.as_code(), "[1, 2, 3]; // trailing");
}

#[test]
fn line_suffix_with_reserved_width() {
let printed = format(&format_args![
group(&format_args![
text("["),
soft_block_indent(&format_with(|f| {
f.fill()
.entry(
&soft_line_break_or_space(),
&format_args!(text("1"), text(",")),
)
.entry(
&soft_line_break_or_space(),
&format_args!(text("2"), text(",")),
)
.entry(
&soft_line_break_or_space(),
&format_args!(text("3"), if_group_breaks(&text(","))),
)
.finish()
})),
text("]")
]),
text(";"),
line_suffix(&format_args![space(), text("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")], 80)
]);

assert_eq!(printed.as_code(), "[\n 1, 2, 3\n]; // aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
}

#[test]
fn conditional_with_group_id_in_fits() {
let content = format_with(|f| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,5 @@
h1 = ((((1, 2))))
h2 = ((((1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurpqurpqurpqurpqurpqurüqurqpuriq"))))
h3 = 1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurpqurpqurpqurpqurpqurüqurqpuriq"

i1 = ("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",) # This should break
53 changes: 46 additions & 7 deletions crates/ruff_python_formatter/src/comments/format.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use ruff_python_ast::Ranged;
use ruff_text_size::{TextLen, TextRange, TextSize};

use ruff_formatter::{format_args, write, FormatError, SourceCode};
use ruff_formatter::{format_args, write, FormatError, FormatOptions, SourceCode};
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use ruff_python_trivia::{lines_after, lines_after_ignoring_trivia, lines_before};
use unicode_width::UnicodeWidthChar;

use crate::comments::{CommentLinePosition, SourceComment};
use crate::context::NodeLevel;
Expand Down Expand Up @@ -151,18 +152,33 @@ impl Format<PyFormatContext<'_>> for FormatTrailingComments<'_> {
write!(
f,
[
line_suffix(&format_args![
empty_lines(lines_before_comment),
format_comment(trailing)
]),
line_suffix(
&format_args![
empty_lines(lines_before_comment),
format_comment(trailing)
],
0 // Reserving width isn't necessary because we don't split comments and the empty lines expand any enclosing group.
),
expand_parent()
]
)?;
} else {
// A trailing comment at the end of a line has a reserved width to consider during line measurement.
// ```python
// tup = (
// "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
// ) # Some comment
// ```
write!(
f,
[
line_suffix(&format_args![space(), space(), format_comment(trailing)]),
line_suffix(
&format_args![space(), space(), format_comment(trailing)],
measure_text(
&f.context().source()[slice.range()],
f.options().tab_width().value()
) + 2 // Account for two added spaces
),
expand_parent()
]
)?;
Expand Down Expand Up @@ -264,10 +280,18 @@ impl Format<PyFormatContext<'_>> for FormatDanglingOpenParenthesisComments<'_> {
"Expected dangling comment to be at the end of the line"
);

let slice = comment.slice();
write!(
f,
[
line_suffix(&format_args!(space(), space(), format_comment(comment))),
line_suffix(
&format_args!(space(), space(), format_comment(comment)),
// Marking the comment as a line suffix with reserved width is safe since we expect the comment to be end of line.
measure_text(
&f.context().source()[slice.range()],
f.options().tab_width().value()
) + 2 // Account for two added spaces
),
expand_parent()
]
)?;
Expand Down Expand Up @@ -374,3 +398,18 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLines {
}
}
}

/// Helper for measuring text.
fn measure_text(text: &str, tab_width: u32) -> u32 {
let mut width = 0;
for c in text.chars() {
let char_width = match c {
'\t' => tab_width,
// SAFETY: A u32 is sufficient to format files <= 4GB
#[allow(clippy::cast_possible_truncation)]
c => c.width().unwrap_or(0) as u32,
};
width += char_width;
}
width
}
11 changes: 7 additions & 4 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,13 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
if format_expression.inspect(f)?.will_break() {
// The group here is necessary because `format_expression` may contain IR elements
// that refer to the group id
group(&format_expression)
.with_group_id(Some(group_id))
.should_expand(true)
.fmt(f)
group(&format_args![
text("("),
soft_block_indent(&format_expression),
text(")")
])
.with_group_id(Some(group_id))
.fmt(f)
} else {
// Only add parentheses if it makes the expression fit on the line.
// Using the flat version as the most expanded version gives a left-to-right splitting behavior
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
)
@@ -108,11 +112,18 @@
@@ -108,11 +112,20 @@
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
)
Expand All @@ -176,7 +176,10 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
+ ], # type: ignore
)
aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items))) # type: ignore[arg-type]
-aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items))) # type: ignore[arg-type]
+aaaaaaaaaaaaa, bbbbbbbbb = map(
+ list, map(itertools.chain.from_iterable, zip(*items))
+) # type: ignore[arg-type]
```

## Ruff Output
Expand Down Expand Up @@ -310,7 +313,9 @@ call_to_some_function_asdf(
], # type: ignore
)
aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items))) # type: ignore[arg-type]
aaaaaaaaaaaaa, bbbbbbbbb = map(
list, map(itertools.chain.from_iterable, zip(*items))
) # type: ignore[arg-type]
```

## Black Output
Expand Down
Loading

0 comments on commit 4e3783f

Please sign in to comment.