From 9b845e449e11823cfbb52db86231538e664cca40 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Wed, 8 Jan 2025 07:28:49 +0000 Subject: [PATCH 1/5] [`pyupgrade`] Handle comments and multiline expressions correctly (`UP037`) --- .../test/fixtures/pyupgrade/UP037_2.pyi | 19 ++++ crates/ruff_linter/src/rules/pyupgrade/mod.rs | 1 + .../pyupgrade/rules/quoted_annotation.rs | 38 ++++++-- ..._rules__pyupgrade__tests__UP037_2.pyi.snap | 91 +++++++++++++++++++ 4 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi new file mode 100644 index 00000000000000..30df1ff0213965 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi @@ -0,0 +1,19 @@ +# https://github.com/astral-sh/ruff/issues/7102 + +def f(a: Foo['SingleLine # Comment']): ... + + +def f(a: Foo['''Bar[ + Multi | + Line]''']): ... + + +def f(a: Foo['''Bar[ + Multi | + Line # Comment +]''']): ... + + +def f(a: Foo['''Bar[ + Multi | + Line] # Comment''']): ... diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 95f02be7da7d4b..315cafa5a7b1ca 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -58,6 +58,7 @@ mod tests { #[test_case(Rule::PrintfStringFormatting, Path::new("UP031_1.py"))] #[test_case(Rule::QuotedAnnotation, Path::new("UP037_0.py"))] #[test_case(Rule::QuotedAnnotation, Path::new("UP037_1.py"))] + #[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))] #[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"))] #[test_case(Rule::RedundantOpenModes, Path::new("UP015_1.py"))] #[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs index 7ee7cee03a919f..0386521a0b513c 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs @@ -1,9 +1,10 @@ -use ruff_text_size::TextRange; +use ruff_text_size::{TextRange, TextSize}; +use crate::checkers::ast::Checker; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; - -use crate::checkers::ast::Checker; +use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; +use ruff_source_file::LineRanges; /// ## What it does /// Checks for the presence of unnecessary quotes in type annotations. @@ -73,10 +74,29 @@ impl AlwaysFixableViolation for QuotedAnnotation { /// UP037 pub(crate) fn quoted_annotation(checker: &mut Checker, annotation: &str, range: TextRange) { - let mut diagnostic = Diagnostic::new(QuotedAnnotation, range); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - annotation.to_string(), - range, - ))); - checker.diagnostics.push(diagnostic); + let diagnostic = Diagnostic::new(QuotedAnnotation, range); + + let len = TextSize::try_from(annotation.len()).unwrap(); + let placeholder_range = TextRange::up_to(len); + let spans_multiple_lines = annotation.count_lines(placeholder_range) > 1; + + let tokenizer = SimpleTokenizer::new(annotation, placeholder_range); + let last_token_is_comment = matches!( + tokenizer.last(), + Some(SimpleToken { + kind: SimpleTokenKind::Comment, + .. + }) + ); + + let new_content = match (spans_multiple_lines, last_token_is_comment) { + (false, false) => format!("{annotation}"), + (true, false) => format!("({annotation})"), + (false, true) => format!("({annotation}\n)"), + (true, true) => format!("({annotation}\n)"), + }; + let edit = Edit::range_replacement(new_content, range); + let fix = Fix::safe_edit(edit); + + checker.diagnostics.push(diagnostic.with_fix(fix)); } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap new file mode 100644 index 00000000000000..81426b82e5f0c6 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap @@ -0,0 +1,91 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +snapshot_kind: text +--- +UP037_2.pyi:3:14: UP037 [*] Remove quotes from type annotation + | +1 | # https://github.com/astral-sh/ruff/issues/7102 +2 | +3 | def f(a: Foo['SingleLine # Comment']): ... + | ^^^^^^^^^^^^^^^^^^^^^^^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +1 1 | # https://github.com/astral-sh/ruff/issues/7102 +2 2 | +3 |-def f(a: Foo['SingleLine # Comment']): ... + 3 |+def f(a: Foo[(SingleLine # Comment + 4 |+)]): ... +4 5 | +5 6 | +6 7 | def f(a: Foo['''Bar[ + +UP037_2.pyi:6:14: UP037 [*] Remove quotes from type annotation + | +6 | def f(a: Foo['''Bar[ + | ______________^ +7 | | Multi | +8 | | Line]''']): ... + | |____________^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +3 3 | def f(a: Foo['SingleLine # Comment']): ... +4 4 | +5 5 | +6 |-def f(a: Foo['''Bar[ + 6 |+def f(a: Foo[(Bar[ +7 7 | Multi | +8 |- Line]''']): ... + 8 |+ Line])]): ... +9 9 | +10 10 | +11 11 | def f(a: Foo['''Bar[ + +UP037_2.pyi:11:14: UP037 [*] Remove quotes from type annotation + | +11 | def f(a: Foo['''Bar[ + | ______________^ +12 | | Multi | +13 | | Line # Comment +14 | | ]''']): ... + | |____^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +8 8 | Line]''']): ... +9 9 | +10 10 | +11 |-def f(a: Foo['''Bar[ + 11 |+def f(a: Foo[(Bar[ +12 12 | Multi | +13 13 | Line # Comment +14 |-]''']): ... + 14 |+])]): ... +15 15 | +16 16 | +17 17 | def f(a: Foo['''Bar[ + +UP037_2.pyi:17:14: UP037 [*] Remove quotes from type annotation + | +17 | def f(a: Foo['''Bar[ + | ______________^ +18 | | Multi | +19 | | Line] # Comment''']): ... + | |_______________________^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +14 14 | ]''']): ... +15 15 | +16 16 | +17 |-def f(a: Foo['''Bar[ + 17 |+def f(a: Foo[(Bar[ +18 18 | Multi | +19 |- Line] # Comment''']): ... + 19 |+ Line] # Comment + 20 |+)]): ... From f41cc973d3a2573a51ddd17749e004a6c619d54c Mon Sep 17 00:00:00 2001 From: InSync Date: Wed, 8 Jan 2025 14:47:44 +0700 Subject: [PATCH 2/5] Clippy --- .../src/rules/pyupgrade/rules/quoted_annotation.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs index 0386521a0b513c..10694bba12308c 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs @@ -90,10 +90,9 @@ pub(crate) fn quoted_annotation(checker: &mut Checker, annotation: &str, range: ); let new_content = match (spans_multiple_lines, last_token_is_comment) { - (false, false) => format!("{annotation}"), + (false, false) => annotation.to_string(), (true, false) => format!("({annotation})"), - (false, true) => format!("({annotation}\n)"), - (true, true) => format!("({annotation}\n)"), + (_, true) => format!("({annotation}\n)"), }; let edit = Edit::range_replacement(new_content, range); let fix = Fix::safe_edit(edit); From 647db1f0b278d0c60e156998df69d4529fb428a4 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Wed, 8 Jan 2025 15:09:39 +0000 Subject: [PATCH 3/5] Per review --- .../test/fixtures/pyupgrade/UP037_2.pyi | 25 ++++++++++ .../pyupgrade/rules/quoted_annotation.rs | 7 ++- ..._rules__pyupgrade__tests__UP037_2.pyi.snap | 50 +++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi index 30df1ff0213965..8452db1efd9fd5 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi @@ -17,3 +17,28 @@ def f(a: Foo['''Bar[ def f(a: Foo['''Bar[ Multi | Line] # Comment''']): ... + + +def f(a: Foo[''' +Bar[ + Multi | + Line] # Comment''']): ... + + +a: '''\\ +list[int]''' = [42] + + +# TODO: These are valid too. String annotations are assumed to be enclosed in parentheses. +# https://github.com/astral-sh/ruff/issues/9467 + +def f(a: Foo[''' + Bar + [ + Multi | + Line + ] # Comment''']): ... + + +a: '''list +[int]''' = [42] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs index 10694bba12308c..597aeb238d7d6f 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs @@ -1,4 +1,4 @@ -use ruff_text_size::{TextRange, TextSize}; +use ruff_text_size::{TextLen, TextRange}; use crate::checkers::ast::Checker; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; @@ -76,9 +76,8 @@ impl AlwaysFixableViolation for QuotedAnnotation { pub(crate) fn quoted_annotation(checker: &mut Checker, annotation: &str, range: TextRange) { let diagnostic = Diagnostic::new(QuotedAnnotation, range); - let len = TextSize::try_from(annotation.len()).unwrap(); - let placeholder_range = TextRange::up_to(len); - let spans_multiple_lines = annotation.count_lines(placeholder_range) > 1; + let placeholder_range = TextRange::up_to(annotation.text_len()); + let spans_multiple_lines = annotation.contains_line_break(placeholder_range); let tokenizer = SimpleTokenizer::new(annotation, placeholder_range); let last_token_is_comment = matches!( diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap index 81426b82e5f0c6..f099e31ca18ad3 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap @@ -89,3 +89,53 @@ UP037_2.pyi:17:14: UP037 [*] Remove quotes from type annotation 19 |- Line] # Comment''']): ... 19 |+ Line] # Comment 20 |+)]): ... +20 21 | +21 22 | +22 23 | def f(a: Foo[''' + +UP037_2.pyi:22:14: UP037 [*] Remove quotes from type annotation + | +22 | def f(a: Foo[''' + | ______________^ +23 | | Bar[ +24 | | Multi | +25 | | Line] # Comment''']): ... + | |_______________________^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +19 19 | Line] # Comment''']): ... +20 20 | +21 21 | +22 |-def f(a: Foo[''' + 22 |+def f(a: Foo[( +23 23 | Bar[ +24 24 | Multi | +25 |- Line] # Comment''']): ... + 25 |+ Line] # Comment + 26 |+)]): ... +26 27 | +27 28 | +28 29 | a: '''\\ + +UP037_2.pyi:28:4: UP037 [*] Remove quotes from type annotation + | +28 | a: '''\\ + | ____^ +29 | | list[int]''' = [42] + | |____________^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +25 25 | Line] # Comment''']): ... +26 26 | +27 27 | +28 |-a: '''\\ +29 |-list[int]''' = [42] + 28 |+a: (\ + 29 |+list[int]) = [42] +30 30 | +31 31 | +32 32 | # TODO: These are valid too. String annotations are assumed to be quoted. From 3637e3952eab4500a22dc2d1f2bbd6d1fdd229d3 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Wed, 8 Jan 2025 15:17:34 +0000 Subject: [PATCH 4/5] Fix --- .../ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap index f099e31ca18ad3..67a7a1fd730204 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap @@ -138,4 +138,4 @@ UP037_2.pyi:28:4: UP037 [*] Remove quotes from type annotation 29 |+list[int]) = [42] 30 30 | 31 31 | -32 32 | # TODO: These are valid too. String annotations are assumed to be quoted. +32 32 | # TODO: These are valid too. String annotations are assumed to be enclosed in parentheses. From b57f80dee2347d24df835c2ceabb14eadafc2760 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Thu, 9 Jan 2025 03:10:56 +0000 Subject: [PATCH 5/5] Per review --- .../test/fixtures/pyupgrade/UP037_2.pyi | 9 ++++ .../pyupgrade/rules/quoted_annotation.rs | 15 +++++- ..._rules__pyupgrade__tests__UP037_2.pyi.snap | 51 +++++++++++++------ 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi index 8452db1efd9fd5..8456637581f11b 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_2.pyi @@ -25,6 +25,10 @@ Bar[ Line] # Comment''']): ... +def f(a: '''list[int] + ''' = []): ... + + a: '''\\ list[int]''' = [42] @@ -32,6 +36,11 @@ list[int]''' = [42] # TODO: These are valid too. String annotations are assumed to be enclosed in parentheses. # https://github.com/astral-sh/ruff/issues/9467 +def f(a: ''' + list[int] + ''' = []): ... + + def f(a: Foo[''' Bar [ diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs index 597aeb238d7d6f..7924c6cada8c2f 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/quoted_annotation.rs @@ -1,8 +1,10 @@ -use ruff_text_size::{TextLen, TextRange}; +use ruff_text_size::{TextLen, TextRange, TextSize}; use crate::checkers::ast::Checker; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::Stmt; +use ruff_python_semantic::SemanticModel; use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_source_file::LineRanges; @@ -89,6 +91,9 @@ pub(crate) fn quoted_annotation(checker: &mut Checker, annotation: &str, range: ); let new_content = match (spans_multiple_lines, last_token_is_comment) { + (_, false) if in_parameter_annotation(range.start(), checker.semantic()) => { + annotation.to_string() + } (false, false) => annotation.to_string(), (true, false) => format!("({annotation})"), (_, true) => format!("({annotation}\n)"), @@ -98,3 +103,11 @@ pub(crate) fn quoted_annotation(checker: &mut Checker, annotation: &str, range: checker.diagnostics.push(diagnostic.with_fix(fix)); } + +fn in_parameter_annotation(offset: TextSize, semantic: &SemanticModel) -> bool { + let Stmt::FunctionDef(stmt) = semantic.current_statement() else { + return false; + }; + + stmt.parameters.range.contains(offset) +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap index 67a7a1fd730204..1519dba9d76025 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_2.pyi.snap @@ -36,10 +36,10 @@ UP037_2.pyi:6:14: UP037 [*] Remove quotes from type annotation 4 4 | 5 5 | 6 |-def f(a: Foo['''Bar[ - 6 |+def f(a: Foo[(Bar[ + 6 |+def f(a: Foo[Bar[ 7 7 | Multi | 8 |- Line]''']): ... - 8 |+ Line])]): ... + 8 |+ Line]]): ... 9 9 | 10 10 | 11 11 | def f(a: Foo['''Bar[ @@ -60,11 +60,11 @@ UP037_2.pyi:11:14: UP037 [*] Remove quotes from type annotation 9 9 | 10 10 | 11 |-def f(a: Foo['''Bar[ - 11 |+def f(a: Foo[(Bar[ + 11 |+def f(a: Foo[Bar[ 12 12 | Multi | 13 13 | Line # Comment 14 |-]''']): ... - 14 |+])]): ... + 14 |+]]): ... 15 15 | 16 16 | 17 17 | def f(a: Foo['''Bar[ @@ -117,14 +117,14 @@ UP037_2.pyi:22:14: UP037 [*] Remove quotes from type annotation 26 |+)]): ... 26 27 | 27 28 | -28 29 | a: '''\\ +28 29 | def f(a: '''list[int] -UP037_2.pyi:28:4: UP037 [*] Remove quotes from type annotation +UP037_2.pyi:28:10: UP037 [*] Remove quotes from type annotation | -28 | a: '''\\ - | ____^ -29 | | list[int]''' = [42] - | |____________^ UP037 +28 | def f(a: '''list[int] + | __________^ +29 | | ''' = []): ... + | |_______^ UP037 | = help: Remove quotes @@ -132,10 +132,31 @@ UP037_2.pyi:28:4: UP037 [*] Remove quotes from type annotation 25 25 | Line] # Comment''']): ... 26 26 | 27 27 | -28 |-a: '''\\ -29 |-list[int]''' = [42] - 28 |+a: (\ - 29 |+list[int]) = [42] +28 |-def f(a: '''list[int] +29 |- ''' = []): ... + 28 |+def f(a: list[int] + 29 |+ = []): ... +30 30 | +31 31 | +32 32 | a: '''\\ + +UP037_2.pyi:32:4: UP037 [*] Remove quotes from type annotation + | +32 | a: '''\\ + | ____^ +33 | | list[int]''' = [42] + | |____________^ UP037 + | + = help: Remove quotes + +ℹ Safe fix +29 29 | ''' = []): ... 30 30 | 31 31 | -32 32 | # TODO: These are valid too. String annotations are assumed to be enclosed in parentheses. +32 |-a: '''\\ +33 |-list[int]''' = [42] + 32 |+a: (\ + 33 |+list[int]) = [42] +34 34 | +35 35 | +36 36 | # TODO: These are valid too. String annotations are assumed to be enclosed in parentheses.