From 60ba7a7c0dbe90b981766d52d47358075ce0ff18 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 4 Jan 2024 19:39:37 -0500 Subject: [PATCH] Allow `# fmt: skip` with interspersed same-line comments (#9395) ## Summary This is similar to https://github.com/astral-sh/ruff/pull/8876, but more limited in scope: 1. It only applies to `# fmt: skip` (like Black). Like `# isort: on`, `# fmt: on` needs to be on its own line (still). 2. It only delimits on `#`, so you can do `# fmt: skip # noqa`, but not `# fmt: skip - some other content` or `# fmt: skip; noqa`. If we want to support the `;`-delimited version, we should revisit later, since we don't support that in the linter (so `# fmt: skip; noqa` wouldn't register a `noqa`). Closes https://github.com/astral-sh/ruff/issues/8892. --- .../test/fixtures/ruff/fmt_skip/reason.py | 9 +++++ .../ruff_python_formatter/src/comments/mod.rs | 33 +++++++++++++------ ...format_skip_with_multiple_comments.py.snap | 5 ++- .../snapshots/format@fmt_skip__reason.py.snap | 32 ++++++++++++++++++ 4 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/reason.py create mode 100644 crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__reason.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/reason.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/reason.py new file mode 100644 index 0000000000..393286cf09 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/reason.py @@ -0,0 +1,9 @@ +# Supported +x = 1 # fmt: skip +x = 1 # fmt: skip # reason +x = 1 # reason # fmt: skip + +# Unsupported +x = 1 # fmt: skip reason +x = 1 # fmt: skip - reason +x = 1 # fmt: skip; noqa diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index b5e738c2f7..6524a56a2b 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -171,24 +171,37 @@ impl SourceComment { pub(crate) fn suppression_kind(&self, source: &str) -> Option { let text = self.slice.text(SourceCode::new(source)); - let trimmed = text.strip_prefix('#').unwrap_or(text).trim_whitespace(); + // Match against `# fmt: on`, `# fmt: off`, `# yapf: disable`, and `# yapf: enable`, which + // must be on their own lines. + let trimmed = text.strip_prefix('#').unwrap_or(text).trim_whitespace(); if let Some(command) = trimmed.strip_prefix("fmt:") { match command.trim_whitespace_start() { - "off" => Some(SuppressionKind::Off), - "on" => Some(SuppressionKind::On), - "skip" => Some(SuppressionKind::Skip), - _ => None, + "off" => return Some(SuppressionKind::Off), + "on" => return Some(SuppressionKind::On), + "skip" => return Some(SuppressionKind::Skip), + _ => {} } } else if let Some(command) = trimmed.strip_prefix("yapf:") { match command.trim_whitespace_start() { - "disable" => Some(SuppressionKind::Off), - "enable" => Some(SuppressionKind::On), - _ => None, + "disable" => return Some(SuppressionKind::Off), + "enable" => return Some(SuppressionKind::On), + _ => {} } - } else { - None } + + // Search for `# fmt: skip` comments, which can be interspersed with other comments (e.g., + // `# fmt: skip # noqa: E501`). + for segment in text.split('#') { + let trimmed = segment.trim_whitespace(); + if let Some(command) = trimmed.strip_prefix("fmt:") { + if command.trim_whitespace_start() == "skip" { + return Some(SuppressionKind::Skip); + } + } + } + + None } /// Returns true if this comment is a `fmt: off` or `yapf: disable` own line suppression comment. diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_single_line_format_skip_with_multiple_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_single_line_format_skip_with_multiple_comments.py.snap index 78dc5bad0d..848e9fe3e2 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_single_line_format_skip_with_multiple_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_single_line_format_skip_with_multiple_comments.py.snap @@ -21,8 +21,7 @@ skip_will_not_work2 = "a" + "b" # some text; fmt:skip happens to be part of i --- Black +++ Ruff @@ -1,8 +1,8 @@ --foo = 123 # fmt: skip # noqa: E501 # pylint -+foo = 123 # fmt: skip # noqa: E501 # pylint + foo = 123 # fmt: skip # noqa: E501 # pylint bar = ( - 123 , - ( 1 + 5 ) # pylint # fmt:skip @@ -38,7 +37,7 @@ skip_will_not_work2 = "a" + "b" # some text; fmt:skip happens to be part of i ## Ruff Output ```python -foo = 123 # fmt: skip # noqa: E501 # pylint +foo = 123 # fmt: skip # noqa: E501 # pylint bar = ( 123, (1 + 5), # pylint # fmt:skip diff --git a/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__reason.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__reason.py.snap new file mode 100644 index 0000000000..296742a0ac --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__reason.py.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/reason.py +--- +## Input +```python +# Supported +x = 1 # fmt: skip +x = 1 # fmt: skip # reason +x = 1 # reason # fmt: skip + +# Unsupported +x = 1 # fmt: skip reason +x = 1 # fmt: skip - reason +x = 1 # fmt: skip; noqa +``` + +## Output +```python +# Supported +x = 1 # fmt: skip +x = 1 # fmt: skip # reason +x = 1 # reason # fmt: skip + +# Unsupported +x = 1 # fmt: skip reason +x = 1 # fmt: skip - reason +x = 1 # fmt: skip; noqa +``` + + +