Skip to content

Commit

Permalink
Prefer splitting right hand side of assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Dec 1, 2023
1 parent 1de4fcd commit 9c4e2b1
Show file tree
Hide file tree
Showing 23 changed files with 609 additions and 269 deletions.
11 changes: 10 additions & 1 deletion crates/ruff_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use ruff_text_size::TextRange;
use Tag::*;

use crate::format_element::tag::{Condition, Tag};
use crate::prelude::tag::{DedentMode, GroupMode, LabelId};
use crate::prelude::tag::{BestFitParenthesizeMode, DedentMode, GroupMode, LabelId};
use crate::prelude::*;
use crate::{write, Argument, Arguments, FormatContext, FormatOptions, GroupId, TextSize};
use crate::{Buffer, VecBuffer};
Expand Down Expand Up @@ -1553,13 +1553,15 @@ pub fn best_fit_parenthesize<Context>(
BestFitParenthesize {
content: Argument::new(content),
group_id: None,
mode: BestFitParenthesizeMode::default(),
}
}

#[derive(Copy, Clone)]
pub struct BestFitParenthesize<'a, Context> {
content: Argument<'a, Context>,
group_id: Option<GroupId>,
mode: BestFitParenthesizeMode,
}

impl<Context> BestFitParenthesize<'_, Context> {
Expand All @@ -1570,12 +1572,19 @@ impl<Context> BestFitParenthesize<'_, Context> {
self.group_id = group_id;
self
}

#[must_use]
pub fn with_mode(mut self, mode: BestFitParenthesizeMode) -> Self {
self.mode = mode;
self
}
}

impl<Context> Format<Context> for BestFitParenthesize<'_, Context> {
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
f.write_element(FormatElement::Tag(StartBestFitParenthesize {
id: self.group_id,
mode: self.mode,
}));

Arguments::from(&self.content).fmt(f)?;
Expand Down
13 changes: 11 additions & 2 deletions crates/ruff_formatter/src/format_element/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::ops::Deref;
use rustc_hash::FxHashMap;

use crate::format_element::tag::{Condition, DedentMode};
use crate::prelude::tag::GroupMode;
use crate::prelude::tag::{BestFitParenthesizeMode, GroupMode};
use crate::prelude::*;
use crate::source_code::SourceCode;
use crate::{
Expand Down Expand Up @@ -518,7 +518,7 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
}
}

StartBestFitParenthesize { id } => {
StartBestFitParenthesize { id, mode } => {
write!(f, [token("best_fit_parenthesize(")])?;

if let Some(group_id) = id {
Expand All @@ -531,6 +531,15 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
]
)?;
}

match mode {
BestFitParenthesizeMode::BestFit => {
write!(f, [token("mode: BestFit,"), space()])?;
}
BestFitParenthesizeMode::Group => {
write!(f, [token("mode: Group,"), space()])?;
}
}
}

StartConditionalGroup(group) => {
Expand Down
10 changes: 10 additions & 0 deletions crates/ruff_formatter/src/format_element/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub enum Tag {
/// See [`crate::builders::best_fit_parenthesize`] for an in-depth explanation.
StartBestFitParenthesize {
id: Option<GroupId>,
mode: BestFitParenthesizeMode,
},
EndBestFitParenthesize,
}
Expand Down Expand Up @@ -414,3 +415,12 @@ impl VerbatimKind {
matches!(self, VerbatimKind::Bogus)
}
}

#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
pub enum BestFitParenthesizeMode {
/// Behaves like `BestFit`, breaking from left to right.
#[default]
BestFit,
/// Behaves like a `Group` regular group, breaking from right to left.
Group,
}
46 changes: 38 additions & 8 deletions crates/ruff_formatter/src/printer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub use printer_options::*;
use ruff_text_size::{Ranged, TextLen, TextSize};

use crate::format_element::document::Document;
use crate::format_element::tag::{Condition, GroupMode};
use crate::format_element::tag::{BestFitParenthesizeMode, Condition, GroupMode};
use crate::format_element::{BestFittingMode, BestFittingVariants, LineMode, PrintMode};
use crate::prelude::tag::{DedentMode, Tag, TagKind, VerbatimKind};
use crate::prelude::{tag, TextWidth};
Expand Down Expand Up @@ -174,7 +174,7 @@ impl<'a> Printer<'a> {
stack.push(TagKind::Group, args.with_print_mode(print_mode));
}

