From a1509dfc7c2b16f3762545fc802d49f5f03726e2 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 4 Oct 2023 13:25:01 +0530 Subject: [PATCH] Use correct start location for class/function clause header (#7802) ## 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. --- .../test/fixtures/ruff/fmt_skip/decorators.py | 13 +++++++++ .../src/statement/clause.rs | 12 +++++++-- .../format@fmt_skip__decorators.py.snap | 27 +++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/decorators.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/decorators.py index 98e3bedc6b94f..051eb28362a88 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/decorators.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/decorators.py @@ -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 diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 449b64e299644..b8138df503773 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -203,15 +203,23 @@ impl<'a> ClauseHeader<'a> { fn first_keyword_range(self, source: &str) -> FormatResult { 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 { diff --git a/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__decorators.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__decorators.py.snap index 330e73752d3f2..720890d63353e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__decorators.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__decorators.py.snap @@ -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 @@ -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 ```