From c1dc4a60bed4177c12e9718fc5c28d2c87327a4e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 19:53:26 -0500 Subject: [PATCH 1/8] Apply some minor changes to `unnecessary-list-index-lookup` (#8932) ## Summary I was late in reviewing this but found a few things I wanted to tweak. No functional changes. --- .../rules/unnecessary_list_index_lookup.rs | 340 +++++++++--------- ...1736_unnecessary_list_index_lookup.py.snap | 18 +- 2 files changed, 171 insertions(+), 187 deletions(-) diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index a668812ff0fd1a..17a42b29e25a26 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -4,15 +4,18 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_semantic::SemanticModel; use ruff_text_size::TextRange; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for access of a list item at the current index when using enumeration. +/// Checks for index-based list accesses during `enumerate` iterations. /// /// ## Why is this bad? -/// It is more succinct to use the variable for the value at the current index which is already in scope from the iterator. +/// When iterating over a list with `enumerate`, the current item is already +/// available alongside its index. Using the index to look up the item is +/// unnecessary. /// /// ## Example /// ```python @@ -39,10 +42,127 @@ impl AlwaysFixableViolation for UnnecessaryListIndexLookup { } fn fix_title(&self) -> String { - format!("Use existing item variable instead") + format!("Use existing variable") } } +/// PLR1736 +pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &StmtFor) { + let Some((sequence, index_name, value_name)) = + enumerate_items(&stmt_for.iter, &stmt_for.target, checker.semantic()) + else { + return; + }; + + let ranges = { + let mut visitor = SubscriptVisitor::new(sequence, index_name); + visitor.visit_body(&stmt_for.body); + visitor.visit_body(&stmt_for.orelse); + visitor.diagnostic_ranges + }; + + for range in ranges { + let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + value_name.to_string(), + range, + ))); + checker.diagnostics.push(diagnostic); + } +} + +/// PLR1736 +pub(crate) fn unnecessary_list_index_lookup_comprehension(checker: &mut Checker, expr: &Expr) { + let (Expr::GeneratorExp(ast::ExprGeneratorExp { + elt, generators, .. + }) + | Expr::DictComp(ast::ExprDictComp { + value: elt, + generators, + .. + }) + | Expr::SetComp(ast::ExprSetComp { + elt, generators, .. + }) + | Expr::ListComp(ast::ExprListComp { + elt, generators, .. + })) = expr + else { + return; + }; + + for comp in generators { + let Some((sequence, index_name, value_name)) = + enumerate_items(&comp.iter, &comp.target, checker.semantic()) + else { + return; + }; + + let ranges = { + let mut visitor = SubscriptVisitor::new(sequence, index_name); + visitor.visit_expr(elt.as_ref()); + visitor.diagnostic_ranges + }; + + for range in ranges { + let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + value_name.to_string(), + range, + ))); + checker.diagnostics.push(diagnostic); + } + } +} + +fn enumerate_items<'a>( + call_expr: &'a Expr, + tuple_expr: &'a Expr, + semantic: &SemanticModel, +) -> Option<(&'a str, &'a str, &'a str)> { + let ast::ExprCall { + func, arguments, .. + } = call_expr.as_call_expr()?; + + // Check that the function is the `enumerate` builtin. + if !semantic + .resolve_call_path(func.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["builtins" | "", "enumerate"])) + { + return None; + } + + let Expr::Tuple(ast::ExprTuple { elts, .. }) = tuple_expr else { + return None; + }; + let [index, value] = elts.as_slice() else { + return None; + }; + + // Grab the variable names. + let Expr::Name(ast::ExprName { id: index_name, .. }) = index else { + return None; + }; + + let Expr::Name(ast::ExprName { id: value_name, .. }) = value else { + return None; + }; + + // If either of the variable names are intentionally ignored by naming them `_`, then don't + // emit. + if index_name == "_" || value_name == "_" { + return None; + } + + // Get the first argument of the enumerate call. + let Some(Expr::Name(ast::ExprName { id: sequence, .. })) = arguments.args.first() else { + return None; + }; + + Some((sequence, index_name, value_name)) +} + +#[derive(Debug)] struct SubscriptVisitor<'a> { sequence_name: &'a str, index_name: &'a str, @@ -61,220 +181,84 @@ impl<'a> SubscriptVisitor<'a> { } } -fn check_target_for_assignment(expr: &Expr, sequence_name: &str, index_name: &str) -> bool { - // if we see the sequence, a subscript, or the index being modified, we'll stop emitting diagnostics - match expr { - Expr::Name(ast::ExprName { id, .. }) => id == sequence_name || id == index_name, - Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return false; - }; - if id == sequence_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return false; - }; - if id == index_name { - return true; - } - } - false - } - _ => false, - } -} - -impl<'a> Visitor<'_> for SubscriptVisitor<'a> { - fn visit_expr(&mut self, expr: &Expr) { - if self.modified { - return; - } +impl SubscriptVisitor<'_> { + fn is_assignment(&self, expr: &Expr) -> bool { + // If we see the sequence, a subscript, or the index being modified, we'll stop emitting + // diagnostics. match expr { - Expr::Subscript(ast::ExprSubscript { - value, - slice, - range, - .. - }) => { + Expr::Name(ast::ExprName { id, .. }) => { + id == self.sequence_name || id == self.index_name + } + Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return; + return false; }; if id == self.sequence_name { let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return; + return false; }; if id == self.index_name { - self.diagnostic_ranges.push(*range); + return true; } } + false } - _ => visitor::walk_expr(self, expr), + _ => false, } } +} +impl<'a> Visitor<'_> for SubscriptVisitor<'a> { fn visit_stmt(&mut self, stmt: &Stmt) { if self.modified { return; } match stmt { Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { - self.modified = targets.iter().any(|target| { - check_target_for_assignment(target, self.sequence_name, self.index_name) - }); + self.modified = targets.iter().any(|target| self.is_assignment(target)); self.visit_expr(value); } Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => { if let Some(value) = value { - self.modified = - check_target_for_assignment(target, self.sequence_name, self.index_name); + self.modified = self.is_assignment(target); self.visit_expr(value); } } Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { - self.modified = - check_target_for_assignment(target, self.sequence_name, self.index_name); + self.modified = self.is_assignment(target); self.visit_expr(value); } Stmt::Delete(ast::StmtDelete { targets, .. }) => { - self.modified = targets.iter().any(|target| match target { - Expr::Name(ast::ExprName { id, .. }) => { - id == self.sequence_name || id == self.index_name - } - Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return false; - }; - if id == self.sequence_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return false; - }; - if id == self.index_name { - return true; - } - } - false - } - _ => false, - }); + self.modified = targets.iter().any(|target| self.is_assignment(target)); } _ => visitor::walk_stmt(self, stmt), } } -} - -/// PLR1736 -pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &StmtFor) { - let Some((sequence, index_name, value_name)) = - enumerate_items(checker, &stmt_for.iter, &stmt_for.target) - else { - return; - }; - - let mut visitor = SubscriptVisitor::new(&sequence, &index_name); - - visitor.visit_body(&stmt_for.body); - visitor.visit_body(&stmt_for.orelse); - - for range in visitor.diagnostic_ranges { - let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); - - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.clone(), - range, - ))); - - checker.diagnostics.push(diagnostic); - } -} -/// PLR1736 -pub(crate) fn unnecessary_list_index_lookup_comprehension(checker: &mut Checker, expr: &Expr) { - match expr { - Expr::GeneratorExp(ast::ExprGeneratorExp { - elt, generators, .. - }) - | Expr::DictComp(ast::ExprDictComp { - value: elt, - generators, - .. - }) - | Expr::SetComp(ast::ExprSetComp { - elt, generators, .. - }) - | Expr::ListComp(ast::ExprListComp { - elt, generators, .. - }) => { - for comp in generators { - let Some((sequence, index_name, value_name)) = - enumerate_items(checker, &comp.iter, &comp.target) - else { + fn visit_expr(&mut self, expr: &Expr) { + if self.modified { + return; + } + match expr { + Expr::Subscript(ast::ExprSubscript { + value, + slice, + range, + .. + }) => { + let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { return; }; - - let mut visitor = SubscriptVisitor::new(&sequence, &index_name); - - visitor.visit_expr(elt.as_ref()); - - for range in visitor.diagnostic_ranges { - let mut diagnostic = Diagnostic::new(UnnecessaryListIndexLookup, range); - - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - value_name.clone(), - range, - ))); - - checker.diagnostics.push(diagnostic); + if id == self.sequence_name { + let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { + return; + }; + if id == self.index_name { + self.diagnostic_ranges.push(*range); + } } } + _ => visitor::walk_expr(self, expr), } - _ => (), - } -} - -fn enumerate_items( - checker: &mut Checker, - call_expr: &Expr, - tuple_expr: &Expr, -) -> Option<(String, String, String)> { - let ast::ExprCall { - func, arguments, .. - } = call_expr.as_call_expr()?; - - // Check that the function is the `enumerate` builtin. - let Some(call_path) = checker.semantic().resolve_call_path(func.as_ref()) else { - return None; - }; - - match call_path.as_slice() { - ["", "enumerate"] => (), - ["builtins", "enumerate"] => (), - _ => return None, } - - let Expr::Tuple(ast::ExprTuple { elts, .. }) = tuple_expr else { - return None; - }; - let [index, value] = elts.as_slice() else { - return None; - }; - - // Grab the variable names - let Expr::Name(ast::ExprName { id: index_name, .. }) = index else { - return None; - }; - - let Expr::Name(ast::ExprName { id: value_name, .. }) = value else { - return None; - }; - - // If either of the variable names are intentionally ignored by naming them `_`, then don't emit - if index_name == "_" || value_name == "_" { - return None; - } - - // Get the first argument of the enumerate call - let Some(Expr::Name(ast::ExprName { id: sequence, .. })) = arguments.args.first() else { - return None; - }; - - Some((sequence.clone(), index_name.clone(), value_name.clone())) } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap index aab105471a4c71..8e4d22472df3d7 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap @@ -9,7 +9,7 @@ unnecessary_list_index_lookup.py:7:6: PLR1736 [*] Unnecessary lookup of list ite 8 | {letters[index] for index, letter in enumerate(letters)} # PLR1736 9 | {letter: letters[index] for index, letter in enumerate(letters)} # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 4 4 | @@ -29,7 +29,7 @@ unnecessary_list_index_lookup.py:8:6: PLR1736 [*] Unnecessary lookup of list ite | ^^^^^^^^^^^^^^ PLR1736 9 | {letter: letters[index] for index, letter in enumerate(letters)} # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 5 5 | @@ -50,7 +50,7 @@ unnecessary_list_index_lookup.py:9:14: PLR1736 [*] Unnecessary lookup of list it 10 | 11 | for index, letter in enumerate(letters): | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 6 6 | def fix_these(): @@ -70,7 +70,7 @@ unnecessary_list_index_lookup.py:12:15: PLR1736 [*] Unnecessary lookup of list i 13 | blah = letters[index] # PLR1736 14 | assert letters[index] == "d" # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 9 9 | {letter: letters[index] for index, letter in enumerate(letters)} # PLR1736 @@ -90,7 +90,7 @@ unnecessary_list_index_lookup.py:13:16: PLR1736 [*] Unnecessary lookup of list i | ^^^^^^^^^^^^^^ PLR1736 14 | assert letters[index] == "d" # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 10 10 | @@ -111,7 +111,7 @@ unnecessary_list_index_lookup.py:14:16: PLR1736 [*] Unnecessary lookup of list i 15 | 16 | for index, letter in builtins.enumerate(letters): | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 11 11 | for index, letter in enumerate(letters): @@ -131,7 +131,7 @@ unnecessary_list_index_lookup.py:17:15: PLR1736 [*] Unnecessary lookup of list i 18 | blah = letters[index] # PLR1736 19 | assert letters[index] == "d" # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 14 14 | assert letters[index] == "d" # PLR1736 @@ -151,7 +151,7 @@ unnecessary_list_index_lookup.py:18:16: PLR1736 [*] Unnecessary lookup of list i | ^^^^^^^^^^^^^^ PLR1736 19 | assert letters[index] == "d" # PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 15 15 | @@ -170,7 +170,7 @@ unnecessary_list_index_lookup.py:19:16: PLR1736 [*] Unnecessary lookup of list i 19 | assert letters[index] == "d" # PLR1736 | ^^^^^^^^^^^^^^ PLR1736 | - = help: Use existing item variable instead + = help: Use existing variable ℹ Safe fix 16 16 | for index, letter in builtins.enumerate(letters): From f06c5dc896a9b751620750e5f449f95ffd84bbc7 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 30 Nov 2023 19:42:46 -0600 Subject: [PATCH 2/8] Use correct range for `TRIO115` fix (#8933) ## Summary This PR fixes the bug where the autofix for `TRIO115` was taking the entire arguments range for the fix which included the parenthesis as well. This means that the fix would remove the arguments and the parenthesis. The fix is to use the correct range. fixes: #8713 ## Test Plan Update existing snapshots :) --- .../src/rules/flake8_trio/rules/zero_sleep_call.rs | 2 +- ..._rules__flake8_trio__tests__TRIO115_TRIO115.py.snap | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs index aa583d0d67eaa0..6ddd3d572d0532 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs @@ -103,7 +103,7 @@ pub(crate) fn zero_sleep_call(checker: &mut Checker, call: &ExprCall) { )?; let reference_edit = Edit::range_replacement(format!("{binding}.checkpoint"), call.func.range()); - let arg_edit = Edit::range_deletion(call.arguments.range); + let arg_edit = Edit::range_deletion(arg.range()); Ok(Fix::safe_edits(import_edit, [reference_edit, arg_edit])) }); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap index 9e2fa6d2b5f5f4..6b4c34efd1197f 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap @@ -17,7 +17,7 @@ TRIO115.py:5:11: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 3 3 | from trio import sleep 4 4 | 5 |- await trio.sleep(0) # TRIO115 - 5 |+ await trio.lowlevel.checkpoint # TRIO115 + 5 |+ await trio.lowlevel.checkpoint() # TRIO115 6 6 | await trio.sleep(1) # OK 7 7 | await trio.sleep(0, 1) # OK 8 8 | await trio.sleep(...) # OK @@ -38,7 +38,7 @@ TRIO115.py:11:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 9 9 | await trio.sleep() # OK 10 10 | 11 |- trio.sleep(0) # TRIO115 - 11 |+ trio.lowlevel.checkpoint # TRIO115 + 11 |+ trio.lowlevel.checkpoint() # TRIO115 12 12 | foo = 0 13 13 | trio.sleep(foo) # TRIO115 14 14 | trio.sleep(1) # OK @@ -59,7 +59,7 @@ TRIO115.py:13:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 11 11 | trio.sleep(0) # TRIO115 12 12 | foo = 0 13 |- trio.sleep(foo) # TRIO115 - 13 |+ trio.lowlevel.checkpoint # TRIO115 + 13 |+ trio.lowlevel.checkpoint() # TRIO115 14 14 | trio.sleep(1) # OK 15 15 | time.sleep(0) # OK 16 16 | @@ -80,7 +80,7 @@ TRIO115.py:17:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 15 15 | time.sleep(0) # OK 16 16 | 17 |- sleep(0) # TRIO115 - 17 |+ trio.lowlevel.checkpoint # TRIO115 + 17 |+ trio.lowlevel.checkpoint() # TRIO115 18 18 | 19 19 | bar = "bar" 20 20 | trio.sleep(bar) @@ -102,6 +102,6 @@ TRIO115.py:30:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 28 28 | 29 29 | def func(): 30 |- sleep(0) # TRIO115 - 30 |+ lowlevel.checkpoint # TRIO115 + 30 |+ lowlevel.checkpoint() # TRIO115 From 019d9aebe9742607cbac7d64bec61d3ab1dd8bd9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 21:11:14 -0500 Subject: [PATCH 3/8] Implement multiline dictionary and list hugging for preview style (#8293) ## Summary This PR implement's Black's new single-argument hugging for lists, sets, and dictionaries under preview style. For example, this: ```python foo( [ 1, 2, 3, ] ) ``` Would instead now be formatted as: ```python foo([ 1, 2, 3, ]) ``` A couple notes: - This doesn't apply when the argument has a magic trailing comma. - This _does_ apply when the argument is starred or double-starred. - We don't apply this when there are comments before or after the argument, though Black does in some cases (and moves the comments outside the call parentheses). It doesn't say it in the originating PR (https://github.com/psf/black/pull/3964), but I think this also applies to parenthesized expressions? At least, it does in my testing of preview vs. stable, though it's possible that behavior predated the linked PR. See: #8279. ## Test Plan Before: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99963 | 10596 | 146 | | poetry | 0.99925 | 317 | 12 | | transformers | 0.99967 | 2657 | 322 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99977 | 654 | 13 | | zulip | 0.99970 | 1459 | 21 | After: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99963 | 10596 | 146 | | poetry | 0.96215 | 317 | 34 | | transformers | 0.99967 | 2657 | 322 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99977 | 654 | 13 | | zulip | 0.99970 | 1459 | 21 | --- ...th_braces_and_square_brackets.options.json | 3 + ..._parens_with_braces_and_square_brackets.py | 141 ++++ ..._with_braces_and_square_brackets.py.expect | 159 ++++ .../test/fixtures/ruff/expression/hug.py | 153 ++++ crates/ruff_python_formatter/src/builders.rs | 27 +- .../src/expression/mod.rs | 95 ++- .../src/expression/parentheses.rs | 21 +- .../src/other/arguments.rs | 70 +- ...ns_with_braces_and_square_brackets.py.snap | 543 +++++++++++++ .../snapshots/format@expression__hug.py.snap | 752 ++++++++++++++++++ ...t@expression__split_empty_brackets.py.snap | 20 + 11 files changed, 1969 insertions(+), 15 deletions(-) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.options.json create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py.expect create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/hug.py create mode 100644 crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__preview_hug_parens_with_braces_and_square_brackets.py.snap create mode 100644 crates/ruff_python_formatter/tests/snapshots/format@expression__hug.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.options.json b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.options.json new file mode 100644 index 00000000000000..c106a9c8f36ea1 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.options.json @@ -0,0 +1,3 @@ +{ + "preview": "enabled" +} diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py new file mode 100644 index 00000000000000..eff37f23c008b2 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py @@ -0,0 +1,141 @@ +def foo_brackets(request): + return JsonResponse( + { + "var_1": foo, + "var_2": bar, + } + ) + +def foo_square_brackets(request): + return JsonResponse( + [ + "var_1", + "var_2", + ] + ) + +func({"a": 37, "b": 42, "c": 927, "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111}) + +func(["random_string_number_one","random_string_number_two","random_string_number_three","random_string_number_four"]) + +func( + { + # expand me + 'a':37, + 'b':42, + 'c':927 + } +) + +func( + [ + 'a', + 'b', + 'c', + ] +) + +func( + [ + 'a', + 'b', + 'c', + ], +) + +func( # a + [ # b + "c", # c + "d", # d + "e", # e + ] # f +) # g + +func( # a + { # b + "c": 1, # c + "d": 2, # d + "e": 3, # e + } # f +) # g + +func( + # preserve me + [ + "c", + "d", + "e", + ] +) + +func( + [ # preserve me but hug brackets + "c", + "d", + "e", + ] +) + +func( + [ + # preserve me but hug brackets + "c", + "d", + "e", + ] +) + +func( + [ + "c", + # preserve me but hug brackets + "d", + "e", + ] +) + +func( + [ + "c", + "d", + "e", + # preserve me but hug brackets + ] +) + +func( + [ + "c", + "d", + "e", + ] # preserve me but hug brackets +) + +func( + [ + "c", + "d", + "e", + ] + # preserve me +) + +func([x for x in "short line"]) +func([x for x in "long line long line long line long line long line long line long line"]) +func([x for x in [x for x in "long line long line long line long line long line long line long line"]]) + +func({"short line"}) +func({"long line", "long long line", "long long long line", "long long long long line", "long long long long long line"}) +func({{"long line", "long long line", "long long long line", "long long long long line", "long long long long long line"}}) + +foooooooooooooooooooo( + [{c: n + 1 for c in range(256)} for n in range(100)] + [{}], {size} +) + +baaaaaaaaaaaaar( + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], {x}, "a string", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +) + +foo(*["long long long long long line", "long long long long long line", "long long long long long line"]) + +foo(*[str(i) for i in range(100000000000000000000000000000000000000000000000000000000000)]) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py.expect b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py.expect new file mode 100644 index 00000000000000..963fb7c4040a4b --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py.expect @@ -0,0 +1,159 @@ +def foo_brackets(request): + return JsonResponse({ + "var_1": foo, + "var_2": bar, + }) + + +def foo_square_brackets(request): + return JsonResponse([ + "var_1", + "var_2", + ]) + + +func({ + "a": 37, + "b": 42, + "c": 927, + "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, +}) + +func([ + "random_string_number_one", + "random_string_number_two", + "random_string_number_three", + "random_string_number_four", +]) + +func({ + # expand me + "a": 37, + "b": 42, + "c": 927, +}) + +func([ + "a", + "b", + "c", +]) + +func( + [ + "a", + "b", + "c", + ], +) + +func([ # a # b + "c", # c + "d", # d + "e", # e +]) # f # g + +func({ # a # b + "c": 1, # c + "d": 2, # d + "e": 3, # e +}) # f # g + +func( + # preserve me + [ + "c", + "d", + "e", + ] +) + +func([ # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + "c", + # preserve me but hug brackets + "d", + "e", +]) + +func([ + "c", + "d", + "e", + # preserve me but hug brackets +]) + +func([ + "c", + "d", + "e", +]) # preserve me but hug brackets + +func( + [ + "c", + "d", + "e", + ] + # preserve me +) + +func([x for x in "short line"]) +func([ + x for x in "long line long line long line long line long line long line long line" +]) +func([ + x + for x in [ + x + for x in "long line long line long line long line long line long line long line" + ] +]) + +func({"short line"}) +func({ + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +}) +func({ + { + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", + } +}) + +foooooooooooooooooooo( + [{c: n + 1 for c in range(256)} for n in range(100)] + [{}], {size} +) + +baaaaaaaaaaaaar( + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], {x}, "a string", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +) + +foo(*[ + "long long long long long line", + "long long long long long line", + "long long long long long line", +]) + +foo(*[ + str(i) for i in range(100000000000000000000000000000000000000000000000000000000000) +]) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/hug.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/hug.py new file mode 100644 index 00000000000000..bbd41b51a8ba77 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/hug.py @@ -0,0 +1,153 @@ +# Preview style: hug brackets to call parentheses. +func([1, 2, 3,]) + +func( # comment +[1, 2, 3,]) + +func( + # comment +[1, 2, 3,]) + +func([1, 2, 3,] # comment +) + +func([1, 2, 3,] + # comment +) + +func([ # comment + 1, 2, 3,] +) + +func(([1, 2, 3,])) + + +func( + ( + 1, + 2, + 3, + ) +) + +# Ensure that comprehensions hug too. +func([(x, y,) for (x, y) in z]) + +# Ensure that dictionaries hug too. +func({1: 2, 3: 4, 5: 6,}) + +# Ensure that the same behavior is applied to parenthesized expressions. +([1, 2, 3,]) + +( # comment + [1, 2, 3,]) + +( + [ # comment + 1, 2, 3,]) + +# Ensure that starred arguments are also hugged. +foo( + *[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + * # comment + [ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + **[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + ** # comment + [ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +# Ensure that multi-argument calls are _not_ hugged. +func([1, 2, 3,], bar) + +func([(x, y,) for (x, y) in z], bar) + +# Ensure that return type annotations (which use `parenthesize_if_expands`) are also hugged. +def func() -> [1, 2, 3,]: + pass + +def func() -> ([1, 2, 3,]): + pass + +def func() -> ([1, 2, 3,]): + pass + +def func() -> ( # comment + [1, 2, 3,]): + pass + +def func() -> ( + [1, 2, 3,] # comment +): + pass + +def func() -> ( + [1, 2, 3,] + # comment +): + pass + +# Ensure that nested lists are hugged. +func([ + [ + 1, + 2, + 3, + ] +]) + + +func([ + # comment + [ + 1, + 2, + 3, + ] +]) + +func([ + [ + 1, + 2, + 3, + ] + # comment +]) + +func([ + [ # comment + 1, + 2, + 3, + ] +]) + + +func([ # comment + [ + 1, + 2, + 3, + ] +]) diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index 581fdc5194a667..e4e2909a4a6dde 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -1,4 +1,4 @@ -use ruff_formatter::{format_args, write, Argument, Arguments}; +use ruff_formatter::{write, Argument, Arguments}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::context::{NodeLevel, WithNodeLevel}; @@ -12,11 +12,20 @@ where { ParenthesizeIfExpands { inner: Argument::new(content), + indent: true, } } pub(crate) struct ParenthesizeIfExpands<'a, 'ast> { inner: Argument<'a, PyFormatContext<'ast>>, + indent: bool, +} + +impl ParenthesizeIfExpands<'_, '_> { + pub(crate) fn with_indent(mut self, indent: bool) -> Self { + self.indent = indent; + self + } } impl<'ast> Format> for ParenthesizeIfExpands<'_, 'ast> { @@ -26,11 +35,17 @@ impl<'ast> Format> for ParenthesizeIfExpands<'_, 'ast> { write!( f, - [group(&format_args![ - if_group_breaks(&token("(")), - soft_block_indent(&Arguments::from(&self.inner)), - if_group_breaks(&token(")")), - ])] + [group(&format_with(|f| { + if_group_breaks(&token("(")).fmt(f)?; + + if self.indent { + soft_block_indent(&Arguments::from(&self.inner)).fmt(f)?; + } else { + Arguments::from(&self.inner).fmt(f)?; + }; + + if_group_breaks(&token(")")).fmt(f) + }))] ) } } diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 86fae5137a448d..b9dc9e8520a045 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -23,6 +23,7 @@ use crate::expression::parentheses::{ OptionalParentheses, Parentheses, Parenthesize, }; use crate::prelude::*; +use crate::PyFormatOptions; mod binary_like; pub(crate) mod expr_attribute; @@ -126,10 +127,12 @@ impl FormatRule> for FormatExpr { Parentheses::Never => false, }; if parenthesize { - let comment = f.context().comments().clone(); - let node_comments = comment.leading_dangling_trailing(expression); + let comments = f.context().comments().clone(); + let node_comments = comments.leading_dangling_trailing(expression); if !node_comments.has_leading() && !node_comments.has_trailing() { - parenthesized("(", &format_expr, ")").fmt(f) + parenthesized("(", &format_expr, ")") + .with_indent(!is_expression_huggable(expression, f.options())) + .fmt(f) } else { format_with_parentheses_comments(expression, &node_comments, f) } @@ -403,9 +406,11 @@ impl Format> for MaybeParenthesizeExpression<'_> { parenthesize_if_expands(&expression.format().with_options(Parentheses::Never)) .fmt(f) } + Parenthesize::IfRequired => { expression.format().with_options(Parentheses::Never).fmt(f) } + Parenthesize::Optional | Parenthesize::IfBreaks => { if can_omit_optional_parentheses(expression, f.context()) { optional_parentheses(&expression.format().with_options(Parentheses::Never)) @@ -427,6 +432,7 @@ impl Format> for MaybeParenthesizeExpression<'_> { Parenthesize::Optional | Parenthesize::IfRequired => { expression.format().with_options(Parentheses::Never).fmt(f) } + Parenthesize::IfBreaks => { // Is the expression the last token in the parent statement. // Excludes `await` and `yield` for which Black doesn't seem to apply the layout? @@ -534,6 +540,7 @@ impl Format> for MaybeParenthesizeExpression<'_> { OptionalParentheses::Never => match parenthesize { Parenthesize::IfBreaksOrIfRequired => { parenthesize_if_expands(&expression.format().with_options(Parentheses::Never)) + .with_indent(!is_expression_huggable(expression, f.options())) .fmt(f) } @@ -1119,6 +1126,86 @@ pub(crate) fn has_own_parentheses( } } +/// Returns `true` if the expression can hug directly to enclosing parentheses, as in Black's +/// `hug_parens_with_braces_and_square_brackets` preview style behavior. +/// +/// For example, in preview style, given: +/// ```python +/// ([1, 2, 3,]) +/// ``` +/// +/// We want to format it as: +/// ```python +/// ([ +/// 1, +/// 2, +/// 3, +/// ]) +/// ``` +/// +/// As opposed to: +/// ```python +/// ( +/// [ +/// 1, +/// 2, +/// 3, +/// ] +/// ) +/// ``` +pub(crate) fn is_expression_huggable(expr: &Expr, options: &PyFormatOptions) -> bool { + if !options.preview().is_enabled() { + return false; + } + + match expr { + Expr::Tuple(_) + | Expr::List(_) + | Expr::Set(_) + | Expr::Dict(_) + | Expr::ListComp(_) + | Expr::SetComp(_) + | Expr::DictComp(_) => true, + + Expr::Starred(ast::ExprStarred { value, .. }) => matches!( + value.as_ref(), + Expr::Tuple(_) + | Expr::List(_) + | Expr::Set(_) + | Expr::Dict(_) + | Expr::ListComp(_) + | Expr::SetComp(_) + | Expr::DictComp(_) + ), + + Expr::BoolOp(_) + | Expr::NamedExpr(_) + | Expr::BinOp(_) + | Expr::UnaryOp(_) + | Expr::Lambda(_) + | Expr::IfExp(_) + | Expr::GeneratorExp(_) + | Expr::Await(_) + | Expr::Yield(_) + | Expr::YieldFrom(_) + | Expr::Compare(_) + | Expr::Call(_) + | Expr::FormattedValue(_) + | Expr::FString(_) + | Expr::Attribute(_) + | Expr::Subscript(_) + | Expr::Name(_) + | Expr::Slice(_) + | Expr::IpyEscapeCommand(_) + | Expr::StringLiteral(_) + | Expr::BytesLiteral(_) + | Expr::NumberLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + | Expr::EllipsisLiteral(_) => false, + } +} + /// The precedence of [python operators](https://docs.python.org/3/reference/expressions.html#operator-precedence) from /// highest to lowest priority. /// @@ -1144,7 +1231,7 @@ enum OperatorPrecedence { Conditional, } -impl From for OperatorPrecedence { +impl From for OperatorPrecedence { fn from(value: Operator) -> Self { match value { Operator::Add | Operator::Sub => OperatorPrecedence::Additive, diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index d05b9dcbd7d5f2..971d9131466095 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -84,6 +84,7 @@ pub enum Parentheses { Never, } +/// Returns `true` if the [`ExpressionRef`] is enclosed by parentheses in the source code. pub(crate) fn is_expression_parenthesized( expr: ExpressionRef, comment_ranges: &CommentRanges, @@ -125,6 +126,7 @@ where FormatParenthesized { left, comments: &[], + indent: true, content: Argument::new(content), right, } @@ -133,6 +135,7 @@ where pub(crate) struct FormatParenthesized<'content, 'ast> { left: &'static str, comments: &'content [SourceComment], + indent: bool, content: Argument<'content, PyFormatContext<'ast>>, right: &'static str, } @@ -153,6 +156,11 @@ impl<'content, 'ast> FormatParenthesized<'content, 'ast> { ) -> FormatParenthesized<'content, 'ast> { FormatParenthesized { comments, ..self } } + + /// Whether to indent the content within the parentheses. + pub(crate) fn with_indent(self, indent: bool) -> FormatParenthesized<'content, 'ast> { + FormatParenthesized { indent, ..self } + } } impl<'ast> Format> for FormatParenthesized<'_, 'ast> { @@ -160,10 +168,15 @@ impl<'ast> Format> for FormatParenthesized<'_, 'ast> { let current_level = f.context().node_level(); let content = format_with(|f| { - group(&format_args![ - dangling_open_parenthesis_comments(self.comments), - soft_block_indent(&Arguments::from(&self.content)) - ]) + group(&format_with(|f| { + dangling_open_parenthesis_comments(self.comments).fmt(f)?; + if self.indent || !self.comments.is_empty() { + soft_block_indent(&Arguments::from(&self.content)).fmt(f)?; + } else { + Arguments::from(&self.content).fmt(f)?; + } + Ok(()) + })) .fmt(f) }); diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index 5baa9fa741c46e..a48596cac0eb37 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -1,11 +1,13 @@ -use ruff_formatter::write; +use ruff_formatter::{write, FormatContext}; use ruff_python_ast::{ArgOrKeyword, Arguments, Expr}; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::comments::SourceComment; use crate::expression::expr_generator_exp::GeneratorExpParentheses; +use crate::expression::is_expression_huggable; use crate::expression::parentheses::{empty_parenthesized, parenthesized, Parentheses}; +use crate::other::commas; use crate::prelude::*; #[derive(Default)] @@ -104,6 +106,7 @@ impl FormatNodeRule for FormatArguments { // ) // ``` parenthesized("(", &group(&all_arguments), ")") + .with_indent(!is_argument_huggable(item, f.context())) .with_dangling_comments(dangling_comments) ] ) @@ -143,3 +146,68 @@ fn is_single_argument_parenthesized(argument: &Expr, call_end: TextSize, source: false } +/// Returns `true` if the arguments can hug directly to the enclosing parentheses in the call, as +/// in Black's `hug_parens_with_braces_and_square_brackets` preview style behavior. +/// +/// For example, in preview style, given: +/// ```python +/// func([1, 2, 3,]) +/// ``` +/// +/// We want to format it as: +/// ```python +/// func([ +/// 1, +/// 2, +/// 3, +/// ]) +/// ``` +/// +/// As opposed to: +/// ```python +/// func( +/// [ +/// 1, +/// 2, +/// 3, +/// ] +/// ) +/// ``` +/// +/// Hugging should only be applied to single-argument collections, like lists, or starred versions +/// of those collections. +fn is_argument_huggable(item: &Arguments, context: &PyFormatContext) -> bool { + let options = context.options(); + if !options.preview().is_enabled() { + return false; + } + + // Find the lone argument or `**kwargs` keyword. + let arg = match (item.args.as_slice(), item.keywords.as_slice()) { + ([arg], []) => arg, + ([], [keyword]) if keyword.arg.is_none() && !context.comments().has(keyword) => { + &keyword.value + } + _ => return false, + }; + + // If the expression itself isn't huggable, then we can't hug it. + if !is_expression_huggable(arg, options) { + return false; + } + + // If the expression has leading or trailing comments, then we can't hug it. + let comments = context.comments().leading_dangling_trailing(arg); + if comments.has_leading() || comments.has_trailing() { + return false; + } + + // If the expression has a trailing comma, then we can't hug it. + if options.magic_trailing_comma().is_respect() + && commas::has_magic_trailing_comma(TextRange::new(arg.end(), item.end()), options, context) + { + return false; + } + + true +} diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__preview_hug_parens_with_braces_and_square_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__preview_hug_parens_with_braces_and_square_brackets.py.snap new file mode 100644 index 00000000000000..df9471aac22dc2 --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__preview_hug_parens_with_braces_and_square_brackets.py.snap @@ -0,0 +1,543 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/preview_hug_parens_with_braces_and_square_brackets.py +--- +## Input + +```python +def foo_brackets(request): + return JsonResponse( + { + "var_1": foo, + "var_2": bar, + } + ) + +def foo_square_brackets(request): + return JsonResponse( + [ + "var_1", + "var_2", + ] + ) + +func({"a": 37, "b": 42, "c": 927, "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111}) + +func(["random_string_number_one","random_string_number_two","random_string_number_three","random_string_number_four"]) + +func( + { + # expand me + 'a':37, + 'b':42, + 'c':927 + } +) + +func( + [ + 'a', + 'b', + 'c', + ] +) + +func( + [ + 'a', + 'b', + 'c', + ], +) + +func( # a + [ # b + "c", # c + "d", # d + "e", # e + ] # f +) # g + +func( # a + { # b + "c": 1, # c + "d": 2, # d + "e": 3, # e + } # f +) # g + +func( + # preserve me + [ + "c", + "d", + "e", + ] +) + +func( + [ # preserve me but hug brackets + "c", + "d", + "e", + ] +) + +func( + [ + # preserve me but hug brackets + "c", + "d", + "e", + ] +) + +func( + [ + "c", + # preserve me but hug brackets + "d", + "e", + ] +) + +func( + [ + "c", + "d", + "e", + # preserve me but hug brackets + ] +) + +func( + [ + "c", + "d", + "e", + ] # preserve me but hug brackets +) + +func( + [ + "c", + "d", + "e", + ] + # preserve me +) + +func([x for x in "short line"]) +func([x for x in "long line long line long line long line long line long line long line"]) +func([x for x in [x for x in "long line long line long line long line long line long line long line"]]) + +func({"short line"}) +func({"long line", "long long line", "long long long line", "long long long long line", "long long long long long line"}) +func({{"long line", "long long line", "long long long line", "long long long long line", "long long long long long line"}}) + +foooooooooooooooooooo( + [{c: n + 1 for c in range(256)} for n in range(100)] + [{}], {size} +) + +baaaaaaaaaaaaar( + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], {x}, "a string", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +) + +foo(*["long long long long long line", "long long long long long line", "long long long long long line"]) + +foo(*[str(i) for i in range(100000000000000000000000000000000000000000000000000000000000)]) +``` + +## Black Differences + +```diff +--- Black ++++ Ruff +@@ -47,17 +47,21 @@ + ], + ) + +-func([ # a # b +- "c", # c +- "d", # d +- "e", # e +-]) # f # g ++func( # a ++ [ # b ++ "c", # c ++ "d", # d ++ "e", # e ++ ] # f ++) # g + +-func({ # a # b +- "c": 1, # c +- "d": 2, # d +- "e": 3, # e +-}) # f # g ++func( # a ++ { # b ++ "c": 1, # c ++ "d": 2, # d ++ "e": 3, # e ++ } # f ++) # g + + func( + # preserve me +@@ -95,11 +99,13 @@ + # preserve me but hug brackets + ]) + +-func([ +- "c", +- "d", +- "e", +-]) # preserve me but hug brackets ++func( ++ [ ++ "c", ++ "d", ++ "e", ++ ] # preserve me but hug brackets ++) + + func( + [ +``` + +## Ruff Output + +```python +def foo_brackets(request): + return JsonResponse({ + "var_1": foo, + "var_2": bar, + }) + + +def foo_square_brackets(request): + return JsonResponse([ + "var_1", + "var_2", + ]) + + +func({ + "a": 37, + "b": 42, + "c": 927, + "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, +}) + +func([ + "random_string_number_one", + "random_string_number_two", + "random_string_number_three", + "random_string_number_four", +]) + +func({ + # expand me + "a": 37, + "b": 42, + "c": 927, +}) + +func([ + "a", + "b", + "c", +]) + +func( + [ + "a", + "b", + "c", + ], +) + +func( # a + [ # b + "c", # c + "d", # d + "e", # e + ] # f +) # g + +func( # a + { # b + "c": 1, # c + "d": 2, # d + "e": 3, # e + } # f +) # g + +func( + # preserve me + [ + "c", + "d", + "e", + ] +) + +func([ # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + "c", + # preserve me but hug brackets + "d", + "e", +]) + +func([ + "c", + "d", + "e", + # preserve me but hug brackets +]) + +func( + [ + "c", + "d", + "e", + ] # preserve me but hug brackets +) + +func( + [ + "c", + "d", + "e", + ] + # preserve me +) + +func([x for x in "short line"]) +func([ + x for x in "long line long line long line long line long line long line long line" +]) +func([ + x + for x in [ + x + for x in "long line long line long line long line long line long line long line" + ] +]) + +func({"short line"}) +func({ + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +}) +func({ + { + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", + } +}) + +foooooooooooooooooooo( + [{c: n + 1 for c in range(256)} for n in range(100)] + [{}], {size} +) + +baaaaaaaaaaaaar( + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], {x}, "a string", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +) + +foo(*[ + "long long long long long line", + "long long long long long line", + "long long long long long line", +]) + +foo(*[ + str(i) for i in range(100000000000000000000000000000000000000000000000000000000000) +]) +``` + +## Black Output + +```python +def foo_brackets(request): + return JsonResponse({ + "var_1": foo, + "var_2": bar, + }) + + +def foo_square_brackets(request): + return JsonResponse([ + "var_1", + "var_2", + ]) + + +func({ + "a": 37, + "b": 42, + "c": 927, + "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, +}) + +func([ + "random_string_number_one", + "random_string_number_two", + "random_string_number_three", + "random_string_number_four", +]) + +func({ + # expand me + "a": 37, + "b": 42, + "c": 927, +}) + +func([ + "a", + "b", + "c", +]) + +func( + [ + "a", + "b", + "c", + ], +) + +func([ # a # b + "c", # c + "d", # d + "e", # e +]) # f # g + +func({ # a # b + "c": 1, # c + "d": 2, # d + "e": 3, # e +}) # f # g + +func( + # preserve me + [ + "c", + "d", + "e", + ] +) + +func([ # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + # preserve me but hug brackets + "c", + "d", + "e", +]) + +func([ + "c", + # preserve me but hug brackets + "d", + "e", +]) + +func([ + "c", + "d", + "e", + # preserve me but hug brackets +]) + +func([ + "c", + "d", + "e", +]) # preserve me but hug brackets + +func( + [ + "c", + "d", + "e", + ] + # preserve me +) + +func([x for x in "short line"]) +func([ + x for x in "long line long line long line long line long line long line long line" +]) +func([ + x + for x in [ + x + for x in "long line long line long line long line long line long line long line" + ] +]) + +func({"short line"}) +func({ + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +}) +func({ + { + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", + } +}) + +foooooooooooooooooooo( + [{c: n + 1 for c in range(256)} for n in range(100)] + [{}], {size} +) + +baaaaaaaaaaaaar( + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], {x}, "a string", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] +) + +foo(*[ + "long long long long long line", + "long long long long long line", + "long long long long long line", +]) + +foo(*[ + str(i) for i in range(100000000000000000000000000000000000000000000000000000000000) +]) +``` + + diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__hug.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__hug.py.snap new file mode 100644 index 00000000000000..f9a4ca0ba5be84 --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__hug.py.snap @@ -0,0 +1,752 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/hug.py +--- +## Input +```python +# Preview style: hug brackets to call parentheses. +func([1, 2, 3,]) + +func( # comment +[1, 2, 3,]) + +func( + # comment +[1, 2, 3,]) + +func([1, 2, 3,] # comment +) + +func([1, 2, 3,] + # comment +) + +func([ # comment + 1, 2, 3,] +) + +func(([1, 2, 3,])) + + +func( + ( + 1, + 2, + 3, + ) +) + +# Ensure that comprehensions hug too. +func([(x, y,) for (x, y) in z]) + +# Ensure that dictionaries hug too. +func({1: 2, 3: 4, 5: 6,}) + +# Ensure that the same behavior is applied to parenthesized expressions. +([1, 2, 3,]) + +( # comment + [1, 2, 3,]) + +( + [ # comment + 1, 2, 3,]) + +# Ensure that starred arguments are also hugged. +foo( + *[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + * # comment + [ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + **[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + ** # comment + [ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +# Ensure that multi-argument calls are _not_ hugged. +func([1, 2, 3,], bar) + +func([(x, y,) for (x, y) in z], bar) + +# Ensure that return type annotations (which use `parenthesize_if_expands`) are also hugged. +def func() -> [1, 2, 3,]: + pass + +def func() -> ([1, 2, 3,]): + pass + +def func() -> ([1, 2, 3,]): + pass + +def func() -> ( # comment + [1, 2, 3,]): + pass + +def func() -> ( + [1, 2, 3,] # comment +): + pass + +def func() -> ( + [1, 2, 3,] + # comment +): + pass + +# Ensure that nested lists are hugged. +func([ + [ + 1, + 2, + 3, + ] +]) + + +func([ + # comment + [ + 1, + 2, + 3, + ] +]) + +func([ + [ + 1, + 2, + 3, + ] + # comment +]) + +func([ + [ # comment + 1, + 2, + 3, + ] +]) + + +func([ # comment + [ + 1, + 2, + 3, + ] +]) +``` + +## Output +```python +# Preview style: hug brackets to call parentheses. +func( + [ + 1, + 2, + 3, + ] +) + +func( # comment + [ + 1, + 2, + 3, + ] +) + +func( + # comment + [ + 1, + 2, + 3, + ] +) + +func( + [ + 1, + 2, + 3, + ] # comment +) + +func( + [ + 1, + 2, + 3, + ] + # comment +) + +func( + [ # comment + 1, + 2, + 3, + ] +) + +func( + ( + [ + 1, + 2, + 3, + ] + ) +) + + +func( + ( + 1, + 2, + 3, + ) +) + +# Ensure that comprehensions hug too. +func( + [ + ( + x, + y, + ) + for (x, y) in z + ] +) + +# Ensure that dictionaries hug too. +func( + { + 1: 2, + 3: 4, + 5: 6, + } +) + +# Ensure that the same behavior is applied to parenthesized expressions. +( + [ + 1, + 2, + 3, + ] +) + +( # comment + [ + 1, + 2, + 3, + ] +) + +( + [ # comment + 1, + 2, + 3, + ] +) + +# Ensure that starred arguments are also hugged. +foo( + *[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + # comment + *[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + **[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +foo( + # comment + **[ + a_long_function_name(a_long_variable_name) + for a_long_variable_name in some_generator + ] +) + +# Ensure that multi-argument calls are _not_ hugged. +func( + [ + 1, + 2, + 3, + ], + bar, +) + +func( + [ + ( + x, + y, + ) + for (x, y) in z + ], + bar, +) + + +# Ensure that return type annotations (which use `parenthesize_if_expands`) are also hugged. +def func() -> ( + [ + 1, + 2, + 3, + ] +): + pass + + +def func() -> ( + [ + 1, + 2, + 3, + ] +): + pass + + +def func() -> ( + [ + 1, + 2, + 3, + ] +): + pass + + +def func() -> ( # comment + [ + 1, + 2, + 3, + ] +): + pass + + +def func() -> ( + [ + 1, + 2, + 3, + ] # comment +): + pass + + +def func() -> ( + [ + 1, + 2, + 3, + ] + # comment +): + pass + + +# Ensure that nested lists are hugged. +func( + [ + [ + 1, + 2, + 3, + ] + ] +) + + +func( + [ + # comment + [ + 1, + 2, + 3, + ] + ] +) + +func( + [ + [ + 1, + 2, + 3, + ] + # comment + ] +) + +func( + [ + [ # comment + 1, + 2, + 3, + ] + ] +) + + +func( + [ # comment + [ + 1, + 2, + 3, + ] + ] +) +``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -1,11 +1,9 @@ + # Preview style: hug brackets to call parentheses. +-func( +- [ +- 1, +- 2, +- 3, +- ] +-) ++func([ ++ 1, ++ 2, ++ 3, ++]) + + func( # comment + [ +@@ -41,61 +39,47 @@ + # comment + ) + +-func( +- [ # comment +- 1, +- 2, +- 3, +- ] +-) ++func([ # comment ++ 1, ++ 2, ++ 3, ++]) + +-func( +- ( +- [ +- 1, +- 2, +- 3, +- ] +- ) +-) ++func(([ ++ 1, ++ 2, ++ 3, ++])) + + +-func( +- ( +- 1, +- 2, +- 3, +- ) +-) ++func(( ++ 1, ++ 2, ++ 3, ++)) + + # Ensure that comprehensions hug too. +-func( +- [ +- ( +- x, +- y, +- ) +- for (x, y) in z +- ] +-) ++func([ ++ ( ++ x, ++ y, ++ ) ++ for (x, y) in z ++]) + + # Ensure that dictionaries hug too. +-func( +- { +- 1: 2, +- 3: 4, +- 5: 6, +- } +-) ++func({ ++ 1: 2, ++ 3: 4, ++ 5: 6, ++}) + + # Ensure that the same behavior is applied to parenthesized expressions. +-( +- [ +- 1, +- 2, +- 3, +- ] +-) ++([ ++ 1, ++ 2, ++ 3, ++]) + + ( # comment + [ +@@ -105,21 +89,17 @@ + ] + ) + +-( +- [ # comment +- 1, +- 2, +- 3, +- ] +-) ++([ # comment ++ 1, ++ 2, ++ 3, ++]) + + # Ensure that starred arguments are also hugged. +-foo( +- *[ +- a_long_function_name(a_long_variable_name) +- for a_long_variable_name in some_generator +- ] +-) ++foo(*[ ++ a_long_function_name(a_long_variable_name) ++ for a_long_variable_name in some_generator ++]) + + foo( + # comment +@@ -129,12 +109,10 @@ + ] + ) + +-foo( +- **[ +- a_long_function_name(a_long_variable_name) +- for a_long_variable_name in some_generator +- ] +-) ++foo(**[ ++ a_long_function_name(a_long_variable_name) ++ for a_long_variable_name in some_generator ++]) + + foo( + # comment +@@ -167,33 +145,27 @@ + + + # Ensure that return type annotations (which use `parenthesize_if_expands`) are also hugged. +-def func() -> ( +- [ +- 1, +- 2, +- 3, +- ] +-): ++def func() -> ([ ++ 1, ++ 2, ++ 3, ++]): + pass + + +-def func() -> ( +- [ +- 1, +- 2, +- 3, +- ] +-): ++def func() -> ([ ++ 1, ++ 2, ++ 3, ++]): + pass + + +-def func() -> ( +- [ +- 1, +- 2, +- 3, +- ] +-): ++def func() -> ([ ++ 1, ++ 2, ++ 3, ++]): + pass + + +@@ -229,56 +201,46 @@ + + + # Ensure that nested lists are hugged. +-func( ++func([ + [ +- [ +- 1, +- 2, +- 3, +- ] ++ 1, ++ 2, ++ 3, + ] +-) ++]) + + +-func( ++func([ ++ # comment + [ +- # comment +- [ +- 1, +- 2, +- 3, +- ] ++ 1, ++ 2, ++ 3, + ] +-) ++]) + +-func( ++func([ + [ +- [ +- 1, +- 2, +- 3, +- ] +- # comment ++ 1, ++ 2, ++ 3, + ] +-) ++ # comment ++]) + +-func( +- [ +- [ # comment +- 1, +- 2, +- 3, +- ] ++func([ ++ [ # comment ++ 1, ++ 2, ++ 3, + ] +-) ++]) + + +-func( +- [ # comment +- [ +- 1, +- 2, +- 3, +- ] ++func([ # comment ++ [ ++ 1, ++ 2, ++ 3, + ] +-) ++]) +``` + + + diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap index 810964acc19a2f..2c9fb1d3808165 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap @@ -194,4 +194,24 @@ response = await sync_to_async( ``` +## Preview changes +```diff +--- Stable ++++ Preview +@@ -62,9 +62,9 @@ + 1 + }.unicodedata.normalize("NFKCNFKCNFKCNFKCNFKC", s2).casefold() + +-ct_match = ([]).unicodedata.normalize("NFKC", s1).casefold() == ( +- [] +-).unicodedata.normalize("NFKCNFKCNFKCNFKCNFKC", s2).casefold() ++ct_match = ([]).unicodedata.normalize( ++ "NFKC", s1 ++).casefold() == ([]).unicodedata.normalize("NFKCNFKCNFKCNFKCNFKC", s2).casefold() + + return await self.http_client.fetch( + f"http://127.0.0.1:{self.port}{path}", +``` + + From eaa310429fc9b6d0b67d7b174ec3444921c80873 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 21:49:28 -0500 Subject: [PATCH 4/8] Insert trailing comma when function breaks with single argument (#8921) ## Summary Given: ```python def _example_function_xxxxxxx( variable: Optional[List[str]] ) -> List[example.ExampleConfig]: pass ``` We should be inserting a trailing comma after the argument (as long as it's a single-argument function). This was an inconsistency with Black, but also led to some internal inconsistencies, whereby we added the comma if the argument contained a trailing end-of-line comment, but not otherwise. Closes https://github.com/astral-sh/ruff/issues/8912. ## Test Plan `cargo test` Before: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99963 | 10596 | 146 | | poetry | 0.99925 | 317 | 12 | | transformers | 0.99967 | 2657 | 322 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99977 | 654 | 13 | | zulip | 0.99970 | 1459 | 21 | After: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99955 | 10596 | 213 | | poetry | 0.99917 | 317 | 13 | | transformers | 0.99967 | 2657 | 324 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99976 | 654 | 14 | | zulip | 0.99957 | 1459 | 36 | --- .../src/other/parameters.rs | 13 ++++ ...funcdef_return_type_trailing_comma.py.snap | 24 ++----- ..._return_annotation_brackets_string.py.snap | 11 +--- ...@cases__return_annotation_brackets.py.snap | 8 +-- ..._opening_parentheses_comment_value.py.snap | 2 +- .../format@statement__function.py.snap | 2 +- ...ormat@statement__return_annotation.py.snap | 66 +++++++++---------- 7 files changed, 60 insertions(+), 66 deletions(-) diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index 86cad5869b2ce1..d095d33ae1975d 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -252,6 +252,19 @@ impl FormatNodeRule for FormatParameters { let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); // No parameters, format any dangling comments between `()` write!(f, [empty_parenthesized("(", dangling, ")")]) + } else if num_parameters == 1 { + // If we have a single argument, avoid the inner group, to ensure that we insert a + // trailing comma if the outer group breaks. + let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); + write!( + f, + [ + token("("), + dangling_open_parenthesis_comments(parenthesis_dangling), + soft_block_indent(&format_inner), + token(")") + ] + ) } else { // Intentionally avoid `parenthesized`, which groups the entire formatted contents. // We want parameters to be grouped alongside return types, one level up, so we diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap index a10744872cd4a5..09c380567e9514 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap @@ -179,13 +179,7 @@ def SimplePyFn( p, q, ]: -@@ -63,16 +67,18 @@ - # long function definition, return type is longer - # this should maybe split on rhs? - def aaaaaaaaaaaaaaaaa( -- bbbbbbbbbbbbbbbbbb, -+ bbbbbbbbbbbbbbbbbb - ) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ... +@@ -68,11 +72,13 @@ # long return type, no param list @@ -204,25 +198,19 @@ def SimplePyFn( # long function name, no param list, no return value -@@ -93,12 +99,16 @@ +@@ -93,7 +99,11 @@ # unskippable type hint (??) -def foo(a) -> list[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]: # type: ignore +def foo( -+ a ++ a, +) -> list[ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +]: # type: ignore pass - def foo( -- a, -+ a - ) -> list[ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - ]: # abpedeifnore @@ -112,7 +122,13 @@ @@ -333,7 +321,7 @@ def aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa( # long function definition, return type is longer # this should maybe split on rhs? def aaaaaaaaaaaaaaaaa( - bbbbbbbbbbbbbbbbbb + bbbbbbbbbbbbbbbbbb, ) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ... @@ -366,7 +354,7 @@ def thiiiiiiiiiiiiiiiiiis_iiiiiiiiiiiiiiiiiiiiiiiiiiiiiis_veeeeeeeeeeeeeeeeeeeee # unskippable type hint (??) def foo( - a + a, ) -> list[ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ]: # type: ignore @@ -374,7 +362,7 @@ def foo( def foo( - a + a, ) -> list[ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ]: # abpedeifnore diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap index 268fcf0eb84878..5f43f1a7d20b58 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap @@ -19,7 +19,7 @@ def frobnicate(a) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisI ```diff --- Black +++ Ruff -@@ -1,13 +1,12 @@ +@@ -1,7 +1,6 @@ # Long string example def frobnicate() -> ( - "ThisIsTrulyUnreasonablyExtremelyLongClassName |" @@ -28,13 +28,6 @@ def frobnicate(a) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisI ): pass - - # splitting the string breaks if there's any parameters - def frobnicate( -- a, -+ a - ) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisIsTrulyUnreasonablyExtremelyLongClassName]": - pass ``` ## Ruff Output @@ -49,7 +42,7 @@ def frobnicate() -> ( # splitting the string breaks if there's any parameters def frobnicate( - a + a, ) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisIsTrulyUnreasonablyExtremelyLongClassName]": pass ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap index 415daad90c7866..9bf6a451152c9f 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap @@ -111,7 +111,7 @@ def foo(a,b) -> tuple[int, int, int,]: # Don't lose the comments -def double(a: int) -> int: # Hello +def double( -+ a: int ++ a: int, +) -> ( # Hello + int +): @@ -120,7 +120,7 @@ def foo(a,b) -> tuple[int, int, int,]: -def double(a: int) -> int: # Hello +def double( -+ a: int ++ a: int, +) -> ( + int # Hello +): @@ -168,7 +168,7 @@ def double(a: int) -> int: # Don't lose the comments def double( - a: int + a: int, ) -> ( # Hello int ): @@ -176,7 +176,7 @@ def double( def double( - a: int + a: int, ) -> ( int # Hello ): diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap index 4c226dd8c3d9a1..2dc81368793b15 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap @@ -253,7 +253,7 @@ except ( # d 9 def e1( # e 1 - x + x, ): pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 5b0684becdfa4c..7a8e97566ecb3c 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap @@ -928,7 +928,7 @@ def f( # first def f( # first - a + a, ): # second ... diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap index d32669cbbc5b66..d7e7f4074a6202 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap @@ -281,7 +281,7 @@ def double( def double( - a: int + a: int, ) -> ( # Hello int ): @@ -289,7 +289,7 @@ def double( def double( - a: int + a: int, ) -> ( # Hello ): return 2 * a @@ -298,7 +298,7 @@ def double( # Breaking over parameters and return types. (Black adds a trailing comma when the # function arguments break here with a single argument; we do not.) def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -310,13 +310,13 @@ def f( def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> a: ... def f( - a + a, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -334,13 +334,13 @@ def f[ def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> a: ... @@ -380,7 +380,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -388,7 +388,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -396,7 +396,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - *args + *args, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -431,13 +431,13 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... @@ -457,7 +457,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> X and Y and foooooooooooooooooooooooooooooooooooo(): ... @@ -477,7 +477,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> ( X | Y | foooooooooooooooooooooooooooooooooooo() # comment ): @@ -495,7 +495,7 @@ def double() -> ( # Dangling comments on return annotations. def double( - a: int + a: int, ) -> ( int # Hello ): @@ -503,7 +503,7 @@ def double( def double( - a: int + a: int, ) -> ( foo.bar # Hello ): @@ -511,7 +511,7 @@ def double( def double( - a: int + a: int, ) -> ( [int] # Hello ): @@ -519,7 +519,7 @@ def double( def double( - a: int + a: int, ) -> ( int | list[int] # Hello ): @@ -527,7 +527,7 @@ def double( def double( - a: int + a: int, ) -> ( int | list[ @@ -608,7 +608,7 @@ def process_board_action( @@ -95,50 +89,44 @@ # function arguments break here with a single argument; we do not.) def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -622,14 +622,14 @@ def process_board_action( def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> a: - ... +) -> a: ... def f( - a + a, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> ( @@ -652,14 +652,14 @@ def process_board_action( def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> a: - ... +) -> a: ... @@ -703,7 +703,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -712,7 +712,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -721,7 +721,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - *args + *args, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -761,14 +761,14 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: - ... +) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: - ... +) -> ( @@ -786,7 +786,7 @@ def process_board_action( -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> X + Y + foooooooooooooooooooooooooooooooooooo(): - ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( -+ x ++ x, +) -> X + Y + foooooooooooooooooooooooooooooooooooo(): ... @@ -798,7 +798,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> X and Y and foooooooooooooooooooooooooooooooooooo(): - ... +) -> X and Y and foooooooooooooooooooooooooooooooooooo(): ... @@ -814,7 +814,7 @@ def process_board_action( -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> X | Y | foooooooooooooooooooooooooooooooooooo(): - ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( -+ x ++ x, +) -> X | Y | foooooooooooooooooooooooooooooooooooo(): ... @@ -826,7 +826,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> ( X | Y | foooooooooooooooooooooooooooooooooooo() # comment -): From b2638c62a56678a357a19a1c4e94e4f6a8f0c31a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 21:57:05 -0500 Subject: [PATCH 5/8] Update formatter fixtures (#8935) I merged a branch that wasn't up-to-date, which left us with test failures on `main`. --- ...ns_with_braces_and_square_brackets.py.snap | 774 +++++++----------- ...__preview_long_strings__regression.py.snap | 127 ++- 2 files changed, 322 insertions(+), 579 deletions(-) diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_hug_parens_with_braces_and_square_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_hug_parens_with_braces_and_square_brackets.py.snap index d9a87a05c5987d..e28ce63ff0fea0 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_hug_parens_with_braces_and_square_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_hug_parens_with_braces_and_square_brackets.py.snap @@ -197,94 +197,7 @@ for foo in ["a", "b"]: ```diff --- Black +++ Ruff -@@ -1,43 +1,55 @@ - def foo_brackets(request): -- return JsonResponse({ -- "var_1": foo, -- "var_2": bar, -- }) -+ return JsonResponse( -+ { -+ "var_1": foo, -+ "var_2": bar, -+ } -+ ) - - - def foo_square_brackets(request): -- return JsonResponse([ -- "var_1", -- "var_2", -- ]) -+ return JsonResponse( -+ [ -+ "var_1", -+ "var_2", -+ ] -+ ) - - --func({ -- "a": 37, -- "b": 42, -- "c": 927, -- "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, --}) -+func( -+ { -+ "a": 37, -+ "b": 42, -+ "c": 927, -+ "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, -+ } -+) - --func([ -- "random_string_number_one", -- "random_string_number_two", -- "random_string_number_three", -- "random_string_number_four", --]) -+func( -+ [ -+ "random_string_number_one", -+ "random_string_number_two", -+ "random_string_number_three", -+ "random_string_number_four", -+ ] -+) - --func({ -- # expand me -- "a": 37, -- "b": 42, -- "c": 927, --}) -+func( -+ { -+ # expand me -+ "a": 37, -+ "b": 42, -+ "c": 927, -+ } -+) - --func([ -- "a", -- "b", -- "c", --]) -+func( -+ [ -+ "a", -+ "b", -+ "c", -+ ] -+) - - func( - [ -@@ -47,17 +59,21 @@ +@@ -47,17 +47,21 @@ ], ) @@ -316,67 +229,9 @@ for foo in ["a", "b"]: func( # preserve me -@@ -68,38 +84,48 @@ - ] - ) - --func([ # preserve me but hug brackets -- "c", -- "d", -- "e", --]) -+func( -+ [ # preserve me but hug brackets -+ "c", -+ "d", -+ "e", -+ ] -+) - --func([ -- # preserve me but hug brackets -- "c", -- "d", -- "e", --]) -+func( -+ [ -+ # preserve me but hug brackets -+ "c", -+ "d", -+ "e", -+ ] -+) - --func([ -- "c", -- # preserve me but hug brackets -- "d", -- "e", --]) -+func( -+ [ -+ "c", -+ # preserve me but hug brackets -+ "d", -+ "e", -+ ] -+) - --func([ -- "c", -- "d", -- "e", -- # preserve me but hug brackets --]) -+func( -+ [ -+ "c", -+ "d", -+ "e", -+ # preserve me but hug brackets -+ ] -+) +@@ -95,11 +99,13 @@ + # preserve me but hug brackets + ]) -func([ - "c", @@ -393,33 +248,24 @@ for foo in ["a", "b"]: func( [ -@@ -114,50 +140,68 @@ - func( - [x for x in "long line long line long line long line long line long line long line"] +@@ -111,10 +117,10 @@ ) --func([ -- x -- for x in [ -+func( -+ [ - x -- for x in "long line long line long line long line long line long line long line" -+ for x in [ -+ x -+ for x in "long line long line long line long line long line long line long line" -+ ] - ] --]) -+) - func({"short line"}) --func({ -- "long line", -- "long long line", -- "long long long line", -- "long long long long line", -- "long long long long long line", --}) + func([x for x in "short line"]) +-func( +- [x for x in "long line long line long line long line long line long line long line"] +-) + func([ ++ x for x in "long line long line long line long line long line long line long line" ++]) ++func([ + x + for x in [ + x +@@ -130,13 +136,15 @@ + "long long long long line", + "long long long long long line", + }) -func({{ - "long line", - "long long line", @@ -427,28 +273,7 @@ for foo in ["a", "b"]: - "long long long long line", - "long long long long long line", -}}) --func(( -- "long line", -- "long long line", -- "long long long line", -- "long long long long line", -- "long long long long long line", --)) --func((( -- "long line", -- "long long line", -- "long long long line", -- "long long long long line", -- "long long long long long line", --))) --func([[ -- "long line", -- "long long line", -- "long long long line", -- "long long long long line", -- "long long long long long line", --]]) -+func( ++func({ + { + "long line", + "long long line", @@ -456,53 +281,96 @@ for foo in ["a", "b"]: + "long long long long line", + "long long long long long line", + } -+) -+func( -+ { -+ { -+ "long line", -+ "long long line", -+ "long long long line", -+ "long long long long line", -+ "long long long long long line", -+ } -+ } -+) -+func( -+ ( ++}) + func(( + "long line", + "long long line", +@@ -151,30 +159,62 @@ + "long long long long line", + "long long long long long line", + ))) +-func([[ +- "long line", +- "long long line", +- "long long long line", +- "long long long long line", +- "long long long long long line", +-]]) ++func([ ++ [ + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", -+ ) -+) -+func( -+ ( -+ ( -+ "long line", -+ "long long line", -+ "long long long line", -+ "long long long long line", -+ "long long long long long line", -+ ) -+ ) -+) -+func( -+ [ -+ [ -+ "long line", -+ "long long line", -+ "long long long line", -+ "long long long long line", -+ "long long long long long line", -+ ] + ] -+) ++]) # Do not hug if the argument fits on a single line. - func( -@@ -194,18 +238,24 @@ +-func( +- {"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"} +-) +-func( +- ("fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line") +-) +-func( +- ["fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"] +-) +-func( +- **{"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit---"} +-) +-func( +- *("fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit----") +-) ++func({ ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++}) ++func(( ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++)) ++func([ ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++]) ++func(**{ ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit---", ++}) ++func(*( ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit line", ++ "fit----", ++)) + array = [ + {"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"} + ] +@@ -194,18 +234,24 @@ ) nested_mapping = { @@ -538,148 +406,64 @@ for foo in ["a", "b"]: explicit_exploding = [ [ [ -@@ -214,30 +264,42 @@ - ], - ], - ] --single_item_do_not_explode = Context({ -- "version": get_docs_version(), --}) -+single_item_do_not_explode = Context( -+ { -+ "version": get_docs_version(), -+ } -+) - --foo(*[ -- "long long long long long line", -- "long long long long long line", -- "long long long long long line", --]) -+foo( -+ *[ -+ "long long long long long line", -+ "long long long long long line", -+ "long long long long long line", -+ ] -+) - --foo(*[ -- str(i) for i in range(100000000000000000000000000000000000000000000000000000000000) --]) -+foo( -+ *[ -+ str(i) -+ for i in range(100000000000000000000000000000000000000000000000000000000000) -+ ] -+) - --foo(**{ -- "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": 1, -- "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": 2, -- "ccccccccccccccccccccccccccccccccc": 3, -- **other, --}) -+foo( -+ **{ -+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": 1, -+ "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": 2, -+ "ccccccccccccccccccccccccccccccccc": 3, -+ **other, -+ } -+) - --foo(**{ -- x: y for x, y in enumerate(["long long long long line", "long long long long line"]) --}) -+foo( -+ **{ -+ x: y -+ for x, y in enumerate(["long long long long line", "long long long long line"]) -+ } -+) +@@ -240,9 +286,9 @@ + }) # Edge case when deciding whether to hug the brackets without inner content. - very_very_very_long_variable = very_very_very_long_module.VeryVeryVeryVeryLongClassName( -@@ -245,11 +307,13 @@ - ) +-very_very_very_long_variable = very_very_very_long_module.VeryVeryVeryVeryLongClassName( +- [[]] +-) ++very_very_very_long_variable = very_very_very_long_module.VeryVeryVeryVeryLongClassName([ ++ [] ++]) for foo in ["a", "b"]: -- output.extend([ -- individual -- for -- # Foobar -- container in xs_by_y[foo] -- # Foobar -- for individual in container["nested"] -- ]) -+ output.extend( -+ [ -+ individual -+ for -+ # Foobar -+ container in xs_by_y[foo] -+ # Foobar -+ for individual in container["nested"] -+ ] -+ ) + output.extend([ ``` ## Ruff Output ```python def foo_brackets(request): - return JsonResponse( - { - "var_1": foo, - "var_2": bar, - } - ) + return JsonResponse({ + "var_1": foo, + "var_2": bar, + }) def foo_square_brackets(request): - return JsonResponse( - [ - "var_1", - "var_2", - ] - ) + return JsonResponse([ + "var_1", + "var_2", + ]) -func( - { - "a": 37, - "b": 42, - "c": 927, - "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, - } -) +func({ + "a": 37, + "b": 42, + "c": 927, + "aaaaaaaaaaaaaaaaaaaaaaaaa": 11111111111111111111111111111111111111111, +}) -func( - [ - "random_string_number_one", - "random_string_number_two", - "random_string_number_three", - "random_string_number_four", - ] -) +func([ + "random_string_number_one", + "random_string_number_two", + "random_string_number_three", + "random_string_number_four", +]) -func( - { - # expand me - "a": 37, - "b": 42, - "c": 927, - } -) +func({ + # expand me + "a": 37, + "b": 42, + "c": 927, +}) -func( - [ - "a", - "b", - "c", - ] -) +func([ + "a", + "b", + "c", +]) func( [ @@ -714,40 +498,32 @@ func( ] ) -func( - [ # preserve me but hug brackets - "c", - "d", - "e", - ] -) +func([ # preserve me but hug brackets + "c", + "d", + "e", +]) -func( - [ - # preserve me but hug brackets - "c", - "d", - "e", - ] -) +func([ + # preserve me but hug brackets + "c", + "d", + "e", +]) -func( - [ - "c", - # preserve me but hug brackets - "d", - "e", - ] -) +func([ + "c", + # preserve me but hug brackets + "d", + "e", +]) -func( - [ - "c", - "d", - "e", - # preserve me but hug brackets - ] -) +func([ + "c", + "d", + "e", + # preserve me but hug brackets +]) func( [ @@ -767,21 +543,26 @@ func( ) func([x for x in "short line"]) -func( - [x for x in "long line long line long line long line long line long line long line"] -) -func( - [ +func([ + x for x in "long line long line long line long line long line long line long line" +]) +func([ + x + for x in [ x - for x in [ - x - for x in "long line long line long line long line long line long line long line" - ] + for x in "long line long line long line long line long line long line long line" ] -) +]) func({"short line"}) -func( +func({ + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +}) +func({ { "long line", "long long line", @@ -789,66 +570,77 @@ func( "long long long long line", "long long long long long line", } -) -func( - { - { - "long line", - "long long line", - "long long long line", - "long long long long line", - "long long long long long line", - } - } -) -func( - ( +}) +func(( + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +)) +func((( + "long line", + "long long line", + "long long long line", + "long long long long line", + "long long long long long line", +))) +func([ + [ "long line", "long long line", "long long long line", "long long long long line", "long long long long long line", - ) -) -func( - ( - ( - "long line", - "long long line", - "long long long line", - "long long long long line", - "long long long long long line", - ) - ) -) -func( - [ - [ - "long line", - "long long line", - "long long long line", - "long long long long line", - "long long long long long line", - ] ] -) +]) # Do not hug if the argument fits on a single line. -func( - {"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"} -) -func( - ("fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line") -) -func( - ["fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"] -) -func( - **{"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit---"} -) -func( - *("fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit----") -) +func({ + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", +}) +func(( + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", +)) +func([ + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", +]) +func(**{ + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit---", +}) +func(*( + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit line", + "fit----", +)) array = [ {"fit line", "fit line", "fit line", "fit line", "fit line", "fit line", "fit line"} ] @@ -894,59 +686,45 @@ explicit_exploding = [ ], ], ] -single_item_do_not_explode = Context( - { - "version": get_docs_version(), - } -) +single_item_do_not_explode = Context({ + "version": get_docs_version(), +}) -foo( - *[ - "long long long long long line", - "long long long long long line", - "long long long long long line", - ] -) +foo(*[ + "long long long long long line", + "long long long long long line", + "long long long long long line", +]) -foo( - *[ - str(i) - for i in range(100000000000000000000000000000000000000000000000000000000000) - ] -) +foo(*[ + str(i) for i in range(100000000000000000000000000000000000000000000000000000000000) +]) -foo( - **{ - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": 1, - "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": 2, - "ccccccccccccccccccccccccccccccccc": 3, - **other, - } -) +foo(**{ + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": 1, + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": 2, + "ccccccccccccccccccccccccccccccccc": 3, + **other, +}) -foo( - **{ - x: y - for x, y in enumerate(["long long long long line", "long long long long line"]) - } -) +foo(**{ + x: y for x, y in enumerate(["long long long long line", "long long long long line"]) +}) # Edge case when deciding whether to hug the brackets without inner content. -very_very_very_long_variable = very_very_very_long_module.VeryVeryVeryVeryLongClassName( - [[]] -) +very_very_very_long_variable = very_very_very_long_module.VeryVeryVeryVeryLongClassName([ + [] +]) for foo in ["a", "b"]: - output.extend( - [ - individual - for - # Foobar - container in xs_by_y[foo] - # Foobar - for individual in container["nested"] - ] - ) + output.extend([ + individual + for + # Foobar + container in xs_by_y[foo] + # Foobar + for individual in container["nested"] + ]) ``` ## Black Output diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap index e05940e360ce9f..1cde924609d48b 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_strings__regression.py.snap @@ -573,7 +573,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ```diff --- Black +++ Ruff -@@ -25,41 +25,42 @@ +@@ -25,20 +25,17 @@ "Jaguar", ) @@ -599,27 +599,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) - class A: - def foo(): -- XXXXXXXXXXXX.append(( -- "xxx_xxxxxxxxxx(xxxxx={}, xxxx={}, xxxxx, xxxx_xxxx_xxxxxxxxxx={})".format( -- xxxxx, xxxx, xxxx_xxxx_xxxxxxxxxx -- ), -- my_var, -- my_other_var, -- )) -+ XXXXXXXXXXXX.append( -+ ( -+ "xxx_xxxxxxxxxx(xxxxx={}, xxxx={}, xxxxx, xxxx_xxxx_xxxxxxxxxx={})".format( -+ xxxxx, xxxx, xxxx_xxxx_xxxxxxxxxx -+ ), -+ my_var, -+ my_other_var, -+ ) -+ ) - - - class A: +@@ -57,9 +54,11 @@ class B: def foo(): bar( @@ -634,7 +614,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ), varX, varY, -@@ -70,9 +71,10 @@ +@@ -70,9 +69,10 @@ def foo(xxxx): for xxx_xxxx, _xxx_xxx, _xxx_xxxxx, xxx_xxxx in xxxx: for xxx in xxx_xxxx: @@ -648,7 +628,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) -@@ -80,10 +82,11 @@ +@@ -80,10 +80,11 @@ def disappearing_comment(): return ( ( # xx -x xxxxxxx xx xxx xxxxxxx. @@ -662,7 +642,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: "--xxxxxxx --xxxxxx=x --xxxxxx-xxxxx=xxxxxx" " --xxxxxx-xxxx=xxxxxxxxxxx.xxx" ) -@@ -113,18 +116,25 @@ +@@ -113,18 +114,25 @@ func_call_where_string_arg_has_method_call_and_bad_parens( @@ -694,7 +674,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) -@@ -132,52 +142,60 @@ +@@ -132,52 +140,60 @@ def append(self): if True: xxxx.xxxxxxx.xxxxx( @@ -788,7 +768,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: } -@@ -185,10 +203,10 @@ +@@ -185,10 +201,10 @@ def foo(self): if True: xxxxx_xxxxxxxxxxxx( @@ -803,7 +783,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) -@@ -232,39 +250,24 @@ +@@ -232,39 +248,24 @@ some_dictionary = { "xxxxx006": [ @@ -852,7 +832,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: some_commented_string = ( # This comment stays at the top. "This string is long but not so long that it needs hahahah toooooo be so greatttt" -@@ -279,36 +282,25 @@ +@@ -279,38 +280,27 @@ ) lpar_and_rpar_have_comments = func_call( # LPAR Comment @@ -892,12 +872,14 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: - f" certainly, absolutely {does}." + f"We have to remember to escape {braces}." " Like {these}." f" But not {this}." ) -- --fstring = f"We have to remember to escape {braces}. Like {{these}}. But not {this}." +-fstring = f"We have to remember to escape {braces}. Like {{these}}. But not {this}." +- class A: -@@ -364,10 +356,7 @@ + class B: + def foo(): +@@ -364,10 +354,7 @@ def foo(): if not hasattr(module, name): raise ValueError( @@ -909,7 +891,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: % (name, module_name, get_docs_version()) ) -@@ -382,35 +371,33 @@ +@@ -382,23 +369,19 @@ class Step(StepBase): def who(self): @@ -931,29 +913,16 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) --xxxxxxx_xxxxxx_xxxxxxx = xxx([ -- xxxxxxxxxxxx( -- xxxxxx_xxxxxxx=( + xxxxxxx_xxxxxx_xxxxxxx = xxx([ + xxxxxxxxxxxx( + xxxxxx_xxxxxxx=( - '((x.aaaaaaaaa = "xxxxxx.xxxxxxxxxxxxxxxxxxxxx") || (x.xxxxxxxxx =' - ' "xxxxxxxxxxxx")) && ' -- # xxxxx xxxxxxxxxxxx xxxx xxx (xxxxxxxxxxxxxxxx) xx x xxxxxxxxx xx xxxxxx. -- "(x.bbbbbbbbbbbb.xxx != " -- '"xxx:xxx:xxx::cccccccccccc:xxxxxxx-xxxx/xxxxxxxxxxx/xxxxxxxxxxxxxxxxx") && ' -+xxxxxxx_xxxxxx_xxxxxxx = xxx( -+ [ -+ xxxxxxxxxxxx( -+ xxxxxx_xxxxxxx=( -+ '((x.aaaaaaaaa = "xxxxxx.xxxxxxxxxxxxxxxxxxxxx") || (x.xxxxxxxxx = "xxxxxxxxxxxx")) && ' -+ # xxxxx xxxxxxxxxxxx xxxx xxx (xxxxxxxxxxxxxxxx) xx x xxxxxxxxx xx xxxxxx. -+ "(x.bbbbbbbbbbbb.xxx != " -+ '"xxx:xxx:xxx::cccccccccccc:xxxxxxx-xxxx/xxxxxxxxxxx/xxxxxxxxxxxxxxxxx") && ' -+ ) - ) -- ) --]) -+ ] -+) - ++ '((x.aaaaaaaaa = "xxxxxx.xxxxxxxxxxxxxxxxxxxxx") || (x.xxxxxxxxx = "xxxxxxxxxxxx")) && ' + # xxxxx xxxxxxxxxxxx xxxx xxx (xxxxxxxxxxxxxxxx) xx x xxxxxxxxx xx xxxxxx. + "(x.bbbbbbbbbbbb.xxx != " + '"xxx:xxx:xxx::cccccccccccc:xxxxxxx-xxxx/xxxxxxxxxxx/xxxxxxxxxxxxxxxxx") && ' +@@ -409,8 +392,8 @@ if __name__ == "__main__": for i in range(4, 8): cmd = ( @@ -964,7 +933,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) -@@ -432,14 +419,12 @@ +@@ -432,14 +415,12 @@ assert xxxxxxx_xxxx in [ x.xxxxx.xxxxxx.xxxxx.xxxxxx, x.xxxxx.xxxxxx.xxxxx.xxxx, @@ -983,7 +952,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: RE_ONE_BACKSLASH = { "asdf_hjkl_jkl": re.compile( -@@ -449,8 +434,7 @@ +@@ -449,8 +430,7 @@ RE_TWO_BACKSLASHES = { "asdf_hjkl_jkl": re.compile( @@ -993,7 +962,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ), } -@@ -462,13 +446,9 @@ +@@ -462,13 +442,9 @@ # We do NOT split on f-string expressions. print( @@ -1009,7 +978,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: # The parens should NOT be removed in this case. ( -@@ -478,8 +458,8 @@ +@@ -478,8 +454,8 @@ # The parens should NOT be removed in this case. ( @@ -1020,7 +989,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) # The parens should NOT be removed in this case. -@@ -518,88 +498,78 @@ +@@ -518,88 +494,78 @@ f"<<{author.display_name}>>\n" ) @@ -1144,7 +1113,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: "6. Click on Create Credential at the top." '7. At the top click the link for "API key".' "8. No application restrictions are needed. Click Create at the bottom." -@@ -613,55 +583,40 @@ +@@ -613,55 +579,40 @@ f"<<{author.display_name}>>\n" ) @@ -1217,7 +1186,7 @@ s = f'Lorem Ipsum is simply dummy text of the printing and typesetting industry: ) # Regression test for https://github.com/psf/black/issues/3455. -@@ -672,9 +627,11 @@ +@@ -672,9 +623,11 @@ } # Regression test for https://github.com/psf/black/issues/3506. @@ -1281,15 +1250,13 @@ class A: class A: def foo(): - XXXXXXXXXXXX.append( - ( - "xxx_xxxxxxxxxx(xxxxx={}, xxxx={}, xxxxx, xxxx_xxxx_xxxxxxxxxx={})".format( - xxxxx, xxxx, xxxx_xxxx_xxxxxxxxxx - ), - my_var, - my_other_var, - ) - ) + XXXXXXXXXXXX.append(( + "xxx_xxxxxxxxxx(xxxxx={}, xxxx={}, xxxxx, xxxx_xxxx_xxxxxxxxxx={})".format( + xxxxx, xxxx, xxxx_xxxx_xxxxxxxxxx + ), + my_var, + my_other_var, + )) class A: @@ -1620,18 +1587,16 @@ class Step(StepBase): ) -xxxxxxx_xxxxxx_xxxxxxx = xxx( - [ - xxxxxxxxxxxx( - xxxxxx_xxxxxxx=( - '((x.aaaaaaaaa = "xxxxxx.xxxxxxxxxxxxxxxxxxxxx") || (x.xxxxxxxxx = "xxxxxxxxxxxx")) && ' - # xxxxx xxxxxxxxxxxx xxxx xxx (xxxxxxxxxxxxxxxx) xx x xxxxxxxxx xx xxxxxx. - "(x.bbbbbbbbbbbb.xxx != " - '"xxx:xxx:xxx::cccccccccccc:xxxxxxx-xxxx/xxxxxxxxxxx/xxxxxxxxxxxxxxxxx") && ' - ) +xxxxxxx_xxxxxx_xxxxxxx = xxx([ + xxxxxxxxxxxx( + xxxxxx_xxxxxxx=( + '((x.aaaaaaaaa = "xxxxxx.xxxxxxxxxxxxxxxxxxxxx") || (x.xxxxxxxxx = "xxxxxxxxxxxx")) && ' + # xxxxx xxxxxxxxxxxx xxxx xxx (xxxxxxxxxxxxxxxx) xx x xxxxxxxxx xx xxxxxx. + "(x.bbbbbbbbbbbb.xxx != " + '"xxx:xxx:xxx::cccccccccccc:xxxxxxx-xxxx/xxxxxxxxxxx/xxxxxxxxxxxxxxxxx") && ' ) - ] -) + ) +]) if __name__ == "__main__": for i in range(4, 8): From 912c39ce2a540d52a435df907409b5660ba9d049 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 22:04:58 -0500 Subject: [PATCH 6/8] Add support for `@functools.singledispatch` (#8934) ## Summary When a function uses `@functools.singledispatch`, we need to treat the first argument of any implementations as runtime-required. Closes https://github.com/astral-sh/ruff/issues/6849. --- .../flake8_type_checking/singledispatch.py | 34 +++++ crates/ruff_linter/src/checkers/ast/mod.rs | 45 ++++--- .../src/rules/flake8_type_checking/helpers.rs | 116 ++++++++++++++---- .../src/rules/flake8_type_checking/mod.rs | 1 + ...-third-party-import_singledispatch.py.snap | 27 ++++ 5 files changed, 183 insertions(+), 40 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_type_checking/singledispatch.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_singledispatch.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/singledispatch.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/singledispatch.py new file mode 100644 index 00000000000000..a519a58b4f15bf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/singledispatch.py @@ -0,0 +1,34 @@ +"""Test module.""" +from __future__ import annotations + +from functools import singledispatch +from typing import TYPE_CHECKING + +from numpy import asarray +from numpy.typing import ArrayLike +from scipy.sparse import spmatrix +from pandas import DataFrame + +if TYPE_CHECKING: + from numpy import ndarray + + +@singledispatch +def to_array_or_mat(a: ArrayLike | spmatrix) -> ndarray | spmatrix: + """Convert arg to array or leaves it as sparse matrix.""" + msg = f"Unhandled type {type(a)}" + raise NotImplementedError(msg) + + +@to_array_or_mat.register +def _(a: ArrayLike) -> ndarray: + return asarray(a) + + +@to_array_or_mat.register +def _(a: spmatrix) -> spmatrix: + return a + + +def _(a: DataFrame) -> DataFrame: + return a diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index d24154badb6f7d..11e8e704aa02fa 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -492,6 +492,13 @@ where // are enabled. let runtime_annotation = !self.semantic.future_annotations(); + // The first parameter may be a single dispatch. + let mut singledispatch = + flake8_type_checking::helpers::is_singledispatch_implementation( + function_def, + self.semantic(), + ); + self.semantic.push_scope(ScopeKind::Type); if let Some(type_params) = type_params { @@ -505,7 +512,7 @@ where .chain(¶meters.kwonlyargs) { if let Some(expr) = ¶meter_with_default.parameter.annotation { - if runtime_annotation { + if runtime_annotation || singledispatch { self.visit_runtime_annotation(expr); } else { self.visit_annotation(expr); @@ -514,6 +521,7 @@ where if let Some(expr) = ¶meter_with_default.default { self.visit_expr(expr); } + singledispatch = false; } if let Some(arg) = ¶meters.vararg { if let Some(expr) = &arg.annotation { @@ -670,23 +678,24 @@ where // available at runtime. // See: https://docs.python.org/3/reference/simple_stmts.html#annotated-assignment-statements let runtime_annotation = if self.semantic.future_annotations() { - if self.semantic.current_scope().kind.is_class() { - let baseclasses = &self - .settings - .flake8_type_checking - .runtime_evaluated_base_classes; - let decorators = &self - .settings - .flake8_type_checking - .runtime_evaluated_decorators; - flake8_type_checking::helpers::runtime_evaluated( - baseclasses, - decorators, - &self.semantic, - ) - } else { - false - } + self.semantic + .current_scope() + .kind + .as_class() + .is_some_and(|class_def| { + flake8_type_checking::helpers::runtime_evaluated_class( + class_def, + &self + .settings + .flake8_type_checking + .runtime_evaluated_base_classes, + &self + .settings + .flake8_type_checking + .runtime_evaluated_decorators, + &self.semantic, + ) + }) } else { matches!( self.semantic.current_scope().kind, diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index d07fe2d6cb1f27..0a51e151f4703c 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -1,7 +1,7 @@ use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::helpers::{map_callable, map_subscript}; -use ruff_python_ast::{self as ast}; -use ruff_python_semantic::{Binding, BindingId, BindingKind, ScopeKind, SemanticModel}; +use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::{Binding, BindingId, BindingKind, SemanticModel}; use rustc_hash::FxHashSet; pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticModel) -> bool { @@ -18,25 +18,26 @@ pub(crate) fn is_valid_runtime_import(binding: &Binding, semantic: &SemanticMode } } -pub(crate) fn runtime_evaluated( +pub(crate) fn runtime_evaluated_class( + class_def: &ast::StmtClassDef, base_classes: &[String], decorators: &[String], semantic: &SemanticModel, ) -> bool { - if !base_classes.is_empty() { - if runtime_evaluated_base_class(base_classes, semantic) { - return true; - } + if runtime_evaluated_base_class(class_def, base_classes, semantic) { + return true; } - if !decorators.is_empty() { - if runtime_evaluated_decorators(decorators, semantic) { - return true; - } + if runtime_evaluated_decorators(class_def, decorators, semantic) { + return true; } false } -fn runtime_evaluated_base_class(base_classes: &[String], semantic: &SemanticModel) -> bool { +fn runtime_evaluated_base_class( + class_def: &ast::StmtClassDef, + base_classes: &[String], + semantic: &SemanticModel, +) -> bool { fn inner( class_def: &ast::StmtClassDef, base_classes: &[String], @@ -78,19 +79,21 @@ fn runtime_evaluated_base_class(base_classes: &[String], semantic: &SemanticMode }) } - semantic - .current_scope() - .kind - .as_class() - .is_some_and(|class_def| { - inner(class_def, base_classes, semantic, &mut FxHashSet::default()) - }) + if base_classes.is_empty() { + return false; + } + + inner(class_def, base_classes, semantic, &mut FxHashSet::default()) } -fn runtime_evaluated_decorators(decorators: &[String], semantic: &SemanticModel) -> bool { - let ScopeKind::Class(class_def) = &semantic.current_scope().kind else { +fn runtime_evaluated_decorators( + class_def: &ast::StmtClassDef, + decorators: &[String], + semantic: &SemanticModel, +) -> bool { + if decorators.is_empty() { return false; - }; + } class_def.decorator_list.iter().any(|decorator| { semantic @@ -102,3 +105,72 @@ fn runtime_evaluated_decorators(decorators: &[String], semantic: &SemanticModel) }) }) } + +/// Returns `true` if a function is registered as a `singledispatch` interface. +/// +/// For example, `fun` below is a `singledispatch` interface: +/// ```python +/// from functools import singledispatch +/// +/// @singledispatch +/// def fun(arg, verbose=False): +/// ... +/// ``` +pub(crate) fn is_singledispatch_interface( + function_def: &ast::StmtFunctionDef, + semantic: &SemanticModel, +) -> bool { + function_def.decorator_list.iter().any(|decorator| { + semantic + .resolve_call_path(&decorator.expression) + .is_some_and(|call_path| { + matches!(call_path.as_slice(), ["functools", "singledispatch"]) + }) + }) +} + +/// Returns `true` if a function is registered as a `singledispatch` implementation. +/// +/// For example, `_` below is a `singledispatch` implementation: +/// For example: +/// ```python +/// from functools import singledispatch +/// +/// @singledispatch +/// def fun(arg, verbose=False): +/// ... +/// +/// @fun.register +/// def _(arg: int, verbose=False): +/// ... +/// ``` +pub(crate) fn is_singledispatch_implementation( + function_def: &ast::StmtFunctionDef, + semantic: &SemanticModel, +) -> bool { + function_def.decorator_list.iter().any(|decorator| { + let Expr::Attribute(attribute) = &decorator.expression else { + return false; + }; + + if attribute.attr.as_str() != "register" { + return false; + }; + + let Some(id) = semantic.lookup_attribute(attribute.value.as_ref()) else { + return false; + }; + + let binding = semantic.binding(id); + let Some(function_def) = binding + .kind + .as_function_definition() + .map(|id| &semantic.scopes[*id]) + .and_then(|scope| scope.kind.as_function()) + else { + return false; + }; + + is_singledispatch_interface(function_def, semantic) + }) +} diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 97e9ad7cd4af0a..82b24755f4277a 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -37,6 +37,7 @@ mod tests { #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TCH003.py"))] #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("TCH002.py"))] + #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("singledispatch.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_1.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_2.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_singledispatch.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_singledispatch.py.snap new file mode 100644 index 00000000000000..5b646c6b34011e --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-third-party-import_singledispatch.py.snap @@ -0,0 +1,27 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +singledispatch.py:10:20: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block + | + 8 | from numpy.typing import ArrayLike + 9 | from scipy.sparse import spmatrix +10 | from pandas import DataFrame + | ^^^^^^^^^ TCH002 +11 | +12 | if TYPE_CHECKING: + | + = help: Move into type-checking block + +ℹ Unsafe fix +7 7 | from numpy import asarray +8 8 | from numpy.typing import ArrayLike +9 9 | from scipy.sparse import spmatrix +10 |-from pandas import DataFrame +11 10 | +12 11 | if TYPE_CHECKING: + 12 |+ from pandas import DataFrame +13 13 | from numpy import ndarray +14 14 | +15 15 | + + From 46a174a22efedd5b8d97942a0f304b01c7e28ccd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 22:09:18 -0500 Subject: [PATCH 7/8] Use full arguments range for zero-sleep-call (#8936) --- .../test/fixtures/flake8_trio/TRIO115.py | 5 +++ .../flake8_trio/rules/zero_sleep_call.rs | 6 ++- ...lake8_trio__tests__TRIO115_TRIO115.py.snap | 40 ++++++++++++++++--- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py index 410408576acc68..aa25cb8e5a3aec 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py @@ -26,5 +26,10 @@ def func(): from trio import Event, sleep + def func(): sleep(0) # TRIO115 + + +async def func(): + await sleep(seconds=0) # TRIO115 diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs index 6ddd3d572d0532..38470abbf99ae4 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs @@ -16,12 +16,16 @@ use crate::importer::ImportRequest; /// /// ## Example /// ```python +/// import trio +/// /// async def func(): /// await trio.sleep(0) /// ``` /// /// Use instead: /// ```python +/// import trio +/// /// async def func(): /// await trio.lowlevel.checkpoint() /// ``` @@ -103,7 +107,7 @@ pub(crate) fn zero_sleep_call(checker: &mut Checker, call: &ExprCall) { )?; let reference_edit = Edit::range_replacement(format!("{binding}.checkpoint"), call.func.range()); - let arg_edit = Edit::range_deletion(arg.range()); + let arg_edit = Edit::range_replacement("()".to_string(), call.arguments.range()); Ok(Fix::safe_edits(import_edit, [reference_edit, arg_edit])) }); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap index 6b4c34efd1197f..0dfeef7c653fb4 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap @@ -85,10 +85,10 @@ TRIO115.py:17:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 19 19 | bar = "bar" 20 20 | trio.sleep(bar) -TRIO115.py:30:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` +TRIO115.py:31:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` | -29 | def func(): -30 | sleep(0) # TRIO115 +30 | def func(): +31 | sleep(0) # TRIO115 | ^^^^^^^^ TRIO115 | = help: Replace with `trio.lowlevel.checkpoint()` @@ -100,8 +100,36 @@ TRIO115.py:30:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 27 |-from trio import Event, sleep 27 |+from trio import Event, sleep, lowlevel 28 28 | -29 29 | def func(): -30 |- sleep(0) # TRIO115 - 30 |+ lowlevel.checkpoint() # TRIO115 +29 29 | +30 30 | def func(): +31 |- sleep(0) # TRIO115 + 31 |+ lowlevel.checkpoint() # TRIO115 +32 32 | +33 33 | +34 34 | async def func(): + +TRIO115.py:35:11: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +34 | async def func(): +35 | await sleep(seconds=0) # TRIO115 + | ^^^^^^^^^^^^^^^^ TRIO115 + | + = help: Replace with `trio.lowlevel.checkpoint()` + +ℹ Safe fix +24 24 | trio.run(trio.sleep(0)) # TRIO115 +25 25 | +26 26 | +27 |-from trio import Event, sleep + 27 |+from trio import Event, sleep, lowlevel +28 28 | +29 29 | +30 30 | def func(): +-------------------------------------------------------------------------------- +32 32 | +33 33 | +34 34 | async def func(): +35 |- await sleep(seconds=0) # TRIO115 + 35 |+ await lowlevel.checkpoint() # TRIO115 From 69dfe0a207a2462a71df87dc87eb19fe8018a17a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 22:34:09 -0500 Subject: [PATCH 8/8] Fix doc formatting for zero-sleep-call (#8937) --- .../ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs index 38470abbf99ae4..6b0e57569c4439 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs @@ -18,6 +18,7 @@ use crate::importer::ImportRequest; /// ```python /// import trio /// +/// /// async def func(): /// await trio.sleep(0) /// ``` @@ -26,6 +27,7 @@ use crate::importer::ImportRequest; /// ```python /// import trio /// +/// /// async def func(): /// await trio.lowlevel.checkpoint() /// ```