FormatElement::Tag(StartBestFitParenthesize { id }) => {
FormatElement::Tag(StartBestFitParenthesize { id, mode: _ }) => {
const OPEN_PAREN: FormatElement = FormatElement::Token { text: "(" };
const INDENT: FormatElement = FormatElement::Tag(Tag::StartIndent);
const HARD_LINE_BREAK: FormatElement = FormatElement::Line(LineMode::Hard);
Expand Down Expand Up @@ -1273,23 +1273,39 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
return Ok(self.fits_group(TagKind::Group, group.mode(), group.id(), args));
}

FormatElement::Tag(StartBestFitParenthesize { id }) => {
FormatElement::Tag(StartBestFitParenthesize { id, mode }) => {
if let Some(id) = id {
self.printer
.state
.group_modes
.insert_print_mode(*id, args.mode());
}

// Don't use the parenthesized with indent layout even when measuring expanded mode similar to `BestFitting`.
// This is to expand the left and not right after the `(` parentheses (it is okay to expand after the content that it wraps).
match mode {
BestFitParenthesizeMode::BestFit => {
// Don't use the parenthesized with indent layout even when measuring expanded mode similar to `BestFitting`.
// This is to expand the left and not right after the `(` parentheses (it is okay to expand after the content that it wraps).
}
BestFitParenthesizeMode::Group => {
if args.mode().is_expanded() {
const OPEN_PAREN: FormatElement = FormatElement::Token { text: "(" };
const INDENT: FormatElement = FormatElement::Tag(Tag::StartIndent);
const HARD_LINE_BREAK: FormatElement =
FormatElement::Line(LineMode::Hard);

self.queue
.extend_back(&[OPEN_PAREN, INDENT, HARD_LINE_BREAK]);
}
}
}

self.stack.push(TagKind::BestFitParenthesize, args);
}

FormatElement::Tag(EndBestFitParenthesize) => {
// If this is the end tag of the outer most parentheses for which we measure if it fits,
// pop the indent.
if args.mode().is_expanded() && self.stack.top_kind() == Some(TagKind::Indent) {
if self.stack.top_kind() == Some(TagKind::Indent) {
self.stack.pop(TagKind::Indent).unwrap();
let unindented = self.stack.pop(TagKind::BestFitParenthesize)?;

Expand All @@ -1299,10 +1315,24 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
self.state.line_width = 0;
self.state.pending_indent = unindented.indention();

// Don't measure the `)` similar to the open parentheses but increment the line width.
return Ok(self.fits_text(Text::Token(")"), unindented));
}

self.stack.pop(TagKind::BestFitParenthesize)?;
// let indented = self.stack.pop(TagKind::Indent).unwrap();
// self.stack.pop(TagKind::BestFitParenthesize)?;
//
// const END_INDENT: FormatElement = FormatElement::Tag(Tag::EndIndent);
// const HARD_LINE_BREAK: FormatElement = FormatElement::Line(LineMode::Hard);
// const CLOSE_PAREN: FormatElement = FormatElement::Token { text: ")" };
//
// // I believe the new implementation is incorrect because it doesn't measure
// // all lines anymore? Because all lines is associated with the `BestFitParenthesize` element and we popped that one.
// self.queue
// .extend_back(&[END_INDENT, HARD_LINE_BREAK, CLOSE_PAREN]);
// self.stack.push(TagKind::Indent, indented);
} else {
self.stack.pop(TagKind::BestFitParenthesize)?;
}
}

FormatElement::Tag(StartConditionalGroup(group)) => {
Expand Down
12 changes: 10 additions & 2 deletions crates/ruff_python_formatter/src/builders.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ruff_formatter::{write, Argument, Arguments};
use ruff_formatter::{write, Argument, Arguments, GroupId};
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::context::{NodeLevel, WithNodeLevel};
Expand All @@ -12,12 +12,14 @@ where
{
ParenthesizeIfExpands {
inner: Argument::new(content),
group_id: None,
indent: true,
}
}

pub(crate) struct ParenthesizeIfExpands<'a, 'ast> {
inner: Argument<'a, PyFormatContext<'ast>>,
group_id: Option<GroupId>,
indent: bool,
}

Expand All @@ -26,6 +28,11 @@ impl ParenthesizeIfExpands<'_, '_> {
self.indent = indent;
self
}

pub(crate) fn with_group_id(mut self, id: Option<GroupId>) -> Self {
self.group_id = id;
self
}
}

impl<'ast> Format<PyFormatContext<'ast>> for ParenthesizeIfExpands<'_, 'ast> {
Expand All @@ -45,7 +52,8 @@ impl<'ast> Format<PyFormatContext<'ast>> for ParenthesizeIfExpands<'_, 'ast> {
};

if_group_breaks(&token(")")).fmt(f)
}))]
}))
.with_group_id(self.group_id)]
)
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_python_formatter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl<'a> PyFormatContext<'a> {
..self
}
}

