From 55d6fd53cde7579a8961bef13333e6692f9aa3fd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 8 Aug 2023 16:48:38 -0400 Subject: [PATCH] Treat comments on open parentheses in return annotations as dangling (#6413) ## Summary Given: ```python def double(a: int) -> ( # Hello int ): return 2*a ``` We currently treat `# Hello` as a trailing comment on the parameters (`(a: int)`). This PR adds a placement method to instead treat it as a dangling comment on the function definition itself, so that it gets formatted at the end of the definition, like: ```python def double(a: int) -> int: # Hello return 2*a ``` The formatting in this case is unchanged, but it's incorrect IMO for that to be a trailing comment on the parameters, and that placement leads to an instability after changing the grouping in #6410. Fixing this led to a _different_ instability related to tuple return type annotations, like: ```python def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] ): ... ``` (This is a real example.) To fix, I had to special-case tuples in that spot, though I'm not certain that's correct. --- .../test/fixtures/ruff/statement/function.py | 38 ++++++ .../src/comments/placement.rs | 39 +++++- .../src/statement/stmt_function_def.rs | 25 ++-- .../format@statement__function.py.snap | 121 ++++++++++++++++++ 4 files changed, 211 insertions(+), 12 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py index 42e643032c..e59547d6ce 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py @@ -371,3 +371,41 @@ def f( # first # third ): ... + +# Handle comments on empty tuple return types. +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + # comment +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + 1 +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + 1, 2 +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + (1, 2) +): ... + +def handleMatch( # type: ignore[override] # https://github.com/python/mypy/issues/10197 + self, m: Match[str], data: str +) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]: + ... + +def double(a: int # Hello +) -> (int): + return 2 * a + +def double(a: int) -> ( # Hello + int +): + return 2*a + +def double(a: int) -> ( # Hello +): + return 2*a diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 1385b8ffa5..113b48ee93 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -67,7 +67,10 @@ pub(super) fn place_comment<'a>( handle_module_level_own_line_comment_before_class_or_function_comment(comment, locator) } AnyNodeRef::WithItem(_) => handle_with_item_comment(comment, locator), - AnyNodeRef::StmtFunctionDef(_) => handle_leading_function_with_decorators_comment(comment), + AnyNodeRef::StmtFunctionDef(function_def) => { + handle_leading_function_with_decorators_comment(comment) + .or_else(|comment| handle_leading_returns_comment(comment, function_def)) + } AnyNodeRef::StmtClassDef(class_def) => { handle_leading_class_with_decorators_comment(comment, class_def) } @@ -783,6 +786,40 @@ fn handle_leading_function_with_decorators_comment(comment: DecoratedComment) -> } } +/// Handles end-of-line comments between function parameters and the return type annotation, +/// attaching them as dangling comments to the function instead of making them trailing +/// parameter comments. +/// +/// ```python +/// def double(a: int) -> ( # Hello +/// int +/// ): +/// return 2*a +/// ``` +fn handle_leading_returns_comment<'a>( + comment: DecoratedComment<'a>, + function_def: &'a ast::StmtFunctionDef, +) -> CommentPlacement<'a> { + let parameters = function_def.parameters.as_ref(); + let Some(returns) = function_def.returns.as_deref() else { + return CommentPlacement::Default(comment); + }; + + let is_preceding_parameters = comment + .preceding_node() + .is_some_and(|node| node == parameters.into()); + + let is_following_returns = comment + .following_node() + .is_some_and(|node| node == returns.into()); + + if comment.line_position().is_end_of_line() && is_preceding_parameters && is_following_returns { + CommentPlacement::dangling(comment.enclosing_node(), comment) + } else { + CommentPlacement::Default(comment) + } +} + /// Handle comments between decorators and the decorated node. /// /// For example, given: diff --git a/crates/ruff_python_formatter/src/statement/stmt_function_def.rs b/crates/ruff_python_formatter/src/statement/stmt_function_def.rs index 80a8da56ca..4533a6012c 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_function_def.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_function_def.rs @@ -61,17 +61,20 @@ impl FormatNodeRule for FormatStmtFunctionDef { write!(f, [item.parameters.format()])?; if let Some(return_annotation) = item.returns.as_ref() { - write!( - f, - [ - space(), - text("->"), - space(), - optional_parentheses( - &return_annotation.format().with_options(Parentheses::Never) - ) - ] - )?; + write!(f, [space(), text("->"), space()])?; + if return_annotation.is_tuple_expr() { + write!( + f, + [return_annotation.format().with_options(Parentheses::Never)] + )?; + } else { + write!( + f, + [optional_parentheses( + &return_annotation.format().with_options(Parentheses::Never), + )] + )?; + } } write!( diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 8fba525759..7868120be5 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap @@ -377,6 +377,44 @@ def f( # first # third ): ... + +# Handle comments on empty tuple return types. +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + # comment +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + 1 +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + 1, 2 +): ... + +def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override] + (1, 2) +): ... + +def handleMatch( # type: ignore[override] # https://github.com/python/mypy/issues/10197 + self, m: Match[str], data: str +) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]: + ... + +def double(a: int # Hello +) -> (int): + return 2 * a + +def double(a: int) -> ( # Hello + int +): + return 2*a + +def double(a: int) -> ( # Hello +): + return 2*a ``` ## Output @@ -905,6 +943,89 @@ def f( # first /, # second ): ... + + +# Handle comments on empty tuple return types. +def zrevrangebylex( + self, + name: _Key, + max: _Value, + min: _Value, + start: int | None = None, + num: int | None = None, +) -> ( # type: ignore[override] +): + ... + + +def zrevrangebylex( + self, + name: _Key, + max: _Value, + min: _Value, + start: int | None = None, + num: int | None = None, +) -> ( # type: ignore[override] + # comment +): + ... + + +def zrevrangebylex( + self, + name: _Key, + max: _Value, + min: _Value, + start: int | None = None, + num: int | None = None, +) -> 1: # type: ignore[override] + ... + + +def zrevrangebylex( + self, + name: _Key, + max: _Value, + min: _Value, + start: int | None = None, + num: int | None = None, +) -> ( # type: ignore[override] + 1, + 2, +): + ... + + +def zrevrangebylex( + self, + name: _Key, + max: _Value, + min: _Value, + start: int | None = None, + num: int | None = None, +) -> (1, 2): # type: ignore[override] + ... + + +def handleMatch( # type: ignore[override] # https://github.com/python/mypy/issues/10197 + self, m: Match[str], data: str +) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]: + ... + + +def double( + a: int, # Hello +) -> int: + return 2 * a + + +def double(a: int) -> int: # Hello + return 2 * a + + +def double(a: int) -> ( # Hello +): + return 2 * a ```