Skip to content

Commit

Permalink
Use correct start location for class/function clause header (#7802)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes the bug where the formatter would panic if a class/function with
decorators had a suppression comment.

The fix is to use to correct start location to find the `async`/`def`/`class`
keyword when decorators are present which is the end of the last
decorator.

## Test Plan

Add test cases for the fix and update the snapshots.
  • Loading branch information
dhruvmanila authored Oct 4, 2023
1 parent 7b4fb4f commit a1509df
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,16 @@ class Test:
def test():
pass


# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
class Foo: # fmt: skip
pass


# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
def foo(): # fmt: skip
pass
12 changes: 10 additions & 2 deletions crates/ruff_python_formatter/src/statement/clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,23 @@ impl<'a> ClauseHeader<'a> {
fn first_keyword_range(self, source: &str) -> FormatResult<TextRange> {
match self {
ClauseHeader::Class(header) => {
find_keyword(header.start(), SimpleTokenKind::Class, source)
let start_position = header
.decorator_list
.last()
.map_or_else(|| header.start(), Ranged::end);
find_keyword(start_position, SimpleTokenKind::Class, source)
}
ClauseHeader::Function(header) => {
let start_position = header
.decorator_list
.last()
.map_or_else(|| header.start(), Ranged::end);
let keyword = if header.is_async {
SimpleTokenKind::Async
} else {
SimpleTokenKind::Def
};
find_keyword(header.start(), keyword, source)
find_keyword(start_position, keyword, source)
}
ClauseHeader::If(header) => find_keyword(header.start(), SimpleTokenKind::If, source),
ClauseHeader::ElifElse(ElifElseClause {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ class Test:
def test():
pass
# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
class Foo: # fmt: skip
pass
# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
def foo(): # fmt: skip
pass
```

## Output
Expand All @@ -43,6 +56,20 @@ class Test:
# leading class comment
def test():
pass
# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
class Foo: # fmt: skip
pass
# Regression test for https://github.com/astral-sh/ruff/issues/7735
@decorator1
@decorator2
def foo(): # fmt: skip
pass
```


Expand Down

0 comments on commit a1509df

Please sign in to comment.