Allow parenthesized content exceed configured line width (#7490)

This commit is contained in:
Micha Reiser 2023-09-20 08:39:25 +02:00 committed by GitHub
parent 5849a75223
commit 192463c2fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 171 additions and 151 deletions

View File

@ -2155,15 +2155,18 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
} }
} }
/// Changes the definition of *fits* for `content`. Instead of measuring it in *flat*, measure it with /// Changes the definition of *fits* for `content`. It measures the width of all lines and allows
/// all line breaks expanded and test if no line exceeds the line width. The [`FitsExpanded`] acts /// the content inside of the [`fits_expanded`] to exceed the configured line width. The content
/// as a expands boundary similar to best fitting, meaning that a [`hard_line_break`] will not cause the parent group to expand. /// coming before and after [`fits_expanded`] must fit into the configured line width.
///
/// The [`fits_expanded`] acts as a expands boundary similar to best fitting,
/// meaning that a [`hard_line_break`] will not cause the parent group to expand.
/// ///
/// Useful in conjunction with a group with a condition. /// Useful in conjunction with a group with a condition.
/// ///
/// ## Examples /// ## Examples
/// The outer group with the binary expression remains *flat* regardless of the array expression /// The outer group with the binary expression remains *flat* regardless of the array expression that
/// that spans multiple lines. /// spans multiple lines with items exceeding the configured line width.
/// ///
/// ``` /// ```
/// # use ruff_formatter::{format, format_args, LineWidth, SimpleFormatOptions, write}; /// # use ruff_formatter::{format, format_args, LineWidth, SimpleFormatOptions, write};
@ -2183,7 +2186,7 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
/// token("["), /// token("["),
/// soft_block_indent(&format_args![ /// soft_block_indent(&format_args![
/// token("a,"), space(), token("# comment"), expand_parent(), soft_line_break_or_space(), /// token("a,"), space(), token("# comment"), expand_parent(), soft_line_break_or_space(),
/// token("b") /// token("'A very long string that exceeds the configured line width of 80 characters but the enclosing binary expression still fits.'")
/// ]), /// ]),
/// token("]") /// token("]")
/// ])) /// ]))
@ -2194,7 +2197,7 @@ impl<Context> std::fmt::Debug for IndentIfGroupBreaks<'_, Context> {
/// let formatted = format!(SimpleFormatContext::default(), [content])?; /// let formatted = format!(SimpleFormatContext::default(), [content])?;
/// ///
/// assert_eq!( /// assert_eq!(
/// "a + [\n\ta, # comment\n\tb\n]", /// "a + [\n\ta, # comment\n\t'A very long string that exceeds the configured line width of 80 characters but the enclosing binary expression still fits.'\n]",
/// formatted.print()?.as_code() /// formatted.print()?.as_code()
/// ); /// );
/// # Ok(()) /// # Ok(())

View File

