Skip to content

Commit

Permalink
Correctly handle other parameter comment placements
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Sep 17, 2024
1 parent 12c11f9 commit 869807f
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -528,3 +528,38 @@ def kwargs_type_annotation(**
# comment
kwargs: int): pass


def args_many_comments(
# before
*
# between * and name
args # trailing args
# after name
): pass


def args_many_comments_with_type_annotation(
# before
*
# between * and name
args # trailing args
# before colon
: # after colon
# before type
int # trailing type
# after type
): pass



def args_with_type_annotations_no_after_colon_comment(
# before
*
# between * and name
args # trailing args
# before colon
:
# before type
int # trailing type
# after type
): pass
52 changes: 40 additions & 12 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use std::cmp::Ordering;

use ast::helpers::comment_indentation_after;
use ruff_python_ast::whitespace::indentation;
use ruff_python_ast::{self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameters};
use ruff_python_ast::{
self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters,
};
use ruff_python_trivia::{
find_only_token_in_range, indentation_at_offset, BackwardsTokenizer, CommentRanges,
SimpleToken, SimpleTokenKind, SimpleTokenizer,
Expand Down Expand Up @@ -202,17 +204,7 @@ fn handle_enclosed_comment<'a>(
}
})
}
AnyNodeRef::Parameter(parameter) => {
let is_arg_or_kwarg = locator.slice(parameter.range()).starts_with('*');

// A comment between the `*` or `**` and the parameter name.
// Move the comment before the node.
if is_arg_or_kwarg && comment.preceding_node().is_none() {
CommentPlacement::leading(parameter, comment)
} else {
CommentPlacement::Default(comment)
}
}
AnyNodeRef::Parameter(parameter) => handle_parameter_comment(comment, parameter, locator),
AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) | AnyNodeRef::PatternArguments(_) => {
handle_bracketed_end_of_line_comment(comment, locator)
}
Expand Down Expand Up @@ -763,6 +755,42 @@ fn handle_parameters_separator_comment<'a>(
CommentPlacement::Default(comment)
}

/// Associate comments that come before the `:` starting the type annotation or before the
/// parameter's name for unannotated parameters as leading parameter-comments.
///
/// The parameter's name isn't a node to which comments can be associated.
/// That's why we pull out all comments that come before the expression name or the type annotation
/// and make them leading parameter comments. For example:
/// * `* # comment\nargs`
/// * `arg # comment\n : int`
///
/// Associate comments with the type annotation when possible.
fn handle_parameter_comment<'a>(
comment: DecoratedComment<'a>,
parameter: &'a Parameter,
locator: &Locator,
) -> CommentPlacement<'a> {
if parameter.annotation.as_deref().is_some() {
let colon = SimpleTokenizer::starts_at(parameter.name.end(), locator.contents())
.skip_trivia()
.next()
.expect("A annotated parameter should have a colon following its name when it is valid syntax.");

assert_eq!(colon.kind(), SimpleTokenKind::Colon);

if comment.start() < colon.start() {
// The comment is before the colon, pull it out and make it a leading comment of the parameter.
CommentPlacement::leading(parameter, comment)
} else {
CommentPlacement::Default(comment)
}
} else if comment.start() < parameter.name.start() {
CommentPlacement::leading(parameter, comment)
} else {
CommentPlacement::Default(comment)
}
}

/// Handles comments between the left side and the operator of a binary expression (trailing comments of the left),
/// and trailing end-of-line comments that are on the same line as the operator.
///
Expand Down
23 changes: 18 additions & 5 deletions crates/ruff_python_formatter/src/other/parameter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use ruff_formatter::write;
use ruff_python_ast::Parameter;

use crate::expression::parentheses::is_expression_parenthesized;
use crate::prelude::*;
use ruff_python_ast::Parameter;

#[derive(Default)]
pub struct FormatParameter;
Expand All @@ -16,8 +15,22 @@ impl FormatNodeRule<Parameter> for FormatParameter {

name.format().fmt(f)?;

if let Some(annotation) = annotation {
write!(f, [token(":"), space(), annotation.format()])?;
if let Some(annotation) = annotation.as_deref() {
token(":").fmt(f)?;

if f.context().comments().has_leading(annotation)
&& !is_expression_parenthesized(
annotation.into(),
f.context().comments().ranges(),
f.context().source(),
)
{
hard_line_break().fmt(f)?;
} else {
space().fmt(f)?;
}

annotation.format().fmt(f)?;
}

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,41 @@ def kwargs_type_annotation(**
# comment
kwargs: int): pass


def args_many_comments(
# before
*
# between * and name
args # trailing args
# after name
): pass


def args_many_comments_with_type_annotation(
# before
*
# between * and name
args # trailing args
# before colon
: # after colon
# before type
int # trailing type
# after type
): pass



def args_with_type_annotations_no_after_colon_comment(
# before
*
# between * and name
args # trailing args
# before colon
:
# before type
int # trailing type
# after type
): pass
```

## Output
Expand Down Expand Up @@ -1247,6 +1282,42 @@ def kwargs_type_annotation(
**kwargs: int,
):
pass


def args_many_comments(
# before
# between * and name
*args, # trailing args
# after name
):
pass


def args_many_comments_with_type_annotation(
# before
# between * and name
# trailing args
# before colon
*args:
# after colon
# before type
int, # trailing type
# after type
):
pass


def args_with_type_annotations_no_after_colon_comment(
# before
# between * and name
# trailing args
# before colon
*args:
# before type
int, # trailing type
# after type
):
pass
```


Expand Down

0 comments on commit 869807f

Please # to comment.