language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-19 01:51:30 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Fix parentheses around return type annotations (#13381)

Browse source
This commit is contained in:
Micha Reiser 2024-09-20 09:23:53 +02:00 • committed by GitHub
parent 7c2011599f
commit 531ebf6dff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 1487 additions and 296 deletions
Download patch file Download diff file Expand all files Collapse all files

31
crates/ruff_python_formatter/src/expression/expr_subscript.rs
View file

@ -8,6 +8,7 @@ use crate::expression::parentheses::{
};
use crate::expression::CallChainLayout;
use crate::prelude::*;
use crate::preview::is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled;
#[derive(Default)]
pub struct FormatExprSubscript {
@ -103,19 +104,25 @@ impl NeedsParentheses for ExprSubscript {
} else {
match self.value.needs_parentheses(self.into(), context) {
OptionalParentheses::BestFit => {
if parent.as_stmt_function_def().is_some_and(|function_def| {
function_def
.returns
.as_deref()
.and_then(Expr::as_subscript_expr)
== Some(self)
}) {
// Don't use the best fitting layout for return type annotation because it results in the
// return type expanding before the parameters.
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit
if let Some(function) = parent.as_stmt_function_def() {
if function.returns.as_deref().is_some_and(|returns| {
AnyNodeRef::ptr_eq(returns.into(), self.into())
}) {
if is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(context) &&
function.parameters.is_empty() && !context.comments().has(&*function.parameters) {
// Apply the `optional_parentheses` layout when the subscript
// is in a return type position of a function without parameters.
// This ensures the subscript is parenthesized if it has a very
// long name that goes over the line length limit.
return OptionalParentheses::Multiline
}
// Don't use the best fitting layout for return type annotation because it results in the
// return type expanding before the parameters.
return OptionalParentheses::Never;
}
}
OptionalParentheses::BestFit
}
parentheses => parentheses,
}

38
crates/ruff_python_formatter/src/expression/mod.rs
View file

@ -19,7 +19,10 @@ use crate::expression::parentheses::{
OptionalParentheses, Parentheses, Parenthesize,
};
use crate::prelude::*;
use crate::preview::is_hug_parens_with_braces_and_square_brackets_enabled;
use crate::preview::{
is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled,
is_hug_parens_with_braces_and_square_brackets_enabled,
};
mod binary_like;
pub(crate) mod expr_attribute;
@ -324,7 +327,7 @@ fn format_with_parentheses_comments(
)
}
/// Wraps an expression in an optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation
/// Wraps an expression in optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation
/// indicates that it is okay to omit the parentheses. For example, parentheses can always be omitted for lists,
/// because they already bring their own parentheses.
pub(crate) fn maybe_parenthesize_expression<'a, T>(
@ -382,23 +385,38 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
OptionalParentheses::Always => OptionalParentheses::Always,
// The reason to add parentheses is to avoid a syntax error when breaking an expression over multiple lines.
// Therefore, it is unnecessary to add an additional pair of parentheses if an outer expression
// is parenthesized.
_ if f.context().node_level().is_parenthesized() => OptionalParentheses::Never,
// is parenthesized. Unless, it's the `Parenthesize::IfBreaksParenthesizedNested` layout
// where parenthesizing nested `maybe_parenthesized_expression` is explicitly desired.
_ if f.context().node_level().is_parenthesized() => {
if !is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(
f.context(),
) {
OptionalParentheses::Never
} else if matches!(parenthesize, Parenthesize::IfBreaksParenthesizedNested) {
return parenthesize_if_expands(
&expression.format().with_options(Parentheses::Never),
)
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f);
} else {
return expression.format().with_options(Parentheses::Never).fmt(f);
}
}
needs_parentheses => needs_parentheses,
};
match needs_parentheses {
OptionalParentheses::Multiline => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested if !is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(f.context()) => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.fmt(f)
}
Parenthesize::IfRequired => {
expression.format().with_options(Parentheses::Never).fmt(f)
}
Parenthesize::Optional | Parenthesize::IfBreaks => {
Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested => {
if can_omit_optional_parentheses(expression, f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
@ -411,7 +429,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
}
},
OptionalParentheses::BestFit => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.fmt(f)
}
@ -435,13 +453,13 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
}
},
OptionalParentheses::Never => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested if !is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(f.context()) => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f)
}
Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfRequired => {
Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfRequired | Parenthesize::IfBreaksParenthesized | Parenthesize::IfBreaksParenthesizedNested => {
expression.format().with_options(Parentheses::Never).fmt(f)
}
},

53
crates/ruff_python_formatter/src/expression/parentheses.rs
View file