@ -84,11 +84,14 @@ pub enum Tag {
StartFitsExpanded(FitsExpanded), StartFitsExpanded(FitsExpanded),
EndFitsExpanded, EndFitsExpanded,
/// Marks the start and end of a best-fitting variant.
StartBestFittingEntry, StartBestFittingEntry,
EndBestFittingEntry, EndBestFittingEntry,
/// Parenthesizes the content but only if adding the parentheses and indenting the content /// Parenthesizes the content but only if adding the parentheses and indenting the content
/// makes the content fit in the configured line width. /// makes the content fit in the configured line width.
///
/// See [`crate::builders::best_fit_parenthesize`] for an in-depth explanation.
StartBestFitParenthesize { StartBestFitParenthesize {
id: Option<GroupId>, id: Option<GroupId>,
}, },

View File

@ -1,5 +1,3 @@
#![allow(dead_code)]
use crate::prelude::*; use crate::prelude::*;
use std::cell::OnceCell; use std::cell::OnceCell;
use std::marker::PhantomData; use std::marker::PhantomData;

View File

@ -1183,7 +1183,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
// line break should be printed as regular line break // line break should be printed as regular line break
return Ok(Fits::Yes); return Ok(Fits::Yes);
} }
MeasureMode::AllLines => { MeasureMode::AllLines | MeasureMode::AllLinesAllowTextOverflow => {
// Continue measuring on the next line // Continue measuring on the next line
self.state.line_width = 0; self.state.line_width = 0;
self.state.pending_indent = args.indention(); self.state.pending_indent = args.indention();
@ -1354,9 +1354,11 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
} }
FormatElement::Tag(StartLineSuffix { reserved_width }) => { FormatElement::Tag(StartLineSuffix { reserved_width }) => {
self.state.line_width += reserved_width; if *reserved_width > 0 {
if self.state.line_width > self.options().line_width.into() { self.state.line_width += reserved_width;
return Ok(Fits::No); if self.state.line_width > self.options().line_width.into() {
return Ok(Fits::No);
}
} }
self.queue.skip_content(TagKind::LineSuffix); self.queue.skip_content(TagKind::LineSuffix);
self.state.has_line_suffix = true; self.state.has_line_suffix = true;
@ -1370,32 +1372,42 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
condition, condition,
propagate_expand, propagate_expand,
})) => { })) => {
let condition_met = match condition { match args.mode() {
Some(condition) => { PrintMode::Expanded => {
let group_mode = match condition.group_id { // As usual, nothing to measure
Some(group_id) => self.group_modes().get_print_mode(group_id)?, self.stack.push(TagKind::FitsExpanded, args);
None => args.mode(), }
PrintMode::Flat => {
let condition_met = match condition {
Some(condition) => {
let group_mode = match condition.group_id {
Some(group_id) => {
self.group_modes().get_print_mode(group_id)?
}
None => args.mode(),
};
condition.mode == group_mode
}
None => true,
}; };
condition.mode == group_mode if condition_met {
} // Measure in fully expanded mode and allow overflows
None => true, self.stack.push(
}; TagKind::FitsExpanded,
args.with_measure_mode(MeasureMode::AllLinesAllowTextOverflow)
.with_print_mode(PrintMode::Expanded),
);
} else {
if propagate_expand.get() {
return Ok(Fits::No);
}
if condition_met { // As usual
// Measure in fully expanded mode. self.stack.push(TagKind::FitsExpanded, args);
self.stack.push( }
TagKind::FitsExpanded,
args.with_print_mode(PrintMode::Expanded)
.with_measure_mode(MeasureMode::AllLines),
);
} else {
if propagate_expand.get() && args.mode().is_flat() {
return Ok(Fits::No);
} }
// As usual
self.stack.push(TagKind::FitsExpanded, args);
} }
} }
@ -1482,7 +1494,8 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
} }
match args.measure_mode() { match args.measure_mode() {
MeasureMode::FirstLine => return Fits::Yes, MeasureMode::FirstLine => return Fits::Yes,
MeasureMode::AllLines => { MeasureMode::AllLines
| MeasureMode::AllLinesAllowTextOverflow => {
self.state.line_width = 0; self.state.line_width = 0;
continue; continue;
} }
@ -1498,7 +1511,9 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
} }
} }
if self.state.line_width > self.options().line_width.into() { if self.state.line_width > self.options().line_width.into()
&& !args.measure_mode().allows_text_overflow()
{
return Fits::No; return Fits::No;
} }
@ -1601,6 +1616,17 @@ enum MeasureMode {
/// The content only fits if none of the lines exceed the print width. Lines are terminated by either /// The content only fits if none of the lines exceed the print width. Lines are terminated by either
/// a hard line break or a soft line break in [`PrintMode::Expanded`]. /// a hard line break or a soft line break in [`PrintMode::Expanded`].
AllLines, AllLines,
/// Measures all lines and allows lines to exceed the configured line width. Useful when it only matters
/// whether the content *before* and *after* fits.
AllLinesAllowTextOverflow,
}
impl MeasureMode {
/// Returns `true` if this mode allows text exceeding the configured line width.
const fn allows_text_overflow(self) -> bool {
matches!(self, MeasureMode::AllLinesAllowTextOverflow)
}
} }
impl From<BestFittingMode> for MeasureMode { impl From<BestFittingMode> for MeasureMode {

View File

@ -120,7 +120,6 @@ impl SourceMapGeneration {
} }
} }
#[allow(dead_code)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] #[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum LineEnding { pub enum LineEnding {