pub(crate) const fn is_preview(&self) -> bool {
self.options.is_preview()
}
}

impl FormatContext for PyFormatContext<'_> {
Expand Down
27 changes: 22 additions & 5 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::cmp::Ordering;
use std::slice;

use ruff_formatter::prelude::tag::BestFitParenthesizeMode;
use ruff_formatter::{
write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
};
Expand All @@ -21,7 +22,10 @@ use crate::expression::parentheses::{
OptionalParentheses, Parentheses, Parenthesize,
};
use crate::prelude::*;
use crate::PyFormatOptions;
use crate::preview::{
is_hug_parens_with_braces_and_square_brackets_enabled,
is_prefer_splitting_right_hand_side_of_assignments_enabled,
};

mod binary_like;
pub(crate) mod expr_attribute;
Expand Down Expand Up @@ -129,7 +133,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.options()))
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f)
} else {
format_with_parentheses_comments(expression, &node_comments, f)
Expand Down Expand Up @@ -439,16 +443,29 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
let group_id = f.group_id("optional_parentheses");
let f = &mut WithNodeLevel::new(NodeLevel::Expression(Some(group_id)), f);

let mode = if is_prefer_splitting_right_hand_side_of_assignments_enabled(
f.context(),
) && (parent.is_stmt_assign()
|| parent.is_stmt_aug_assign()
|| parent.is_stmt_ann_assign()
|| parent.is_stmt_type_alias())
{
BestFitParenthesizeMode::Group
} else {
BestFitParenthesizeMode::BestFit
};

best_fit_parenthesize(&expression.format().with_options(Parentheses::Never))
.with_group_id(Some(group_id))
.with_mode(mode)
.fmt(f)
}
}
},
OptionalParentheses::Never => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.with_indent(!is_expression_huggable(expression, f.options()))
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f)
}

Expand Down Expand Up @@ -1061,8 +1078,8 @@ pub(crate) fn has_own_parentheses(
/// ]
/// )
/// ```
pub(crate) fn is_expression_huggable(expr: &Expr, options: &PyFormatOptions) -> bool {
if !options.preview().is_enabled() {
pub(crate) fn is_expression_huggable(expr: &Expr, context: &PyFormatContext) -> bool {
if !is_hug_parens_with_braces_and_square_brackets_enabled(context) {
return false;
}

Expand Down
14 changes: 5 additions & 9 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ mod options;
pub(crate) mod other;
pub(crate) mod pattern;
mod prelude;
mod preview;
mod shared_traits;
pub(crate) mod statement;
pub(crate) mod type_param;
Expand Down Expand Up @@ -180,7 +181,7 @@ mod tests {

use ruff_python_parser::{parse_ok_tokens, AsMode};

use crate::{format_module_ast, format_module_source, PyFormatOptions};
use crate::{format_module_ast, format_module_source, PreviewMode, PyFormatOptions};

/// Very basic test intentionally kept very similar to the CLI
#[test]
Expand Down Expand Up @@ -208,13 +209,7 @@ if True:
#[test]
fn quick_test() {
let source = r#"
def main() -> None:
if True:
some_very_long_variable_name_abcdefghijk = Foo()
some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
== "This is a very long string abcdefghijk"
]
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = 0
"#;
let source_type = PySourceType::Python;
Expand All @@ -223,7 +218,8 @@ def main() -> None:
// Parse the AST.
let source_path = "code_inline.py";
let module = parse_ok_tokens(tokens, source, source_type.as_mode(), source_path).unwrap();
let options = PyFormatOptions::from_extension(Path::new(source_path));
let options = PyFormatOptions::from_extension(Path::new(source_path))
.with_preview(PreviewMode::Enabled);
let formatted = format_module_ast(&module, &comment_ranges, source, options).unwrap();

// Uncomment the `dbg` to print the IR.
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_python_formatter/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ impl PyFormatOptions {
self.preview
}

pub const fn is_preview(&self) -> bool {
self.preview.is_enabled()
}

#[must_use]
pub fn with_indent_width(mut self, indent_width: IndentWidth) -> Self {
self.indent_width = indent_width;
Expand Down
Loading

0 comments on commit 9c4e2b1

Please sign in to comment.