Skip to content

Commit

Permalink
Avoid searching for bracketed comments in unparenthesized generators (#…
Browse files Browse the repository at this point in the history
…7627)

Similar to tuples, a generator _can_ be parenthesized or
unparenthesized. Only search for bracketed comments if it contains its
own parentheses.

Closes #7623.
  • Loading branch information
charliermarsh committed Sep 24, 2023
1 parent 1a22eae commit 865c898
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 7 deletions.
Expand Up @@ -52,3 +52,20 @@
aaaaaaaaaaaaaaaaaaaaa = (
o for o in self.registry.values if o.__class__ is not ModelAdmin
)

# Regression test for: https://github.com/astral-sh/ruff/issues/7623
tuple(
0 # comment
for x in y
)

tuple(
(0 # comment
for x in y)
)

tuple(
( # comment
0 for x in y
)
)
7 changes: 6 additions & 1 deletion crates/ruff_python_formatter/src/comments/placement.rs
Expand Up @@ -11,6 +11,7 @@ use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};

use crate::comments::visitor::{CommentPlacement, DecoratedComment};
use crate::expression::expr_generator_exp::is_generator_parenthesized;
use crate::expression::expr_slice::{assign_comment_in_slice, ExprSliceCommentSection};
use crate::expression::expr_tuple::is_tuple_parenthesized;
use crate::other::parameters::{
Expand Down Expand Up @@ -290,12 +291,16 @@ fn handle_enclosed_comment<'a>(
AnyNodeRef::ExprFString(fstring) => CommentPlacement::dangling(fstring, comment),
AnyNodeRef::ExprList(_)
| AnyNodeRef::ExprSet(_)
| AnyNodeRef::ExprGeneratorExp(_)
| AnyNodeRef::ExprListComp(_)
| AnyNodeRef::ExprSetComp(_) => handle_bracketed_end_of_line_comment(comment, locator),
AnyNodeRef::ExprTuple(tuple) if is_tuple_parenthesized(tuple, locator.contents()) => {
handle_bracketed_end_of_line_comment(comment, locator)
}
AnyNodeRef::ExprGeneratorExp(generator)
if is_generator_parenthesized(generator, locator.contents()) =>
{
handle_bracketed_end_of_line_comment(comment, locator)
}
_ => CommentPlacement::Default(comment),
}
}
Expand Down
Expand Up @@ -102,8 +102,9 @@ impl NeedsParentheses for ExprGeneratorExp {
}
}

fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> bool {
// / Count the number of open parentheses between the start of the tuple and the first element.
/// Return `true` if a generator is parenthesized in the source code.
pub(crate) fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> bool {
// Count the number of open parentheses between the start of the generator and the first element.
let open_parentheses_count = SimpleTokenizer::new(
source,
TextRange::new(generator.start(), generator.elt.start()),
Expand All @@ -115,7 +116,7 @@ fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> boo
return false;
}

// Count the number of parentheses between the end of the first element and its trailing comma.
// Count the number of parentheses between the end of the generator and its trailing comma.
let close_parentheses_count = SimpleTokenizer::new(
source,
TextRange::new(
Expand All @@ -130,7 +131,7 @@ fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> boo
.filter(|token| token.kind() == SimpleTokenKind::RParen)
.count();

// If the number of open parentheses is greater than the number of close parentheses, the generator
// is parenthesized.
// If the number of open parentheses is greater than the number of close parentheses, the
// generator is parenthesized.
open_parentheses_count > close_parentheses_count
}
2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/expression/expr_tuple.rs
Expand Up @@ -221,7 +221,7 @@ impl NeedsParentheses for ExprTuple {
}
}

/// Check if a tuple has already had parentheses in the input
/// Return `true` if a tuple is parenthesized in the source code.
pub(crate) fn is_tuple_parenthesized(tuple: &ExprTuple, source: &str) -> bool {
let Some(elt) = tuple.elts.first() else {
return false;
Expand Down
Expand Up @@ -58,6 +58,23 @@ a = (
aaaaaaaaaaaaaaaaaaaaa = (
o for o in self.registry.values if o.__class__ is not ModelAdmin
)
# Regression test for: https://github.com/astral-sh/ruff/issues/7623
tuple(
0 # comment
for x in y
)
tuple(
(0 # comment
for x in y)
)
tuple(
( # comment
0 for x in y
)
)
```

## Output
Expand Down Expand Up @@ -122,6 +139,25 @@ a = (
aaaaaaaaaaaaaaaaaaaaa = (
o for o in self.registry.values if o.__class__ is not ModelAdmin
)
# Regression test for: https://github.com/astral-sh/ruff/issues/7623
tuple(
0 # comment
for x in y
)
tuple(
(
0 # comment
for x in y
)
)
tuple(
( # comment
0 for x in y
)
)
```


Expand Down

0 comments on commit 865c898

Please sign in to comment.