View File

@ -394,3 +394,15 @@ z = (
# c: and this comment # c: and this comment
+ a + a
) )
# Test for https://github.com/astral-sh/ruff/issues/7431
if True:
if True:
if True:
if True:
msg += " " + _(
"Since the role is not mentionable, it will be momentarily made mentionable "
"when announcing a streamalert. Please make sure I have the correct "
"permissions to manage this role, or else members of this role won't receive "
"a notification."
)

View File

@ -94,8 +94,3 @@ def f():
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = ( aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = (
True True
) )
# Regression test for https://github.com/astral-sh/ruff/issues/7462
if grid is not None:
rgrid = (rgrid.rio.reproject_match(grid, nodata=fillvalue) # rio.reproject nodata is use to initlialize the destination array
.where(~grid.isnull()))

View File

@ -153,3 +153,19 @@ def test():
key9: value9, key9: value9,
} }
), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee" ), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee"
# Test for https://github.com/astral-sh/ruff/issues/7246
assert items == [
"a very very very very very very very very very very very very very very very long string",
]
assert package.files == [
{
"file": "pytest-3.5.0-py2.py3-none-any.whl",
"hash": "sha256:6266f87ab64692112e5477eba395cfedda53b1933ccd29478e671e73b420c19c", # noqa: E501
},
{
"file": "pytest-3.5.0.tar.gz",
"hash": "sha256:fae491d1874f199537fd5872b5e1f0e74a009b979df9d53d1553fd03da1703e1", # noqa: E501
},
]

View File

@ -155,32 +155,34 @@ impl<'content, 'ast> FormatParenthesized<'content, 'ast> {
impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> { impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'ast>>) -> FormatResult<()> { fn fmt(&self, f: &mut Formatter<PyFormatContext<'ast>>) -> FormatResult<()> {
let inner = format_with(|f| { let current_level = f.context().node_level();
let content = format_with(|f| {
group(&format_args![ group(&format_args![
token(self.left),
dangling_open_parenthesis_comments(self.comments), dangling_open_parenthesis_comments(self.comments),
soft_block_indent(&Arguments::from(&self.content)), soft_block_indent(&Arguments::from(&self.content))
token(self.right)
]) ])
.fmt(f) .fmt(f)
}); });
let current_level = f.context().node_level(); let inner = format_with(|f| {
if let NodeLevel::Expression(Some(group_id)) = current_level {
// Use fits expanded if there's an enclosing group that adds the optional parentheses.
// This ensures that expanding this parenthesized expression does not expand the optional parentheses group.
write!(
f,
[fits_expanded(&content)
.with_condition(Some(Condition::if_group_fits_on_line(group_id)))]
)
} else {
// It's not necessary to wrap the content if it is not inside of an optional_parentheses group.
content.fmt(f)
}
});
let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f);
if let NodeLevel::Expression(Some(group_id)) = current_level { write!(f, [token(self.left), inner, token(self.right)])
// Use fits expanded if there's an enclosing group that adds the optional parentheses.
// This ensures that expanding this parenthesized expression does not expand the optional parentheses group.
write!(
f,
[fits_expanded(&inner)
.with_condition(Some(Condition::if_group_fits_on_line(group_id)))]
)
} else {
// It's not necessary to wrap the content if it is not inside of an optional_parentheses group.
write!(f, [inner])
}
} }
} }

View File

