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 3a07d72
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 24 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
55 changes: 41 additions & 14 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ 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,
find_only_token_in_range, first_non_trivia_token, indentation_at_offset, BackwardsTokenizer,
CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer,
};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextLen, TextRange};
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,41 @@ 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 = first_non_trivia_token(parameter.name.end(), locator.contents()).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
8 changes: 3 additions & 5 deletions crates/ruff_python_trivia/src/tokenizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use crate::{is_python_whitespace, Cursor};

/// Searches for the first non-trivia character in `range`.
/// Searches for the first non-trivia character after `offset`.
///
/// The search skips over any whitespace and comments.
///
/// Returns `Some` if the range contains any non-trivia character. The first item is the absolute offset
/// of the character, the second item the non-trivia character.
///
/// Returns `None` if the range is empty or only contains trivia (whitespace or comments).
/// Returns `Some` if the source code after `offset` contains any non-trivia character.///
/// Returns `None` if the text after `offset` is empty or only contains trivia (whitespace or comments).
pub fn first_non_trivia_token(offset: TextSize, code: &str) -> Option<SimpleToken> {
SimpleTokenizer::starts_at(offset, code)
.skip_trivia()
Expand Down

0 comments on commit 3a07d72

Please sign in to comment.