@ -56,10 +56,15 @@ pub(crate) enum Parenthesize {
/// Adding parentheses is desired to prevent the comments from wandering.
IfRequired,
/// Parenthesizes the expression if the group doesn't fit on a line (e.g., even name expressions are parenthesized), or if
/// the expression doesn't break, but _does_ reports that it always requires parentheses in this position (e.g., walrus
/// operators in function return annotations).
IfBreaksOrIfRequired,
/// Same as [`Self::IfBreaks`] except that it uses [`parenthesize_if_expands`] for expressions
/// with the layout [`NeedsParentheses::BestFit`] which is used by non-splittable
/// expressions like literals, name, and strings.
IfBreaksParenthesized,
/// Same as [`Self::IfBreaksParenthesized`] but uses [`parenthesize_if_expands`] for nested
/// [`maybe_parenthesized_expression`] calls unlike other layouts that always omit parentheses
/// when outer parentheses are present.
IfBreaksParenthesizedNested,
}
impl Parenthesize {
@ -416,27 +421,25 @@ impl Format<PyFormatContext<'_>> for FormatEmptyParenthesized<'_> {
debug_assert!(self.comments[end_of_line_split..]
.iter()
.all(|comment| comment.line_position().is_own_line()));
write!(
f,
[group(&format_args![
token(self.left),
// end-of-line comments
trailing_comments(&self.comments[..end_of_line_split]),
// Avoid unstable formatting with
// ```python
// x = () - (#
// )
// ```
// Without this the comment would go after the empty tuple first, but still expand
// the bin op. In the second formatting pass they are trailing bin op comments
// so the bin op collapse. Suboptimally we keep parentheses around the bin op in
// either case.
(!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()),
// own line comments, which need to be indented
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
token(self.right)
])]
)
group(&format_args![
token(self.left),
// end-of-line comments
trailing_comments(&self.comments[..end_of_line_split]),
// Avoid unstable formatting with
// ```python
// x = () - (#
// )
// ```
// Without this the comment would go after the empty tuple first, but still expand
// the bin op. In the second formatting pass they are trailing bin op comments
// so the bin op collapse. Suboptimally we keep parentheses around the bin op in
// either case.
(!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()),
// own line comments, which need to be indented
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
token(self.right)
])
.fmt(f)
}
}

2
crates/ruff_python_formatter/src/other/with_item.rs
View file

@ -112,7 +112,7 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
maybe_parenthesize_expression(
context_expr,
item,
Parenthesize::IfBreaksOrIfRequired,
Parenthesize::IfBreaksParenthesizedNested,
)
.fmt(f)?;
} else {

7
crates/ruff_python_formatter/src/preview.rs
View file

@ -29,3 +29,10 @@ pub(crate) fn is_comprehension_leading_expression_comments_same_line_enabled(
) -> bool {
context.is_preview()
}
/// See [#9447](https://github.com/astral-sh/ruff/issues/9447)
pub(crate) fn is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(
context: &PyFormatContext,
) -> bool {
context.is_preview()
}

45
crates/ruff_python_formatter/src/statement/stmt_function_def.rs
View file

@ -1,6 +1,3 @@
use ruff_formatter::write;
use ruff_python_ast::{NodeKind, StmtFunctionDef};
use crate::comments::format::{
empty_lines_after_leading_comments, empty_lines_before_trailing_comments,
};
@ -10,6 +7,8 @@ use crate::prelude::*;
use crate::statement::clause::{clause_body, clause_header, ClauseHeader};
use crate::statement::stmt_class_def::FormatDecorators;
use crate::statement::suite::SuiteKind;
use ruff_formatter::write;
use ruff_python_ast::{NodeKind, StmtFunctionDef};
#[derive(Default)]
pub struct FormatStmtFunctionDef;
@ -112,23 +111,23 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
write!(f, [token("def"), space(), name.format()])?;
if let Some(type_params) = type_params.as_ref() {
write!(f, [type_params.format()])?;
type_params.format().fmt(f)?;
}
let format_inner = format_with(|f: &mut PyFormatter| {
write!(f, [parameters.format()])?;
parameters.format().fmt(f)?;
if let Some(return_annotation) = returns.as_ref() {
if let Some(return_annotation) = returns.as_deref() {
write!(f, [space(), token("->"), space()])?;
if return_annotation.is_tuple_expr() {
let parentheses = if comments.has_leading(return_annotation.as_ref()) {
let parentheses = if comments.has_leading(return_annotation) {
Parentheses::Always
} else {
Parentheses::Never
};
write!(f, [return_annotation.format().with_options(parentheses)])?;
} else if comments.has_trailing(return_annotation.as_ref()) {
return_annotation.format().with_options(parentheses).fmt(f)
} else if comments.has_trailing(return_annotation) {
// Intentionally parenthesize any return annotations with trailing comments.
// This avoids an instability in cases like:
// ```python
@ -156,15 +155,17 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
// requires that the parent be aware of how the child is formatted, which
// is challenging. As a compromise, we break those expressions to avoid an
// instability.
write!(
f,
[return_annotation.format().with_options(Parentheses::Always)]
)?;
return_annotation
.format()
.with_options(Parentheses::Always)
.fmt(f)
} 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
// If the parameters are empty, add parentheses around literal expressions
// (any non splitable expression) but avoid parenthesizing subscripts and
// other parenthesized expressions unless necessary.
Parenthesize::IfBreaksParenthesized
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
@ -179,17 +180,11 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
// ```
Parenthesize::IfBreaks
};
write!(
f,
[maybe_parenthesize_expression(
return_annotation,
item,
parenthesize
)]
)?;
maybe_parenthesize_expression(return_annotation, item, parenthesize).fmt(f)
}
} else {
Ok(())
}
Ok(())
});
group(&format_inner).fmt(f)