Treat parameters-with-newline as empty in function formatting (#7550)

## Summary

If a function has no parameters (and no comments within the parameters'
`()`), we're supposed to wrap the return annotation _whenever_ it
breaks. However, our `empty_parameters` test didn't properly account for
the case in which the parameters include a newline (but no other
content), like:

```python
def get_dashboards_hierarchy(
) -> Dict[Type['BaseDashboard'], List[Type['BaseDashboard']]]:
    """Get hierarchy of dashboards classes.

    Returns:
        Dict of dashboards classes.
    """
    dashboards_hierarchy = {}
```

This PR fixes that detection. Instead of lexing, it now checks if the
parameters itself is empty (or if it contains comments).

Closes https://github.com/astral-sh/ruff/issues/7457.
This commit is contained in:
Charlie Marsh 2023-09-20 16:20:22 -04:00 committed by GitHub
parent 6540321966
commit 5df0326bc8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 50 additions and 44 deletions

View file

@ -2135,6 +2135,15 @@ impl Parameters {
}
false
}
/// Returns `true` if the [`Parameters`] is empty.
pub fn is_empty(&self) -> bool {
self.posonlyargs.is_empty()
&& self.args.is_empty()
&& self.kwonlyargs.is_empty()
&& self.vararg.is_none()
&& self.kwarg.is_none()
}
}
/// An alternative type of AST `arg`. This is used for each function argument that might have a default value.

View file

@ -75,6 +75,12 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
]:
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]

View file

@ -1,9 +1,7 @@
use crate::comments::format::empty_lines_before_trailing_comments;
use ruff_formatter::write;
use ruff_python_ast::{Parameters, StmtFunctionDef};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::Ranged;
use ruff_python_ast::StmtFunctionDef;
use crate::comments::format::empty_lines_before_trailing_comments;
use crate::comments::SourceComment;
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::{Parentheses, Parenthesize};
@ -147,29 +145,30 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
[return_annotation.format().with_options(Parentheses::Always)]
)?;
} else {
let parenthesize = if parameters.is_empty() && !comments.has(parameters.as_ref()) {
// If the parameters are empty, add parentheses if the return annotation
// breaks at all.
Parenthesize::IfBreaksOrIfRequired
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
// ```python
// def f(
// x,
// ) -> Tuple[
// int,
// int,
// ]:
// ...
// ```
Parenthesize::IfBreaks
};
write!(
f,
[maybe_parenthesize_expression(
return_annotation,
item,
if empty_parameters(parameters, f.context().source()) {
// If the parameters are empty, add parentheses if the return annotation
// breaks at all.
Parenthesize::IfBreaksOrIfRequired
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
// ```python
// def f(
// x,
// ) -> Tuple[
// int,
// int,
// ]:
// ...
// ```
Parenthesize::IfBreaks
},
parenthesize
)]
)?;
}
@ -179,25 +178,3 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
group(&format_inner).fmt(f)
}
/// Returns `true` if [`Parameters`] is empty (no parameters, no comments, etc.).
fn empty_parameters(parameters: &Parameters, source: &str) -> bool {
let mut tokenizer = SimpleTokenizer::new(source, parameters.range())
.filter(|token| !matches!(token.kind, SimpleTokenKind::Whitespace));
let Some(lpar) = tokenizer.next() else {
return false;
};
if !matches!(lpar.kind, SimpleTokenKind::LParen) {
return false;
}
let Some(rpar) = tokenizer.next() else {
return false;
};
if !matches!(rpar.kind, SimpleTokenKind::RParen) {
return false;
}
true
}

View file

@ -81,6 +81,12 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
]:
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]
@ -365,6 +371,14 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]
):
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
x
) -> Set[