Make Parameters an optional field on ExprLambda (#6669)

## Summary

If a lambda doesn't contain any parameters, or any parameter _tokens_
(like `*`), we can use `None` for the parameters. This feels like a
better representation to me, since, e.g., what should the `TextRange` be
for a non-existent set of parameters? It also allows us to remove
several sites where we check if the `Parameters` is empty by seeing if
it contains any arguments, so semantically, we're already trying to
detect and model around this elsewhere.

Changing this also fixes a number of issues with dangling comments in
parameter-less lambdas, since those comments are now automatically
marked as dangling on the lambda. (As-is, we were also doing something
not-great whereby the lambda was responsible for formatting dangling
comments on the parameters, which has been removed.)

Closes https://github.com/astral-sh/ruff/issues/6646.

Closes https://github.com/astral-sh/ruff/issues/6647.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2023-08-18 11:34:54 -04:00 committed by GitHub
parent ea72d5feba
commit 6a5acde226
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
30 changed files with 517 additions and 412 deletions

View file

@ -56,12 +56,16 @@ lambda x: lambda y: lambda z: (
z) # Trailing
# Trailing
a = (
lambda # Dangling
: 1
)
a = (
lambda x # Dangling
, y: 1
)
# Regression test: lambda empty arguments ranges were too long, leading to unstable
# formatting
(lambda:(#
@ -93,3 +97,24 @@ lambda a, *args, b, **kwds,: 0
lambda a, *, b, **kwds,: 0
lambda a, /: a
lambda a, /, c: a
# Dangling comments without parameters.
(
lambda
: # 3
None
)
(
lambda
# 3
: None
)
(
lambda # 1
# 2
: # 3
# 4
None # 5
)

View file

@ -162,9 +162,14 @@ fn handle_enclosed_comment<'a>(
locator: &Locator,
) -> CommentPlacement<'a> {
match comment.enclosing_node() {
AnyNodeRef::Parameters(arguments) => {
handle_parameters_separator_comment(comment, arguments, locator)
.or_else(|comment| handle_bracketed_end_of_line_comment(comment, locator))
AnyNodeRef::Parameters(parameters) => {
handle_parameters_separator_comment(comment, parameters, locator).or_else(|comment| {
if are_parameters_parenthesized(parameters, locator.contents()) {
handle_bracketed_end_of_line_comment(comment, locator)
} else {
CommentPlacement::Default(comment)
}
})
}
AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) => {
handle_bracketed_end_of_line_comment(comment, locator)
@ -1544,6 +1549,13 @@ fn is_first_statement_in_alternate_body(statement: AnyNodeRef, has_body: AnyNode
}
}
/// Returns `true` if the parameters are parenthesized (as in a function definition), or `false` if
/// not (as in a lambda).
fn are_parameters_parenthesized(parameters: &Parameters, contents: &str) -> bool {
// A lambda never has parentheses around its parameters, but a function definition always does.
contents[parameters.range()].starts_with('(')
}
/// Counts the number of empty lines in `contents`.
fn max_empty_lines(contents: &str) -> u32 {
let mut newlines = 0u32;

View file

@ -1,13 +1,14 @@
use crate::comments::{dangling_node_comments, SourceComment};
use ruff_formatter::prelude::{space, text};
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprLambda;
use crate::comments::{dangling_comments, SourceComment};
use crate::context::PyFormatContext;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::other::parameters::ParametersParentheses;
use crate::AsFormat;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::prelude::{space, text};
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprLambda;
#[derive(Default)]
pub struct FormatExprLambda;
@ -20,13 +21,12 @@ impl FormatNodeRule<ExprLambda> for FormatExprLambda {
body,
} = item;
let comments = f.context().comments().clone();
let dangling = comments.dangling(item);
write!(f, [text("lambda")])?;
if !parameters.args.is_empty()
|| !parameters.posonlyargs.is_empty()
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
{
if let Some(parameters) = parameters {
write!(
f,
[
@ -38,21 +38,15 @@ impl FormatNodeRule<ExprLambda> for FormatExprLambda {
)?;
}
write!(
f,
[
text(":"),
space(),
body.format(),
// It's possible for some `Arguments` of `lambda`s to be assigned dangling comments.
//
// a = (
// lambda # Dangling
// : 1
// )
dangling_node_comments(parameters.as_ref())
]
)
write!(f, [text(":")])?;
if dangling.is_empty() {
write!(f, [space()])?;
} else {
write!(f, [dangling_comments(dangling)])?;
}
write!(f, [body.format()])
}
fn fmt_dangling_comments(

View file

@ -7,8 +7,8 @@ use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{TextRange, TextSize};
use crate::comments::{
dangling_open_parenthesis_comments, leading_comments, leading_node_comments, trailing_comments,
CommentLinePosition, SourceComment,
dangling_comments, dangling_open_parenthesis_comments, leading_comments, leading_node_comments,
trailing_comments, CommentLinePosition, SourceComment,
};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::parentheses::empty_parenthesized;
@ -242,7 +242,7 @@ impl FormatNodeRule<Parameters> for FormatParameters {
+ usize::from(kwarg.is_some());
if self.parentheses == ParametersParentheses::Never {
write!(f, [group(&format_inner)])
write!(f, [group(&format_inner), dangling_comments(dangling)])
} else if num_parameters == 0 {
// No parameters, format any dangling comments between `()`
write!(f, [empty_parenthesized("(", dangling, ")")])

View file

@ -62,12 +62,16 @@ lambda x: lambda y: lambda z: (
z) # Trailing
# Trailing
a = (
lambda # Dangling
: 1
)
a = (
lambda x # Dangling
, y: 1
)
# Regression test: lambda empty arguments ranges were too long, leading to unstable
# formatting
(lambda:(#
@ -99,6 +103,27 @@ lambda a, *args, b, **kwds,: 0
lambda a, *, b, **kwds,: 0
lambda a, /: a
lambda a, /, c: a
# Dangling comments without parameters.
(
lambda
: # 3
None
)
(
lambda
# 3
: None
)
(
lambda # 1
# 2
: # 3
# 4
None # 5
)
```
## Output
@ -159,9 +184,14 @@ lambda x: lambda y: lambda z: (
) # Trailing
# Trailing
a = (
lambda: # Dangling
1
)
a = (
lambda: 1 # Dangling
lambda x, # Dangling
y: 1
)
# Regression test: lambda empty arguments ranges were too long, leading to unstable
@ -188,7 +218,7 @@ lambda **kwds,: 0
lambda a, *args,: 0
lambda a, **kwds,: 0
lambda *args, b,: 0
lambda: 0
lambda *, b,: 0
lambda *args, **kwds,: 0
lambda a, *args, b,: 0
lambda a, *, b,: 0
@ -199,6 +229,25 @@ lambda a, *args, b, **kwds,: 0
lambda a, *, b, **kwds,: 0
lambda a, /: a
lambda a, /, c: a
# Dangling comments without parameters.
(
lambda: # 3
None
)
(
lambda: # 3
None
)
(
lambda: # 1
# 2
# 3
# 4
None # 5
)
```