mirror of https://github.com/astral-sh/ruff
Don't format trailing comma for lambda arguments (#5946)
**Summary** lambda arguments don't have parentheses, so they shouldn't get a magic trailing comma either. This fixes some unstable formatting **Test Plan** Added a regression test. 89 (from previously 145) instances of unstable formatting remaining. ``` $ cargo run --bin ruff_dev --release -- format-dev --stability-check --error-file formatter-ecosystem-errors.txt --multi-project target/checkouts > formatter-ecosystem-progress.txt $ rg "Unstable formatting" target/formatter-ecosystem-errors.txt | wc -l 89 ``` Closes #5892
This commit is contained in:
parent
40f54375cb
commit
06d9ff9577
|
|
@ -66,3 +66,12 @@ a = (
|
||||||
# formatting
|
# formatting
|
||||||
(lambda:(#
|
(lambda:(#
|
||||||
),)
|
),)
|
||||||
|
|
||||||
|
# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
|
||||||
|
def f(
|
||||||
|
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
# ...but we do preserve a trailing comma after the arguments
|
||||||
|
a = lambda b,: 0
|
||||||
|
|
|
||||||
|
|
@ -20,3 +20,10 @@
|
||||||
"seventh entry",
|
"seventh entry",
|
||||||
"eighth entry",
|
"eighth entry",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Regression test: Respect setting in Arguments formatting
|
||||||
|
def f(a): pass
|
||||||
|
def g(a,): pass
|
||||||
|
|
||||||
|
x1 = lambda y: 1
|
||||||
|
x2 = lambda y,: 1
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@ use ruff_text_size::{TextRange, TextSize};
|
||||||
|
|
||||||
use ruff_formatter::{format_args, write, FormatRuleWithOptions};
|
use ruff_formatter::{format_args, write, FormatRuleWithOptions};
|
||||||
use ruff_python_ast::node::{AnyNodeRef, AstNode};
|
use ruff_python_ast::node::{AnyNodeRef, AstNode};
|
||||||
use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
|
use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
|
||||||
|
|
||||||
use crate::comments::{
|
use crate::comments::{
|
||||||
dangling_comments, leading_comments, leading_node_comments, trailing_comments,
|
dangling_comments, leading_comments, leading_node_comments, trailing_comments,
|
||||||
|
|
@ -160,34 +160,31 @@ impl FormatNodeRule<Arguments> for FormatArguments {
|
||||||
|
|
||||||
joiner.finish()?;
|
joiner.finish()?;
|
||||||
|
|
||||||
write!(f, [if_group_breaks(&text(","))])?;
|
// Functions use the regular magic trailing comma logic, lambdas may or may not have
|
||||||
|
// a trailing comma but it's just preserved without any magic.
|
||||||
|
// ```python
|
||||||
|
// # Add magic trailing comma if its expands
|
||||||
|
// def f(a): pass
|
||||||
|
// # Expands if magic trailing comma setting is respect, otherwise remove the comma
|
||||||
|
// def g(a,): pass
|
||||||
|
// # Never expands
|
||||||
|
// x1 = lambda y: 1
|
||||||
|
// # Never expands, the comma is always preserved
|
||||||
|
// x2 = lambda y,: 1
|
||||||
|
// ```
|
||||||
|
if self.parentheses == ArgumentsParentheses::SkipInsideLambda {
|
||||||
|
// For lambdas (no parentheses), preserve the trailing comma. It doesn't
|
||||||
|
// behave like a magic trailing comma, it's just preserved
|
||||||
|
if has_trailing_comma(item, last_node, f.context().source()) {
|
||||||
|
write!(f, [text(",")])?;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
write!(f, [if_group_breaks(&text(","))])?;
|
||||||
|
|
||||||
// Expand the group if the source has a trailing *magic* comma.
|
if f.options().magic_trailing_comma().is_respect()
|
||||||
if let Some(last_node) = last_node {
|
&& has_trailing_comma(item, last_node, f.context().source())
|
||||||
let ends_with_pos_only_argument_separator = !posonlyargs.is_empty()
|
{
|
||||||
&& args.is_empty()
|
// Make the magic trailing comma expand the group
|
||||||
&& vararg.is_none()
|
|
||||||
&& kwonlyargs.is_empty()
|
|
||||||
&& kwarg.is_none();
|
|
||||||
|
|
||||||
let maybe_comma_token = if ends_with_pos_only_argument_separator {
|
|
||||||
// `def a(b, c, /): ... `
|
|
||||||
let mut tokens =
|
|
||||||
SimpleTokenizer::starts_at(last_node.end(), f.context().source())
|
|
||||||
.skip_trivia();
|
|
||||||
|
|
||||||
let comma = tokens.next();
|
|
||||||
assert!(matches!(comma, Some(SimpleToken { kind: SimpleTokenKind::Comma, .. })), "The last positional only argument must be separated by a `,` from the positional only arguments separator `/` but found '{comma:?}'.");
|
|
||||||
|
|
||||||
let slash = tokens.next();
|
|
||||||
assert!(matches!(slash, Some(SimpleToken { kind: SimpleTokenKind::Slash, .. })), "The positional argument separator must be present for a function that has positional only arguments but found '{slash:?}'.");
|
|
||||||
|
|
||||||
tokens.next()
|
|
||||||
} else {
|
|
||||||
first_non_trivia_token(last_node.end(), f.context().source())
|
|
||||||
};
|
|
||||||
|
|
||||||
if maybe_comma_token.map_or(false, |token| token.kind() == SimpleTokenKind::Comma) {
|
|
||||||
write!(f, [hard_line_break()])?;
|
write!(f, [hard_line_break()])?;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -591,3 +588,33 @@ pub(crate) enum ArgumentSeparatorCommentLocation {
|
||||||
StarLeading,
|
StarLeading,
|
||||||
StarTrailing,
|
StarTrailing,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn has_trailing_comma(arguments: &Arguments, last_node: Option<AnyNodeRef>, source: &str) -> bool {
|
||||||
|
// No nodes, no trailing comma
|
||||||
|
let Some(last_node) = last_node else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
let ends_with_pos_only_argument_separator = !arguments.posonlyargs.is_empty()
|
||||||
|
&& arguments.args.is_empty()
|
||||||
|
&& arguments.vararg.is_none()
|
||||||
|
&& arguments.kwonlyargs.is_empty()
|
||||||
|
&& arguments.kwarg.is_none();
|
||||||
|
|
||||||
|
let mut tokens = SimpleTokenizer::starts_at(last_node.end(), source).skip_trivia();
|
||||||
|
// `def a(b, c, /): ... `
|
||||||
|
// The slash lacks its own node
|
||||||
|
if ends_with_pos_only_argument_separator {
|
||||||
|
let comma = tokens.next();
|
||||||
|
assert!(matches!(comma, Some(SimpleToken { kind: SimpleTokenKind::Comma, .. })), "The last positional only argument must be separated by a `,` from the positional only arguments separator `/` but found '{comma:?}'.");
|
||||||
|
|
||||||
|
let slash = tokens.next();
|
||||||
|
assert!(matches!(slash, Some(SimpleToken { kind: SimpleTokenKind::Slash, .. })), "The positional argument separator must be present for a function that has positional only arguments but found '{slash:?}'.");
|
||||||
|
}
|
||||||
|
|
||||||
|
tokens
|
||||||
|
.next()
|
||||||
|
.expect("There must be a token after the argument list")
|
||||||
|
.kind()
|
||||||
|
== SimpleTokenKind::Comma
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -72,6 +72,15 @@ a = (
|
||||||
# formatting
|
# formatting
|
||||||
(lambda:(#
|
(lambda:(#
|
||||||
),)
|
),)
|
||||||
|
|
||||||
|
# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
|
||||||
|
def f(
|
||||||
|
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
# ...but we do preserve a trailing comma after the arguments
|
||||||
|
a = lambda b,: 0
|
||||||
```
|
```
|
||||||
|
|
||||||
## Output
|
## Output
|
||||||
|
|
@ -143,6 +152,17 @@ a = (
|
||||||
lambda: ( #
|
lambda: ( #
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
|
||||||
|
def f(
|
||||||
|
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
# ...but we do preserve a trailing comma after the arguments
|
||||||
|
a = lambda b,: 0
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -26,6 +26,13 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/skip_magic
|
||||||
"seventh entry",
|
"seventh entry",
|
||||||
"eighth entry",
|
"eighth entry",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Regression test: Respect setting in Arguments formatting
|
||||||
|
def f(a): pass
|
||||||
|
def g(a,): pass
|
||||||
|
|
||||||
|
x1 = lambda y: 1
|
||||||
|
x2 = lambda y,: 1
|
||||||
```
|
```
|
||||||
|
|
||||||
## Outputs
|
## Outputs
|
||||||
|
|
@ -56,6 +63,21 @@ magic-trailing-comma = Respect
|
||||||
"seventh entry",
|
"seventh entry",
|
||||||
"eighth entry",
|
"eighth entry",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Regression test: Respect setting in Arguments formatting
|
||||||
|
def f(a):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def g(
|
||||||
|
a,
|
||||||
|
):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
x1 = lambda y: 1
|
||||||
|
x2 = lambda y,: 1
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -82,6 +104,19 @@ magic-trailing-comma = Ignore
|
||||||
"seventh entry",
|
"seventh entry",
|
||||||
"eighth entry",
|
"eighth entry",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Regression test: Respect setting in Arguments formatting
|
||||||
|
def f(a):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def g(a):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
x1 = lambda y: 1
|
||||||
|
x2 = lambda y,: 1
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue