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.

Sign up for GitHub

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

Handle pattern parentheses in FormatPattern #6800

Merged
merged 2 commits into from
Aug 25, 2023
Merged

Handle pattern parentheses in FormatPattern #6800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fc6145
Handle pattern parentheses in FormatPattern
charliermarsh Aug 23, 2023
728e378
Break
charliermarsh Aug 23, 2023
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
45 changes: 45 additions & 0 deletions crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ def foo():
):
y = 1


match foo:
case [1, 2, *rest]:
pass
Expand Down Expand Up @@ -299,3 +300,47 @@ def foo():
_, 1, 2]:
pass


match foo:
case (1):
pass
case ((1)):
pass
case [(1), 2]:
pass
case [( # comment
1
), 2]:
pass
case [ # outer
( # inner
1
), 2]:
pass
case [
( # outer
[ # inner
1,
]
)
]:
pass
case [ # outer
( # inner outer
[ # inner
1,
]
)
]:
pass
case [ # outer
# own line
( # inner outer
[ # inner
1,
]
)
]:
pass
case [(*rest), (a as b)]:
pass
23 changes: 0 additions & 23 deletions crates/ruff_python_formatter/src/comments/placement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ fn handle_enclosed_comment<'a>(
handle_leading_class_with_decorators_comment(comment, class_def)
}
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
AnyNodeRef::MatchCase(match_case) => handle_match_case_comment(comment, match_case),
AnyNodeRef::StmtWith(with_) => handle_with_comment(comment, with_),
AnyNodeRef::ExprConstant(_) => {
if let Some(AnyNodeRef::ExprFString(fstring)) = comment.enclosing_parent() {
Expand Down Expand Up @@ -1361,28 +1360,6 @@ fn handle_import_from_comment<'a>(
}
}

/// Attach an enclosed end-of-line comment to a [`MatchCase`].
///
/// For example, given:
/// ```python
/// case ( # comment
/// pattern
/// ):
/// ...
/// ```
///
/// The comment will be attached to the [`MatchCase`] node as a dangling comment.
fn handle_match_case_comment<'a>(
comment: DecoratedComment<'a>,
match_case: &'a MatchCase,
) -> CommentPlacement<'a> {
if comment.line_position().is_end_of_line() && comment.start() < match_case.pattern.start() {
CommentPlacement::dangling(comment.enclosing_node(), comment)
} else {
CommentPlacement::Default(comment)
}
}

/// Attach an enclosed end-of-line comment to a [`ast::StmtWith`].
///
/// For example, given:
Expand Down
74 changes: 28 additions & 46 deletions crates/ruff_python_formatter/src/other/match_case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::{MatchCase, Pattern, Ranged};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::TextRange;
use ruff_python_ast::node::AstNode;
use ruff_python_ast::MatchCase;

use crate::builders::parenthesize_if_expands;
use crate::comments::SourceComment;
use crate::expression::parentheses::parenthesized;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses};
use crate::prelude::*;
use crate::statement::clause::{clause_body, clause_header, ClauseHeader};
use crate::{FormatError, FormatNodeRule, PyFormatter};
use crate::{FormatNodeRule, PyFormatter};

#[derive(Default)]
pub struct FormatMatchCase;
Expand All @@ -21,13 +21,8 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
body,
} = item;

// Distinguish dangling comments that appear on the open parenthesis from those that
// appear on the trailing colon.
let comments = f.context().comments().clone();
let dangling_item_comments = comments.dangling(item);
let (open_parenthesis_comments, trailing_colon_comments) = dangling_item_comments.split_at(
dangling_item_comments.partition_point(|comment| comment.start() < pattern.start()),
);

write!(
f,
Expand All @@ -38,12 +33,29 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
&format_with(|f| {
write!(f, [text("case"), space()])?;

if is_match_case_pattern_parenthesized(item, pattern, f.context())? {
parenthesized("(", &pattern.format(), ")")
.with_dangling_comments(open_parenthesis_comments)
.fmt(f)?;
let has_comments = comments.has_leading(pattern)
|| comments.has_trailing_own_line(pattern);

if has_comments {
pattern.format().with_options(Parentheses::Always).fmt(f)?;
} else {
pattern.format().fmt(f)?;
match pattern.needs_parentheses(item.as_any_node_ref(), f.context()) {
OptionalParentheses::Multiline => {
parenthesize_if_expands(
&pattern.format().with_options(Parentheses::Never),
)
.fmt(f)?;
}
OptionalParentheses::Always => {
pattern.format().with_options(Parentheses::Always).fmt(f)?;
}
OptionalParentheses::Never => {
pattern.format().with_options(Parentheses::Never).fmt(f)?;
}
OptionalParentheses::BestFit => {
pattern.format().with_options(Parentheses::Never).fmt(f)?;
}
}
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 relatively similar to maybe_parenthesize_expression...

}

if let Some(guard) = guard {
Expand All @@ -53,7 +65,7 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
Ok(())
}),
),
clause_body(body, trailing_colon_comments),
clause_body(body, dangling_item_comments),
]
)
}
Expand All @@ -67,33 +79,3 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
Ok(())
}
}