@ -1,77 +0,0 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/trailing_comma_optional_parens3.py
---
## Input
```py
if True:
if True:
if True:
return _(
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
) % {"reported_username": reported_username, "report_reason": report_reason}
```
## Black Differences
```diff
--- Black
+++ Ruff
@@ -1,8 +1,14 @@
if True:
if True:
if True:
- return _(
- "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
- + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
- "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
- ) % {"reported_username": reported_username, "report_reason": report_reason}
+ return (
+ _(
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
+ + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
+ )
+ % {
+ "reported_username": reported_username,
+ "report_reason": report_reason,
+ }
+ )
```
## Ruff Output
```py
if True:
if True:
if True:
return (
_(
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
)
% {
"reported_username": reported_username,
"report_reason": report_reason,
}
)
```
## Black Output
```py
if True:
if True:
if True:
return _(
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas "
+ "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.",
"qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe",
) % {"reported_username": reported_username, "report_reason": report_reason}
```

View File

@ -400,6 +400,18 @@ z = (
# c: and this comment # c: and this comment
+ a + a
) )
# Test for https://github.com/astral-sh/ruff/issues/7431
if True:
if True:
if True:
if True:
msg += " " + _(
"Since the role is not mentionable, it will be momentarily made mentionable "
"when announcing a streamalert. Please make sure I have the correct "
"permissions to manage this role, or else members of this role won't receive "
"a notification."
)
``` ```
## Output ## Output
@ -849,6 +861,18 @@ z = (
# c: and this comment # c: and this comment
+ a + a
) )
# Test for https://github.com/astral-sh/ruff/issues/7431
if True:
if True:
if True:
if True:
msg += " " + _(
"Since the role is not mentionable, it will be momentarily made mentionable "
"when announcing a streamalert. Please make sure I have the correct "
"permissions to manage this role, or else members of this role won't receive "
"a notification."
)
``` ```

View File

@ -100,11 +100,6 @@ def f():
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = ( aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = (
True True
) )
# Regression test for https://github.com/astral-sh/ruff/issues/7462
if grid is not None:
rgrid = (rgrid.rio.reproject_match(grid, nodata=fillvalue) # rio.reproject nodata is use to initlialize the destination array
.where(~grid.isnull()))
``` ```
## Output ## Output
@ -227,15 +222,6 @@ def f():
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = ( aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = (
True True
) )
# Regression test for https://github.com/astral-sh/ruff/issues/7462
if grid is not None:
rgrid = rgrid.rio.reproject_match(
grid, nodata=fillvalue
).where( # rio.reproject nodata is use to initlialize the destination array
~grid.isnull()
)
``` ```

View File

@ -159,6 +159,22 @@ def test():
key9: value9, key9: value9,
} }
), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee" ), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee"
# Test for https://github.com/astral-sh/ruff/issues/7246
assert items == [
"a very very very very very very very very very very very very very very very long string",
]
assert package.files == [
{
"file": "pytest-3.5.0-py2.py3-none-any.whl",
"hash": "sha256:6266f87ab64692112e5477eba395cfedda53b1933ccd29478e671e73b420c19c", # noqa: E501
},
{
"file": "pytest-3.5.0.tar.gz",
"hash": "sha256:fae491d1874f199537fd5872b5e1f0e74a009b979df9d53d1553fd03da1703e1", # noqa: E501
},
]
``` ```
## Output ## Output
@ -326,6 +342,23 @@ def test():
key9: value9, key9: value9,
} }
), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee" ), "Not what we expected and the message is too long to fit in one lineeeeeeeeeeeeeee"
# Test for https://github.com/astral-sh/ruff/issues/7246
assert items == [
"a very very very very very very very very very very very very very very very long string",
]
assert package.files == [
{
"file": "pytest-3.5.0-py2.py3-none-any.whl",
"hash": "sha256:6266f87ab64692112e5477eba395cfedda53b1933ccd29478e671e73b420c19c", # noqa: E501
},
{
"file": "pytest-3.5.0.tar.gz",
"hash": "sha256:fae491d1874f199537fd5872b5e1f0e74a009b979df9d53d1553fd03da1703e1", # noqa: E501
},
]
``` ```