fn is_match_case_pattern_parenthesized(
case: &MatchCase,
pattern: &Pattern,
context: &PyFormatContext,
) -> FormatResult<bool> {
let mut tokenizer = SimpleTokenizer::new(
context.source(),
TextRange::new(case.start(), pattern.start()),
)
.skip_trivia();

let case_keyword = tokenizer.next().ok_or(FormatError::syntax_error(
"Expected a `case` keyword, didn't find any token",
))?;

debug_assert_eq!(
case_keyword.kind(),
SimpleTokenKind::Case,
"Expected `case` keyword but at {case_keyword:?}"
);

match tokenizer.next() {
Some(left_paren) => {
debug_assert_eq!(left_paren.kind(), SimpleTokenKind::LParen);
Ok(true)
}
None => Ok(false),
}
}
124 changes: 108 additions & 16 deletions crates/ruff_python_formatter/src/pattern/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule};
use ruff_python_ast::Pattern;
use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::{Pattern, Ranged};
use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};

use crate::expression::parentheses::{
parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::prelude::*;

pub(crate) mod pattern_match_as;
Expand All @@ -12,20 +17,64 @@ pub(crate) mod pattern_match_singleton;
pub(crate) mod pattern_match_star;
pub(crate) mod pattern_match_value;

#[derive(Default)]
pub struct FormatPattern;
#[derive(Copy, Clone, PartialEq, Eq, Default)]
pub struct FormatPattern {
parentheses: Parentheses,
}

impl FormatRuleWithOptions<Pattern, PyFormatContext<'_>> for FormatPattern {
type Options = Parentheses;

fn with_options(mut self, options: Self::Options) -> Self {
self.parentheses = options;
self
}
}

impl FormatRule<Pattern, PyFormatContext<'_>> for FormatPattern {
fn fmt(&self, item: &Pattern, f: &mut PyFormatter) -> FormatResult<()> {
match item {
Pattern::MatchValue(p) => p.format().fmt(f),
Pattern::MatchSingleton(p) => p.format().fmt(f),
Pattern::MatchSequence(p) => p.format().fmt(f),
Pattern::MatchMapping(p) => p.format().fmt(f),
Pattern::MatchClass(p) => p.format().fmt(f),
Pattern::MatchStar(p) => p.format().fmt(f),
Pattern::MatchAs(p) => p.format().fmt(f),
Pattern::MatchOr(p) => p.format().fmt(f),
fn fmt(&self, pattern: &Pattern, f: &mut PyFormatter) -> FormatResult<()> {
let format_pattern = format_with(|f| match pattern {
Pattern::MatchValue(pattern) => pattern.format().fmt(f),
Pattern::MatchSingleton(pattern) => pattern.format().fmt(f),
Pattern::MatchSequence(pattern) => pattern.format().fmt(f),
Pattern::MatchMapping(pattern) => pattern.format().fmt(f),
Pattern::MatchClass(pattern) => pattern.format().fmt(f),
Pattern::MatchStar(pattern) => pattern.format().fmt(f),
Pattern::MatchAs(pattern) => pattern.format().fmt(f),
Pattern::MatchOr(pattern) => pattern.format().fmt(f),
});

let parenthesize = match self.parentheses {
Parentheses::Preserve => is_pattern_parenthesized(pattern, f.context().source()),
Parentheses::Always => true,
Parentheses::Never => false,
};

if parenthesize {
let comments = f.context().comments().clone();

// Any comments on the open parenthesis.
//
// For example, `# comment` in:
// ```python
// ( # comment
// 1
// )
// ```
let open_parenthesis_comment = comments
.leading(pattern)
.first()
.filter(|comment| comment.line_position().is_end_of_line());

parenthesized("(", &format_pattern, ")")
.with_dangling_comments(
open_parenthesis_comment
.map(std::slice::from_ref)
.unwrap_or_default(),
)
.fmt(f)
} else {
format_pattern.fmt(f)
}
}
}
Expand All @@ -34,14 +83,57 @@ impl<'ast> AsFormat<PyFormatContext<'ast>> for Pattern {
type Format<'a> = FormatRefWithRule<'a, Pattern, FormatPattern, PyFormatContext<'ast>>;

fn format(&self) -> Self::Format<'_> {
FormatRefWithRule::new(self, FormatPattern)
FormatRefWithRule::new(self, FormatPattern::default())
}
}

impl<'ast> IntoFormat<PyFormatContext<'ast>> for Pattern {
type Format = FormatOwnedWithRule<Pattern, FormatPattern, PyFormatContext<'ast>>;

fn into_format(self) -> Self::Format {
FormatOwnedWithRule::new(self, FormatPattern)
FormatOwnedWithRule::new(self, FormatPattern::default())
}
}

fn is_pattern_parenthesized(pattern: &Pattern, contents: &str) -> bool {
// First test if there's a closing parentheses because it tends to be cheaper.
if matches!(
first_non_trivia_token(pattern.end(), contents),
Some(SimpleToken {
kind: SimpleTokenKind::RParen,
..
})
) {
let mut tokenizer =
SimpleTokenizer::up_to_without_back_comment(pattern.start(), contents).skip_trivia();

matches!(
tokenizer.next_back(),
Some(SimpleToken {
kind: SimpleTokenKind::LParen,
..
})
)
} else {
false
}
}

impl NeedsParentheses for Pattern {
fn needs_parentheses(
&self,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
match self {
Pattern::MatchValue(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchSingleton(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchSequence(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchMapping(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchClass(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchStar(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchAs(pattern) => pattern.needs_parentheses(parent, context),
Pattern::MatchOr(pattern) => pattern.needs_parentheses(parent, context),
}
}
}
Loading