diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index e96b25f5fd..b880f84d4c 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -34,6 +34,7 @@ jobs: args: --out dist - name: "Test sdist" run: | + rustup default $(cat rust-toolchain) pip install dist/${{ env.PACKAGE_NAME }}-*.tar.gz --force-reinstall ruff --help python -m ruff --help diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf2933368a..572edd2781 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,12 @@ fail_fast: true + +exclude: | + (?x)^( + crates/ruff/resources/.*| + crates/ruff_python_formatter/resources/.*| + crates/ruff_python_formatter/src/snapshots/.* + )$ + repos: - repo: https://github.com/abravalheri/validate-pyproject rev: v0.12.1 @@ -19,7 +27,7 @@ repos: - id: markdownlint-fix - repo: https://github.com/crate-ci/typos - rev: v1.14.8 + rev: v1.14.12 hooks: - id: typos diff --git a/Cargo.lock b/Cargo.lock index 861a87f509..e74010d357 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -691,7 +691,7 @@ dependencies = [ [[package]] name = "flake8-to-ruff" -version = "0.0.270" +version = "0.0.271" dependencies = [ "anyhow", "clap", @@ -1725,7 +1725,7 @@ dependencies = [ [[package]] name = "ruff" -version = "0.0.270" +version = "0.0.271" dependencies = [ "annotate-snippets 0.9.1", "anyhow", @@ -1818,7 +1818,7 @@ dependencies = [ [[package]] name = "ruff_cli" -version = "0.0.270" +version = "0.0.271" dependencies = [ "annotate-snippets 0.9.1", "anyhow", diff --git a/README.md b/README.md index a562d347a1..314392fb87 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ Ruff can also be used as a [pre-commit](https://pre-commit.com) hook: ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.0.270 + rev: v0.0.271 hooks: - id: ruff ``` diff --git a/_typos.toml b/_typos.toml index 8f5f834f9b..778ba59eaf 100644 --- a/_typos.toml +++ b/_typos.toml @@ -1,5 +1,5 @@ [files] -extend-exclude = ["snapshots", "black"] +extend-exclude = ["resources", "snapshots"] [default.extend-words] trivias = "trivias" diff --git a/crates/flake8_to_ruff/Cargo.toml b/crates/flake8_to_ruff/Cargo.toml index 52596b641d..5df10eae89 100644 --- a/crates/flake8_to_ruff/Cargo.toml +++ b/crates/flake8_to_ruff/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flake8-to-ruff" -version = "0.0.270" +version = "0.0.271" edition = { workspace = true } rust-version = { workspace = true } diff --git a/crates/ruff/Cargo.toml b/crates/ruff/Cargo.toml index b27307b5f9..15409936e4 100644 --- a/crates/ruff/Cargo.toml +++ b/crates/ruff/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ruff" -version = "0.0.270" +version = "0.0.271" authors.workspace = true edition.workspace = true rust-version.workspace = true diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F522.py b/crates/ruff/resources/test/fixtures/pyflakes/F522.py index f84a28d53e..18e3d072f2 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F522.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F522.py @@ -2,3 +2,6 @@ "{bar}{}".format(1, bar=2, spam=3) # F522 "{bar:{spam}}".format(bar=2, spam=3) # No issues "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522 +# Not fixable +('' + .format(x=2)) \ No newline at end of file diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py b/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py index 89bae2b6ef..28d5af1f3b 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py @@ -83,6 +83,11 @@ def f(): pass +def f(): + with (Nested(m)) as (cm): + pass + + def f(): toplevel = tt = lexer.get_token() if not tt: diff --git a/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py b/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py index a27313b4d8..1b22612e52 100644 --- a/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py +++ b/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py @@ -3,12 +3,6 @@ for item in {"apples", "lemons", "water"}: # flags in-line set literals print(f"I like {item}.") -for item in set(("apples", "lemons", "water")): # flags set() calls - print(f"I like {item}.") - -for number in {i for i in range(10)}: # flags set comprehensions - print(number) - numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions @@ -36,3 +30,9 @@ numbers_set = {i for i in (1, 2, 3)} # tuples in comprehensions are fine numbers_dict = {str(i): i for i in [1, 2, 3]} # lists in dict comprehensions are fine numbers_gen = (i for i in (1, 2, 3)) # tuples in generator expressions are fine + +for item in set(("apples", "lemons", "water")): # set constructor is fine + print(f"I like {item}.") + +for number in {i for i in range(10)}: # set comprehensions are fine + print(number) diff --git a/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py b/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py index baa823565b..154ee23bec 100644 --- a/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py +++ b/crates/ruff/resources/test/fixtures/pylint/nonlocal_without_binding.py @@ -17,3 +17,30 @@ def f(): def f(): nonlocal y + + +def f(): + x = 1 + + def g(): + nonlocal x + + del x + + +def f(): + def g(): + nonlocal x + + del x + + +def f(): + try: + pass + except Exception as x: + pass + + def g(): + nonlocal x + x = 2 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 3e1e87359a..100c58c5e4 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -307,9 +307,18 @@ where stmt.range(), ExecutionContext::Runtime, ); - } else { - // Ensure that every nonlocal has an existing binding from a parent scope. - if self.enabled(Rule::NonlocalWithoutBinding) { + } + + // Ensure that every nonlocal has an existing binding from a parent scope. + if self.enabled(Rule::NonlocalWithoutBinding) { + if self + .semantic_model + .scopes + .ancestors(self.semantic_model.scope_id) + .skip(1) + .take_while(|scope| !scope.kind.is_module()) + .all(|scope| !scope.declares(name.as_str())) + { self.diagnostics.push(Diagnostic::new( pylint::rules::NonlocalWithoutBinding { name: name.to_string(), @@ -4042,7 +4051,7 @@ where let name_range = helpers::excepthandler_name_range(excepthandler, self.locator).unwrap(); - if self.semantic_model.scope().defines(name) { + if self.semantic_model.scope().has(name) { self.handle_node_store( name, &Expr::Name(ast::ExprName { @@ -4067,7 +4076,7 @@ where if let Some(binding_id) = { let scope = self.semantic_model.scope_mut(); - scope.remove(name) + scope.delete(name) } { if !self.semantic_model.is_used(binding_id) { if self.enabled(Rule::UnusedVariable) { @@ -4697,19 +4706,16 @@ impl<'a> Checker<'a> { } let scope = self.semantic_model.scope_mut(); - if scope.remove(id.as_str()).is_some() { - return; + if scope.delete(id.as_str()).is_none() { + if self.enabled(Rule::UndefinedName) { + self.diagnostics.push(Diagnostic::new( + pyflakes::rules::UndefinedName { + name: id.to_string(), + }, + expr.range(), + )); + } } - if !self.enabled(Rule::UndefinedName) { - return; - } - - self.diagnostics.push(Diagnostic::new( - pyflakes::rules::UndefinedName { - name: id.to_string(), - }, - expr.range(), - )); } fn check_deferred_future_type_definitions(&mut self) { @@ -4977,7 +4983,7 @@ impl<'a> Checker<'a> { .collect(); if !sources.is_empty() { for (name, range) in &exports { - if !scope.defines(name) { + if !scope.has(name) { diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedLocalWithImportStarUsage { name: (*name).to_string(), diff --git a/crates/ruff/src/rules/pyflakes/rules/strings.rs b/crates/ruff/src/rules/pyflakes/rules/strings.rs index 5afec5c441..11edb1bc45 100644 --- a/crates/ruff/src/rules/pyflakes/rules/strings.rs +++ b/crates/ruff/src/rules/pyflakes/rules/strings.rs @@ -386,7 +386,9 @@ pub struct StringDotFormatExtraNamedArguments { missing: Vec, } -impl AlwaysAutofixableViolation for StringDotFormatExtraNamedArguments { +impl Violation for StringDotFormatExtraNamedArguments { + const AUTOFIX: AutofixKind = AutofixKind::Sometimes; + #[derive_message_formats] fn message(&self) -> String { let StringDotFormatExtraNamedArguments { missing } = self; @@ -394,10 +396,10 @@ impl AlwaysAutofixableViolation for StringDotFormatExtraNamedArguments { format!("`.format` call has unused named argument(s): {message}") } - fn autofix_title(&self) -> String { + fn autofix_title(&self) -> Option { let StringDotFormatExtraNamedArguments { missing } = self; let message = missing.join(", "); - format!("Remove extra named arguments: {message}") + Some(format!("Remove extra named arguments: {message}")) } } diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs index 07ed283517..e4edea697d 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_export.rs @@ -51,7 +51,7 @@ impl Violation for UndefinedExport { pub(crate) fn undefined_export(name: &str, range: TextRange, scope: &Scope) -> Vec { let mut diagnostics = Vec::new(); if !scope.uses_star_imports() { - if !scope.defines(name) { + if !scope.has(name) { diagnostics.push(Diagnostic::new( UndefinedExport { name: (*name).to_string(), diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs index 1479fdb945..20c8fdef62 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs @@ -47,7 +47,7 @@ impl Violation for UndefinedLocal { pub(crate) fn undefined_local(checker: &mut Checker, name: &str) { // If the name hasn't already been defined in the current scope... let current = checker.semantic_model().scope(); - if !current.kind.is_any_function() || current.defines(name) { + if !current.kind.is_any_function() || current.has(name) { return; } diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs index afb8500de0..67b201f83b 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs @@ -1,5 +1,5 @@ use itertools::Itertools; -use ruff_text_size::TextRange; +use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::{self, Ranged, Stmt}; use rustpython_parser::{lexer, Mode, Tok}; @@ -61,21 +61,37 @@ impl Violation for UnusedVariable { } } -/// Return the [`TextRange`] of the token after the next match of -/// the predicate, skipping over any bracketed expressions. -fn match_token_after(located: &T, locator: &Locator, f: F) -> TextRange +/// Return the [`TextRange`] of the token before the next match of the predicate +fn match_token_before(location: TextSize, locator: &Locator, f: F) -> Option where F: Fn(Tok) -> bool, - T: Ranged, { - let contents = locator.after(located.start()); + let contents = locator.after(location); + for ((_, range), (tok, _)) in lexer::lex_starts_at(contents, Mode::Module, location) + .flatten() + .tuple_windows() + { + if f(tok) { + return Some(range); + } + } + None +} + +/// Return the [`TextRange`] of the token after the next match of the predicate, skipping over +/// any bracketed expressions. +fn match_token_after(location: TextSize, locator: &Locator, f: F) -> Option +where + F: Fn(Tok) -> bool, +{ + let contents = locator.after(location); // Track the bracket depth. let mut par_count = 0u32; let mut sqb_count = 0u32; let mut brace_count = 0u32; - for ((tok, _), (_, range)) in lexer::lex_starts_at(contents, Mode::Module, located.start()) + for ((tok, _), (_, range)) in lexer::lex_starts_at(contents, Mode::Module, location) .flatten() .tuple_windows() { @@ -91,97 +107,83 @@ where } Tok::Rpar => { par_count = par_count.saturating_sub(1); - // If this is a closing bracket, continue. - if par_count == 0 { - continue; - } } Tok::Rsqb => { sqb_count = sqb_count.saturating_sub(1); - // If this is a closing bracket, continue. - if sqb_count == 0 { - continue; - } } Tok::Rbrace => { brace_count = brace_count.saturating_sub(1); - // If this is a closing bracket, continue. - if brace_count == 0 { - continue; - } } _ => {} } + // If we're in nested brackets, continue. if par_count > 0 || sqb_count > 0 || brace_count > 0 { continue; } if f(tok) { - return range; + return Some(range); } } - unreachable!("No token after matched"); + None } -/// Return the [`TextRange`] of the token matching the predicate, -/// skipping over any bracketed expressions. -fn match_token(located: &T, locator: &Locator, f: F) -> TextRange +/// Return the [`TextRange`] of the token matching the predicate or the first mismatched +/// bracket, skipping over any bracketed expressions. +fn match_token_or_closing_brace(location: TextSize, locator: &Locator, f: F) -> Option where F: Fn(Tok) -> bool, - T: Ranged, { - let contents = locator.after(located.start()); + let contents = locator.after(location); // Track the bracket depth. - let mut par_count = 0; - let mut sqb_count = 0; - let mut brace_count = 0; + let mut par_count = 0u32; + let mut sqb_count = 0u32; + let mut brace_count = 0u32; - for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, located.start()).flatten() { + for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, location).flatten() { match tok { Tok::Lpar => { - par_count += 1; + par_count = par_count.saturating_add(1); } Tok::Lsqb => { - sqb_count += 1; + sqb_count = sqb_count.saturating_add(1); } Tok::Lbrace => { - brace_count += 1; + brace_count = brace_count.saturating_add(1); } Tok::Rpar => { - par_count -= 1; - // If this is a closing bracket, continue. if par_count == 0 { - continue; + return Some(range); } + par_count = par_count.saturating_sub(1); } Tok::Rsqb => { - sqb_count -= 1; - // If this is a closing bracket, continue. if sqb_count == 0 { - continue; + return Some(range); } + sqb_count = sqb_count.saturating_sub(1); } Tok::Rbrace => { - brace_count -= 1; - // If this is a closing bracket, continue. if brace_count == 0 { - continue; + return Some(range); } + brace_count = brace_count.saturating_sub(1); } _ => {} } + // If we're in nested brackets, continue. if par_count > 0 || sqb_count > 0 || brace_count > 0 { continue; } if f(tok) { - return range; + return Some(range); } } - unreachable!("No token after matched"); + None } /// Generate a [`Edit`] to remove an unused variable assignment, given the @@ -201,10 +203,10 @@ fn remove_unused_variable( { // If the expression is complex (`x = foo()`), remove the assignment, // but preserve the right-hand side. - let edit = Edit::deletion( - target.start(), - match_token_after(target, checker.locator, |tok| tok == Tok::Equal).start(), - ); + let start = target.start(); + let end = + match_token_after(start, checker.locator, |tok| tok == Tok::Equal)?.start(); + let edit = Edit::deletion(start, end); Some(Fix::suggested(edit)) } else { // If (e.g.) assigning to a constant (`x = 1`), delete the entire statement. @@ -232,10 +234,10 @@ fn remove_unused_variable( return if contains_effect(value, |id| checker.semantic_model().is_builtin(id)) { // If the expression is complex (`x = foo()`), remove the assignment, // but preserve the right-hand side. - let edit = Edit::deletion( - stmt.start(), - match_token_after(stmt, checker.locator, |tok| tok == Tok::Equal).start(), - ); + let start = stmt.start(); + let end = + match_token_after(start, checker.locator, |tok| tok == Tok::Equal)?.start(); + let edit = Edit::deletion(start, end); Some(Fix::suggested(edit)) } else { // If (e.g.) assigning to a constant (`x = 1`), delete the entire statement. @@ -258,15 +260,20 @@ fn remove_unused_variable( for item in items { if let Some(optional_vars) = &item.optional_vars { if optional_vars.range() == range { - let edit = Edit::deletion( - item.context_expr.end(), - // The end of the `Withitem` is the colon, comma, or closing - // parenthesis following the `optional_vars`. - match_token(&item.context_expr, checker.locator, |tok| { - tok == Tok::Colon || tok == Tok::Comma || tok == Tok::Rpar - }) - .start(), - ); + // Find the first token before the `as` keyword. + let start = + match_token_before(item.context_expr.start(), checker.locator, |tok| { + tok == Tok::As + })? + .end(); + + // Find the first colon, comma, or closing bracket after the `as` keyword. + let end = match_token_or_closing_brace(start, checker.locator, |tok| { + tok == Tok::Colon || tok == Tok::Comma + })? + .start(); + + let edit = Edit::deletion(start, end); return Some(Fix::suggested(edit)); } } diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap index ca5ac6ec6e..b1ae9be974 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F522_F522.py.snap @@ -33,6 +33,7 @@ F522.py:2:1: F522 [*] `.format` call has unused named argument(s): spam 2 |+"{bar}{}".format(1, bar=2, ) # F522 3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues 4 4 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522 +5 5 | # Not fixable F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham | @@ -40,6 +41,8 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham 5 | "{bar:{spam}}".format(bar=2, spam=3) # No issues 6 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F522 +7 | # Not fixable +8 | ('' | = help: Remove extra named arguments: eggs, ham @@ -49,5 +52,19 @@ F522.py:4:1: F522 [*] `.format` call has unused named argument(s): eggs, ham 3 3 | "{bar:{spam}}".format(bar=2, spam=3) # No issues 4 |-"{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522 4 |+"{bar:{spam}}".format(bar=2, spam=3, ) # F522 +5 5 | # Not fixable +6 6 | ('' +7 7 | .format(x=2)) + +F522.py:6:2: F522 `.format` call has unused named argument(s): x + | +6 | "{bar:{spam}}".format(bar=2, spam=3, eggs=4, ham=5) # F522 +7 | # Not fixable +8 | ('' + | __^ +9 | | .format(x=2)) + | |_____________^ F522 + | + = help: Remove extra named arguments: x diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap index fe73c600ea..327fa2a561 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap @@ -377,126 +377,145 @@ F841_3.py:77:25: F841 [*] Local variable `cm` is assigned to but never used 79 79 | 80 80 | -F841_3.py:87:5: F841 [*] Local variable `toplevel` is assigned to but never used +F841_3.py:87:26: F841 [*] Local variable `cm` is assigned to but never used | 87 | def f(): -88 | toplevel = tt = lexer.get_token() - | ^^^^^^^^ F841 -89 | if not tt: -90 | break +88 | with (Nested(m)) as (cm): + | ^^ F841 +89 | pass | - = help: Remove assignment to unused variable `toplevel` + = help: Remove assignment to unused variable `cm` ℹ Suggested fix 84 84 | 85 85 | 86 86 | def f(): -87 |- toplevel = tt = lexer.get_token() - 87 |+ tt = lexer.get_token() -88 88 | if not tt: -89 89 | break +87 |- with (Nested(m)) as (cm): + 87 |+ with (Nested(m)): +88 88 | pass +89 89 | 90 90 | -F841_3.py:93:5: F841 [*] Local variable `toplevel` is assigned to but never used +F841_3.py:92:5: F841 [*] Local variable `toplevel` is assigned to but never used | -93 | def f(): -94 | toplevel = tt = lexer.get_token() +92 | def f(): +93 | toplevel = tt = lexer.get_token() + | ^^^^^^^^ F841 +94 | if not tt: +95 | break + | + = help: Remove assignment to unused variable `toplevel` + +ℹ Suggested fix +89 89 | +90 90 | +91 91 | def f(): +92 |- toplevel = tt = lexer.get_token() + 92 |+ tt = lexer.get_token() +93 93 | if not tt: +94 94 | break +95 95 | + +F841_3.py:98:5: F841 [*] Local variable `toplevel` is assigned to but never used + | +98 | def f(): +99 | toplevel = tt = lexer.get_token() | ^^^^^^^^ F841 | = help: Remove assignment to unused variable `toplevel` ℹ Suggested fix -90 90 | -91 91 | -92 92 | def f(): -93 |- toplevel = tt = lexer.get_token() - 93 |+ tt = lexer.get_token() -94 94 | 95 95 | -96 96 | def f(): +96 96 | +97 97 | def f(): +98 |- toplevel = tt = lexer.get_token() + 98 |+ tt = lexer.get_token() +99 99 | +100 100 | +101 101 | def f(): -F841_3.py:93:16: F841 [*] Local variable `tt` is assigned to but never used +F841_3.py:98:16: F841 [*] Local variable `tt` is assigned to but never used | -93 | def f(): -94 | toplevel = tt = lexer.get_token() +98 | def f(): +99 | toplevel = tt = lexer.get_token() | ^^ F841 | = help: Remove assignment to unused variable `tt` ℹ Suggested fix -90 90 | -91 91 | -92 92 | def f(): -93 |- toplevel = tt = lexer.get_token() - 93 |+ toplevel = lexer.get_token() -94 94 | 95 95 | -96 96 | def f(): - -F841_3.py:97:5: F841 [*] Local variable `toplevel` is assigned to but never used - | -97 | def f(): -98 | toplevel = (a, b) = lexer.get_token() - | ^^^^^^^^ F841 - | - = help: Remove assignment to unused variable `toplevel` - -ℹ Suggested fix -94 94 | -95 95 | -96 96 | def f(): -97 |- toplevel = (a, b) = lexer.get_token() - 97 |+ (a, b) = lexer.get_token() -98 98 | +96 96 | +97 97 | def f(): +98 |- toplevel = tt = lexer.get_token() + 98 |+ toplevel = lexer.get_token() 99 99 | -100 100 | def f(): +100 100 | +101 101 | def f(): -F841_3.py:101:14: F841 [*] Local variable `toplevel` is assigned to but never used +F841_3.py:102:5: F841 [*] Local variable `toplevel` is assigned to but never used | -101 | def f(): -102 | (a, b) = toplevel = lexer.get_token() - | ^^^^^^^^ F841 - | - = help: Remove assignment to unused variable `toplevel` - -ℹ Suggested fix -98 98 | -99 99 | -100 100 | def f(): -101 |- (a, b) = toplevel = lexer.get_token() - 101 |+ (a, b) = lexer.get_token() -102 102 | -103 103 | -104 104 | def f(): - -F841_3.py:105:5: F841 [*] Local variable `toplevel` is assigned to but never used - | -105 | def f(): -106 | toplevel = tt = 1 +102 | def f(): +103 | toplevel = (a, b) = lexer.get_token() | ^^^^^^^^ F841 | = help: Remove assignment to unused variable `toplevel` ℹ Suggested fix -102 102 | +99 99 | +100 100 | +101 101 | def f(): +102 |- toplevel = (a, b) = lexer.get_token() + 102 |+ (a, b) = lexer.get_token() 103 103 | -104 104 | def f(): -105 |- toplevel = tt = 1 - 105 |+ tt = 1 +104 104 | +105 105 | def f(): -F841_3.py:105:16: F841 [*] Local variable `tt` is assigned to but never used +F841_3.py:106:14: F841 [*] Local variable `toplevel` is assigned to but never used | -105 | def f(): -106 | toplevel = tt = 1 +106 | def f(): +107 | (a, b) = toplevel = lexer.get_token() + | ^^^^^^^^ F841 + | + = help: Remove assignment to unused variable `toplevel` + +ℹ Suggested fix +103 103 | +104 104 | +105 105 | def f(): +106 |- (a, b) = toplevel = lexer.get_token() + 106 |+ (a, b) = lexer.get_token() +107 107 | +108 108 | +109 109 | def f(): + +F841_3.py:110:5: F841 [*] Local variable `toplevel` is assigned to but never used + | +110 | def f(): +111 | toplevel = tt = 1 + | ^^^^^^^^ F841 + | + = help: Remove assignment to unused variable `toplevel` + +ℹ Suggested fix +107 107 | +108 108 | +109 109 | def f(): +110 |- toplevel = tt = 1 + 110 |+ tt = 1 + +F841_3.py:110:16: F841 [*] Local variable `tt` is assigned to but never used + | +110 | def f(): +111 | toplevel = tt = 1 | ^^ F841 | = help: Remove assignment to unused variable `tt` ℹ Suggested fix -102 102 | -103 103 | -104 104 | def f(): -105 |- toplevel = tt = 1 - 105 |+ toplevel = 1 +107 107 | +108 108 | +109 109 | def f(): +110 |- toplevel = tt = 1 + 110 |+ toplevel = 1 diff --git a/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs b/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs index 9d76516a68..b98806c7c8 100644 --- a/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs +++ b/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs @@ -1,4 +1,4 @@ -use rustpython_parser::ast::{Expr, ExprName, Ranged}; +use rustpython_parser::ast::{Expr, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -6,7 +6,7 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for iterations over `set` literals and comprehensions. +/// Checks for iterations over `set` literals. /// /// ## Why is this bad? /// Iterating over a `set` is less efficient than iterating over a sequence @@ -38,23 +38,7 @@ impl Violation for IterationOverSet { /// PLC0208 pub(crate) fn iteration_over_set(checker: &mut Checker, expr: &Expr) { - let is_set = match expr { - // Ex) `for i in {1, 2, 3}` - Expr::Set(_) => true, - // Ex)` for i in {n for n in range(1, 4)}` - Expr::SetComp(_) => true, - // Ex) `for i in set(1, 2, 3)` - Expr::Call(call) => { - if let Expr::Name(ExprName { id, .. }) = call.func.as_ref() { - id.as_str() == "set" && checker.semantic_model().is_builtin("set") - } else { - false - } - } - _ => false, - }; - - if is_set { + if expr.is_set_expr() { checker .diagnostics .push(Diagnostic::new(IterationOverSet, expr.range())); diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap index 5fa6435a0d..88c00a735f 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap @@ -10,62 +10,44 @@ iteration_over_set.py:3:13: PLC0208 Use a sequence type instead of a `set` when 6 | print(f"I like {item}.") | -iteration_over_set.py:6:13: PLC0208 Use a sequence type instead of a `set` when iterating over values - | -6 | print(f"I like {item}.") -7 | -8 | for item in set(("apples", "lemons", "water")): # flags set() calls - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208 -9 | print(f"I like {item}.") - | - -iteration_over_set.py:9:15: PLC0208 Use a sequence type instead of a `set` when iterating over values +iteration_over_set.py:6:28: PLC0208 Use a sequence type instead of a `set` when iterating over values | - 9 | print(f"I like {item}.") -10 | -11 | for number in {i for i in range(10)}: # flags set comprehensions - | ^^^^^^^^^^^^^^^^^^^^^^ PLC0208 -12 | print(number) - | - -iteration_over_set.py:12:28: PLC0208 Use a sequence type instead of a `set` when iterating over values - | -12 | print(number) -13 | -14 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions + 6 | print(f"I like {item}.") + 7 | + 8 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions | ^^^^^^^^^ PLC0208 -15 | -16 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions + 9 | +10 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions | -iteration_over_set.py:14:27: PLC0208 Use a sequence type instead of a `set` when iterating over values +iteration_over_set.py:8:27: PLC0208 Use a sequence type instead of a `set` when iterating over values | -14 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions -15 | -16 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions + 8 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions + 9 | +10 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions | ^^^^^^^^^ PLC0208 -17 | -18 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions +11 | +12 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions | -iteration_over_set.py:16:36: PLC0208 Use a sequence type instead of a `set` when iterating over values +iteration_over_set.py:10:36: PLC0208 Use a sequence type instead of a `set` when iterating over values | -16 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions -17 | -18 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions +10 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions +11 | +12 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions | ^^^^^^^^^ PLC0208 -19 | -20 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions +13 | +14 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions | -iteration_over_set.py:18:27: PLC0208 Use a sequence type instead of a `set` when iterating over values +iteration_over_set.py:12:27: PLC0208 Use a sequence type instead of a `set` when iterating over values | -18 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions -19 | -20 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions +12 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions +13 | +14 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions | ^^^^^^^^^ PLC0208 -21 | -22 | # Non-errors +15 | +16 | # Non-errors | diff --git a/crates/ruff_cli/Cargo.toml b/crates/ruff_cli/Cargo.toml index 6f0cb1ef9d..c175b48ab0 100644 --- a/crates/ruff_cli/Cargo.toml +++ b/crates/ruff_cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ruff_cli" -version = "0.0.270" +version = "0.0.271" authors = ["Charlie Marsh "] edition = { workspace = true } rust-version = { workspace = true } diff --git a/crates/ruff_python_ast/src/function.rs b/crates/ruff_python_ast/src/function.rs new file mode 100644 index 0000000000..1f7f114f69 --- /dev/null +++ b/crates/ruff_python_ast/src/function.rs @@ -0,0 +1,114 @@ +use crate::node::AnyNodeRef; +use ruff_text_size::TextRange; +use rustpython_ast::{ + Arguments, Expr, Identifier, Ranged, StmtAsyncFunctionDef, StmtFunctionDef, Suite, +}; + +/// Enum that represents any python function definition. +#[derive(Copy, Clone, PartialEq, Debug)] +pub enum AnyFunctionDefinition<'a> { + FunctionDefinition(&'a StmtFunctionDef), + AsyncFunctionDefinition(&'a StmtAsyncFunctionDef), +} + +impl<'a> AnyFunctionDefinition<'a> { + pub const fn cast_ref(reference: AnyNodeRef<'a>) -> Option { + match reference { + AnyNodeRef::StmtAsyncFunctionDef(definition) => { + Some(Self::AsyncFunctionDefinition(definition)) + } + AnyNodeRef::StmtFunctionDef(definition) => Some(Self::FunctionDefinition(definition)), + _ => None, + } + } + + /// Returns `Some` if this is a [`StmtFunctionDef`] and `None` otherwise. + pub const fn as_function_definition(self) -> Option<&'a StmtFunctionDef> { + if let Self::FunctionDefinition(definition) = self { + Some(definition) + } else { + None + } + } + + /// Returns `Some` if this is a [`StmtAsyncFunctionDef`] and `None` otherwise. + pub const fn as_async_function_definition(self) -> Option<&'a StmtAsyncFunctionDef> { + if let Self::AsyncFunctionDefinition(definition) = self { + Some(definition) + } else { + None + } + } + + /// Returns the function's name + pub const fn name(self) -> &'a Identifier { + match self { + Self::FunctionDefinition(definition) => &definition.name, + Self::AsyncFunctionDefinition(definition) => &definition.name, + } + } + + /// Returns the function arguments (parameters). + pub fn arguments(self) -> &'a Arguments { + match self { + Self::FunctionDefinition(definition) => definition.args.as_ref(), + Self::AsyncFunctionDefinition(definition) => definition.args.as_ref(), + } + } + + /// Returns the function's body + pub const fn body(self) -> &'a Suite { + match self { + Self::FunctionDefinition(definition) => &definition.body, + Self::AsyncFunctionDefinition(definition) => &definition.body, + } + } + + /// Returns the decorators attributing the function. + pub fn decorators(self) -> &'a [Expr] { + match self { + Self::FunctionDefinition(definition) => &definition.decorator_list, + Self::AsyncFunctionDefinition(definition) => &definition.decorator_list, + } + } + + pub fn returns(self) -> Option<&'a Expr> { + match self { + Self::FunctionDefinition(definition) => definition.returns.as_deref(), + Self::AsyncFunctionDefinition(definition) => definition.returns.as_deref(), + } + } + + pub fn type_comments(self) -> Option<&'a str> { + match self { + Self::FunctionDefinition(definition) => definition.type_comment.as_deref(), + Self::AsyncFunctionDefinition(definition) => definition.type_comment.as_deref(), + } + } + + /// Returns `true` if this is [`Self::AsyncFunctionDefinition`] + pub const fn is_async(self) -> bool { + matches!(self, Self::AsyncFunctionDefinition(_)) + } +} + +impl Ranged for AnyFunctionDefinition<'_> { + fn range(&self) -> TextRange { + match self { + AnyFunctionDefinition::FunctionDefinition(definition) => definition.range(), + AnyFunctionDefinition::AsyncFunctionDefinition(definition) => definition.range(), + } + } +} + +impl<'a> From<&'a StmtFunctionDef> for AnyFunctionDefinition<'a> { + fn from(value: &'a StmtFunctionDef) -> Self { + Self::FunctionDefinition(value) + } +} + +impl<'a> From<&'a StmtAsyncFunctionDef> for AnyFunctionDefinition<'a> { + fn from(value: &'a StmtAsyncFunctionDef) -> Self { + Self::AsyncFunctionDefinition(value) + } +} diff --git a/crates/ruff_python_ast/src/lib.rs b/crates/ruff_python_ast/src/lib.rs index 77bb89fa1a..552c7f02bd 100644 --- a/crates/ruff_python_ast/src/lib.rs +++ b/crates/ruff_python_ast/src/lib.rs @@ -2,6 +2,7 @@ pub mod all; pub mod call_path; pub mod cast; pub mod comparable; +pub mod function; pub mod hashable; pub mod helpers; pub mod imports; diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_expression.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_expression.py new file mode 100644 index 0000000000..90042fa065 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary_expression.py @@ -0,0 +1,43 @@ +(aaaaaaaa + + # trailing operator comment + b # trailing right comment +) + + +(aaaaaaaa # trailing left comment + + # trailing operator comment + # leading right comment + b +) + + +# Black breaks the right side first for the following expressions: +aaaaaaaaaaaaaa + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(argument1, argument2, argument3) +aaaaaaaaaaaaaa + [bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee] +aaaaaaaaaaaaaa + (bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee) +aaaaaaaaaaaaaa + { key1:bbbbbbbbbbbbbbbbbbbbbb, key2: ccccccccccccccccccccc, key3: dddddddddddddddd, key4: eeeeeee } +aaaaaaaaaaaaaa + { bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee } +aaaaaaaaaaaaaa + [a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ] +aaaaaaaaaaaaaa + (a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ) +aaaaaaaaaaaaaa + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb} + +# Wraps it in parentheses if it needs to break both left and right +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + [ + bbbbbbbbbbbbbbbbbbbbbb, + ccccccccccccccccccccc, + dddddddddddddddd, + eee +] # comment + + + +# But only for expressions that have a statement parent. +not (aaaaaaaaaaaaaa + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb}) +[a + [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] in c ] + + +# leading comment +( + # comment + content + b +) diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index 45a9ee248e..48d79e4411 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -12,7 +12,7 @@ pub(crate) trait PyFormatterExtensions<'ast, 'buf> { /// /// * [`NodeLevel::Module`]: Up to two empty lines /// * [`NodeLevel::CompoundStatement`]: Up to one empty line - /// * [`NodeLevel::Parenthesized`]: No empty lines + /// * [`NodeLevel::Expression`]: No empty lines fn join_nodes<'fmt>(&'fmt mut self, level: NodeLevel) -> JoinNodesBuilder<'fmt, 'ast, 'buf>; } @@ -48,18 +48,18 @@ impl<'fmt, 'ast, 'buf> JoinNodesBuilder<'fmt, 'ast, 'buf> { { let node_level = self.node_level; let separator = format_with(|f: &mut PyFormatter| match node_level { - NodeLevel::TopLevel => match lines_before(f.context().contents(), node.start()) { + NodeLevel::TopLevel => match lines_before(node.start(), f.context().contents()) { 0 | 1 => hard_line_break().fmt(f), 2 => empty_line().fmt(f), _ => write!(f, [empty_line(), empty_line()]), }, NodeLevel::CompoundStatement => { - match lines_before(f.context().contents(), node.start()) { + match lines_before(node.start(), f.context().contents()) { 0 | 1 => hard_line_break().fmt(f), _ => empty_line().fmt(f), } } - NodeLevel::Parenthesized => hard_line_break().fmt(f), + NodeLevel::Expression => hard_line_break().fmt(f), }); self.entry_with_separator(&separator, content); @@ -200,7 +200,7 @@ no_leading_newline = 30"# // Removes all empty lines #[test] fn ranged_builder_parenthesized_level() { - let printed = format_ranged(NodeLevel::Parenthesized); + let printed = format_ranged(NodeLevel::Expression); assert_eq!( &printed, diff --git a/crates/ruff_python_formatter/src/comments/format.rs b/crates/ruff_python_formatter/src/comments/format.rs index 286905ce85..44e77de3d4 100644 --- a/crates/ruff_python_formatter/src/comments/format.rs +++ b/crates/ruff_python_formatter/src/comments/format.rs @@ -39,7 +39,7 @@ impl Format> for FormatLeadingComments<'_> { for comment in leading_comments { let slice = comment.slice(); - let lines_after_comment = lines_after(f.context().contents(), slice.end()); + let lines_after_comment = lines_after(slice.end(), f.context().contents()); write!( f, [format_comment(comment), empty_lines(lines_after_comment)] @@ -80,7 +80,7 @@ impl Format> for FormatLeadingAlternateBranchComments<'_> { if let Some(first_leading) = self.comments.first() { // Leading comments only preserves the lines after the comment but not before. // Insert the necessary lines. - if lines_before(f.context().contents(), first_leading.slice().start()) > 1 { + if lines_before(first_leading.slice().start(), f.context().contents()) > 1 { write!(f, [empty_line()])?; } @@ -88,7 +88,7 @@ impl Format> for FormatLeadingAlternateBranchComments<'_> { } else if let Some(last_preceding) = self.last_node { // The leading comments formatting ensures that it preserves the right amount of lines after // We need to take care of this ourselves, if there's no leading `else` comment. - if lines_after(f.context().contents(), last_preceding.end()) > 1 { + if lines_after(last_preceding.end(), f.context().contents()) > 1 { write!(f, [empty_line()])?; } } @@ -132,7 +132,7 @@ impl Format> for FormatTrailingComments<'_> { has_trailing_own_line_comment |= trailing.position().is_own_line(); if has_trailing_own_line_comment { - let lines_before_comment = lines_before(f.context().contents(), slice.start()); + let lines_before_comment = lines_before(slice.start(), f.context().contents()); // A trailing comment at the end of a body or list // ```python @@ -175,20 +175,26 @@ pub(crate) fn dangling_node_comments(node: &T) -> FormatDanglingComments where T: AstNode, { - FormatDanglingComments { - node: node.as_any_node_ref(), - } + FormatDanglingComments::Node(node.as_any_node_ref()) } -pub(crate) struct FormatDanglingComments<'a> { - node: AnyNodeRef<'a>, +pub(crate) fn dangling_comments(comments: &[SourceComment]) -> FormatDanglingComments { + FormatDanglingComments::Comments(comments) +} + +pub(crate) enum FormatDanglingComments<'a> { + Node(AnyNodeRef<'a>), + Comments(&'a [SourceComment]), } impl Format> for FormatDanglingComments<'_> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { let comments = f.context().comments().clone(); - let dangling_comments = comments.dangling_comments(self.node); + let dangling_comments = match self { + Self::Comments(comments) => comments, + Self::Node(node) => comments.dangling_comments(*node), + }; let mut first = true; for comment in dangling_comments { @@ -200,7 +206,7 @@ impl Format> for FormatDanglingComments<'_> { f, [ format_comment(comment), - empty_lines(lines_after(f.context().contents(), comment.slice().end())) + empty_lines(lines_after(comment.slice().end(), f.context().contents())) ] )?; @@ -301,7 +307,7 @@ impl Format> for FormatEmptyLines { }, // Remove all whitespace in parenthesized expressions - NodeLevel::Parenthesized => write!(f, [hard_line_break()]), + NodeLevel::Expression => write!(f, [hard_line_break()]), } } } diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index ae61709e01..6822efb461 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -103,8 +103,8 @@ use crate::comments::map::MultiMap; use crate::comments::node_key::NodeRefEqualityKey; use crate::comments::visitor::CommentsVisitor; pub(crate) use format::{ - dangling_node_comments, leading_alternate_branch_comments, leading_node_comments, - trailing_comments, trailing_node_comments, + dangling_comments, dangling_node_comments, leading_alternate_branch_comments, + leading_node_comments, trailing_comments, trailing_node_comments, }; use ruff_formatter::{SourceCode, SourceCodeSlice}; use ruff_python_ast::node::AnyNodeRef; diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 062cd17a8d..c90d446b25 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -520,8 +520,8 @@ fn handle_trailing_end_of_line_condition_comment<'a>( if preceding.ptr_eq(last_before_colon) { let mut start = preceding.end(); while let Some((offset, c)) = find_first_non_trivia_character_in_range( - locator.contents(), TextRange::new(start, following.start()), + locator.contents(), ) { match c { ':' => { @@ -655,7 +655,7 @@ fn handle_trailing_binary_expression_left_or_operator_comment<'a>( ); let operator_offset = loop { - match find_first_non_trivia_character_in_range(locator.contents(), between_operands_range) { + match find_first_non_trivia_character_in_range(between_operands_range, locator.contents()) { // Skip over closing parens Some((offset, ')')) => { between_operands_range = @@ -733,17 +733,17 @@ fn find_pos_only_slash_offset( locator: &Locator, ) -> Option { // First find the comma separating the two arguments - find_first_non_trivia_character_in_range(locator.contents(), between_arguments_range).and_then( + find_first_non_trivia_character_in_range(between_arguments_range, locator.contents()).and_then( |(comma_offset, comma)| { debug_assert_eq!(comma, ','); // Then find the position of the `/` operator find_first_non_trivia_character_in_range( - locator.contents(), TextRange::new( comma_offset + TextSize::new(1), between_arguments_range.end(), ), + locator.contents(), ) .map(|(offset, c)| { debug_assert_eq!(c, '/'); diff --git a/crates/ruff_python_formatter/src/context.rs b/crates/ruff_python_formatter/src/context.rs index bef99c4514..074d5cc08d 100644 --- a/crates/ruff_python_formatter/src/context.rs +++ b/crates/ruff_python_formatter/src/context.rs @@ -82,5 +82,5 @@ pub(crate) enum NodeLevel { CompoundStatement, /// Formatting nodes that are enclosed in a parenthesized expression. - Parenthesized, + Expression, } diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index 3c566aeae9..71aa2e83f8 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprAttribute; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprAttribute { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprAttribute { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_await.rs b/crates/ruff_python_formatter/src/expression/expr_await.rs index 4b4d74d22f..60651ce674 100644 --- a/crates/ruff_python_formatter/src/expression/expr_await.rs +++ b/crates/ruff_python_formatter/src/expression/expr_await.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprAwait; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprAwait { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprAwait { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs index 102e55b2a7..76cf01ad4b 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs @@ -1,12 +1,215 @@ -use crate::{verbatim_text, FormatNodeRule, PyFormatter}; -use ruff_formatter::{write, Buffer, FormatResult}; -use rustpython_parser::ast::ExprBinOp; +use crate::comments::trailing_comments; +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parenthesize, +}; +use crate::expression::Parentheses; +use crate::prelude::*; +use crate::FormatNodeRule; +use ruff_formatter::{ + format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions, +}; +use ruff_python_ast::node::AstNode; +use rustpython_parser::ast::{ + Constant, Expr, ExprAttribute, ExprBinOp, ExprConstant, ExprUnaryOp, Operator, Unaryop, +}; #[derive(Default)] -pub struct FormatExprBinOp; +pub struct FormatExprBinOp { + parentheses: Option, +} + +impl FormatRuleWithOptions> for FormatExprBinOp { + type Options = Option; + + fn with_options(mut self, options: Self::Options) -> Self { + self.parentheses = options; + self + } +} impl FormatNodeRule for FormatExprBinOp { fn fmt_fields(&self, item: &ExprBinOp, f: &mut PyFormatter) -> FormatResult<()> { - write!(f, [verbatim_text(item.range)]) + let ExprBinOp { + left, + right, + op, + range: _, + } = item; + + let should_break_right = self.parentheses == Some(Parentheses::Custom); + + if should_break_right { + let left = left.format().memoized(); + let right = right.format().memoized(); + + write!( + f, + [best_fitting![ + // The whole expression on a single line + format_args![left, space(), op.format(), space(), right], + // Break the right, but keep the left flat + format_args![ + left, + space(), + op.format(), + space(), + group(&right).should_expand(true), + ], + // Break after the operator, try to keep the right flat, otherwise expand it + format_args![ + text("("), + block_indent(&format_args![ + left, + hard_line_break(), + op.format(), + space(), + group(&right), + ]), + text(")") + ], + ]] + ) + } else { + let comments = f.context().comments().clone(); + let operator_comments = comments.dangling_comments(item.as_any_node_ref()); + let needs_space = !is_simple_power_expression(item); + + let before_operator_space = if needs_space { + soft_line_break_or_space() + } else { + soft_line_break() + }; + + write!( + f, + [ + left.format(), + before_operator_space, + op.format(), + trailing_comments(operator_comments), + ] + )?; + + // Format the operator on its own line if the operator has trailing comments and the right side has leading comments. + if !operator_comments.is_empty() && comments.has_leading_comments(right.as_ref().into()) + { + write!(f, [hard_line_break()])?; + } else if needs_space { + write!(f, [space()])?; + } + + write!(f, [group(&right.format())]) + } + } + + fn fmt_dangling_comments(&self, _node: &ExprBinOp, _f: &mut PyFormatter) -> FormatResult<()> { + // Handled inside of `fmt_fields` + Ok(()) + } +} + +const fn is_simple_power_expression(expr: &ExprBinOp) -> bool { + expr.op.is_pow() && is_simple_power_operand(&expr.left) && is_simple_power_operand(&expr.right) +} + +/// Return `true` if an [`Expr`] adheres to [Black's definition](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-breaks-binary-operators) +/// of a non-complex expression, in the context of a power operation. +const fn is_simple_power_operand(expr: &Expr) -> bool { + match expr { + Expr::UnaryOp(ExprUnaryOp { + op: Unaryop::Not, .. + }) => false, + Expr::Constant(ExprConstant { + value: Constant::Complex { .. } | Constant::Float(_) | Constant::Int(_), + .. + }) => true, + Expr::Name(_) => true, + Expr::UnaryOp(ExprUnaryOp { operand, .. }) => is_simple_power_operand(operand), + Expr::Attribute(ExprAttribute { value, .. }) => is_simple_power_operand(value), + _ => false, + } +} + +#[derive(Copy, Clone)] +pub struct FormatOperator; + +impl<'ast> AsFormat> for Operator { + type Format<'a> = FormatRefWithRule<'a, Operator, FormatOperator, PyFormatContext<'ast>>; + + fn format(&self) -> Self::Format<'_> { + FormatRefWithRule::new(self, FormatOperator) + } +} + +impl<'ast> IntoFormat> for Operator { + type Format = FormatOwnedWithRule>; + fn into_format(self) -> Self::Format { + FormatOwnedWithRule::new(self, FormatOperator) + } +} + +impl FormatRule> for FormatOperator { + fn fmt(&self, item: &Operator, f: &mut Formatter>) -> FormatResult<()> { + let operator = match item { + Operator::Add => "+", + Operator::Sub => "-", + Operator::Mult => "*", + Operator::MatMult => "@", + Operator::Div => "/", + Operator::Mod => "%", + Operator::Pow => "**", + Operator::LShift => "<<", + Operator::RShift => ">>", + Operator::BitOr => "|", + Operator::BitXor => "^", + Operator::BitAnd => "&", + Operator::FloorDiv => "//", + }; + + text(operator).fmt(f) + } +} + +impl NeedsParentheses for ExprBinOp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => { + if should_binary_break_right_side_first(self) { + Parentheses::Custom + } else { + Parentheses::Optional + } + } + parentheses => parentheses, + } + } +} + +pub(super) fn should_binary_break_right_side_first(expr: &ExprBinOp) -> bool { + use ruff_python_ast::prelude::*; + + if expr.left.is_bin_op_expr() { + false + } else { + match expr.right.as_ref() { + Expr::Tuple(ExprTuple { + elts: expressions, .. + }) + | Expr::List(ExprList { + elts: expressions, .. + }) + | Expr::Set(ExprSet { + elts: expressions, .. + }) + | Expr::Dict(ExprDict { + values: expressions, + .. + }) => !expressions.is_empty(), + Expr::Call(ExprCall { args, keywords, .. }) => !args.is_empty() && !keywords.is_empty(), + Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::GeneratorExp(_) => { + true + } + _ => false, + } } } diff --git a/crates/ruff_python_formatter/src/expression/expr_bool_op.rs b/crates/ruff_python_formatter/src/expression/expr_bool_op.rs index 26dc802bfb..a3ae0b7cf0 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bool_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bool_op.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprBoolOp; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprBoolOp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprBoolOp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index 92a9467c2d..5a2076a7d4 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprCall; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprCall { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprCall { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_compare.rs b/crates/ruff_python_formatter/src/expression/expr_compare.rs index b3abbe305c..7915151cf5 100644 --- a/crates/ruff_python_formatter/src/expression/expr_compare.rs +++ b/crates/ruff_python_formatter/src/expression/expr_compare.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprCompare; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprCompare { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprCompare { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_constant.rs b/crates/ruff_python_formatter/src/expression/expr_constant.rs index c085fb409e..9a69fec5c7 100644 --- a/crates/ruff_python_formatter/src/expression/expr_constant.rs +++ b/crates/ruff_python_formatter/src/expression/expr_constant.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprConstant; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprConstant { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprConstant { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_dict.rs b/crates/ruff_python_formatter/src/expression/expr_dict.rs index 41c1ce3db7..c9926b822d 100644 --- a/crates/ruff_python_formatter/src/expression/expr_dict.rs +++ b/crates/ruff_python_formatter/src/expression/expr_dict.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprDict; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprDict { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprDict { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_dict_comp.rs b/crates/ruff_python_formatter/src/expression/expr_dict_comp.rs index 9b3a7b1b78..6052a2ecd5 100644 --- a/crates/ruff_python_formatter/src/expression/expr_dict_comp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_dict_comp.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprDictComp; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprDictComp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprDictComp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs b/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs index 774e529a16..f66302b9e6 100644 --- a/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs +++ b/crates/ruff_python_formatter/src/expression/expr_formatted_value.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprFormattedValue; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprFormattedValue { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprFormattedValue { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs b/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs index 488279489d..7ee276e5ad 100644 --- a/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprGeneratorExp; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprGeneratorExp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprGeneratorExp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_if_exp.rs b/crates/ruff_python_formatter/src/expression/expr_if_exp.rs index dd46ab26a2..b13162413c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_if_exp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_if_exp.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprIfExp; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprIfExp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprIfExp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_joined_str.rs b/crates/ruff_python_formatter/src/expression/expr_joined_str.rs index abf4664b74..235f712bba 100644 --- a/crates/ruff_python_formatter/src/expression/expr_joined_str.rs +++ b/crates/ruff_python_formatter/src/expression/expr_joined_str.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprJoinedStr; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprJoinedStr { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprJoinedStr { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_lambda.rs b/crates/ruff_python_formatter/src/expression/expr_lambda.rs index 08fd02ddb1..a9b081c91e 100644 --- a/crates/ruff_python_formatter/src/expression/expr_lambda.rs +++ b/crates/ruff_python_formatter/src/expression/expr_lambda.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprLambda; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprLambda { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprLambda { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_list.rs b/crates/ruff_python_formatter/src/expression/expr_list.rs index 105eed3916..869542621e 100644 --- a/crates/ruff_python_formatter/src/expression/expr_list.rs +++ b/crates/ruff_python_formatter/src/expression/expr_list.rs @@ -1,5 +1,10 @@ -use crate::{verbatim_text, FormatNodeRule, PyFormatter}; -use ruff_formatter::{write, Buffer, FormatResult}; +use crate::comments::dangling_comments; +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; +use crate::prelude::*; +use crate::FormatNodeRule; +use ruff_formatter::{format_args, write}; use rustpython_parser::ast::ExprList; #[derive(Default)] @@ -7,6 +12,55 @@ pub struct FormatExprList; impl FormatNodeRule for FormatExprList { fn fmt_fields(&self, item: &ExprList, f: &mut PyFormatter) -> FormatResult<()> { - write!(f, [verbatim_text(item.range)]) + let ExprList { + range: _, + elts, + ctx: _, + } = item; + + let items = format_with(|f| { + let mut iter = elts.iter(); + + if let Some(first) = iter.next() { + write!(f, [first.format()])?; + } + + for item in iter { + write!(f, [text(","), soft_line_break_or_space(), item.format()])?; + } + + if !elts.is_empty() { + write!(f, [if_group_breaks(&text(","))])?; + } + + Ok(()) + }); + + let comments = f.context().comments().clone(); + let dangling = comments.dangling_comments(item.into()); + + write!( + f, + [group(&format_args![ + text("["), + dangling_comments(dangling), + soft_block_indent(&items), + text("]") + ])] + ) + } + + fn fmt_dangling_comments(&self, _node: &ExprList, _f: &mut PyFormatter) -> FormatResult<()> { + // Handled as part of `fmt_fields` + Ok(()) + } +} + +impl NeedsParentheses for ExprList { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } } } diff --git a/crates/ruff_python_formatter/src/expression/expr_list_comp.rs b/crates/ruff_python_formatter/src/expression/expr_list_comp.rs index 2dcd66dc76..e95327faed 100644 --- a/crates/ruff_python_formatter/src/expression/expr_list_comp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_list_comp.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprListComp; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprListComp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprListComp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_name.rs b/crates/ruff_python_formatter/src/expression/expr_name.rs index 18605603e5..57ad099c38 100644 --- a/crates/ruff_python_formatter/src/expression/expr_name.rs +++ b/crates/ruff_python_formatter/src/expression/expr_name.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::prelude::*; use crate::FormatNodeRule; use ruff_formatter::{write, FormatContext}; @@ -22,6 +25,12 @@ impl FormatNodeRule for FormatExprName { } } +impl NeedsParentheses for ExprName { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} + #[cfg(test)] mod tests { use ruff_text_size::{TextRange, TextSize}; diff --git a/crates/ruff_python_formatter/src/expression/expr_named_expr.rs b/crates/ruff_python_formatter/src/expression/expr_named_expr.rs index 508d0f1c46..ca7182604c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_named_expr.rs +++ b/crates/ruff_python_formatter/src/expression/expr_named_expr.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprNamedExpr; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprNamedExpr { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprNamedExpr { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_set.rs b/crates/ruff_python_formatter/src/expression/expr_set.rs index 8f1981defb..283ea6370e 100644 --- a/crates/ruff_python_formatter/src/expression/expr_set.rs +++ b/crates/ruff_python_formatter/src/expression/expr_set.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprSet; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprSet { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprSet { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_set_comp.rs b/crates/ruff_python_formatter/src/expression/expr_set_comp.rs index 918d3526b1..a74829aee4 100644 --- a/crates/ruff_python_formatter/src/expression/expr_set_comp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_set_comp.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprSetComp; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprSetComp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprSetComp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_slice.rs b/crates/ruff_python_formatter/src/expression/expr_slice.rs index 16b70bfba1..8b019564d6 100644 --- a/crates/ruff_python_formatter/src/expression/expr_slice.rs +++ b/crates/ruff_python_formatter/src/expression/expr_slice.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprSlice; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprSlice { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprSlice { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_starred.rs b/crates/ruff_python_formatter/src/expression/expr_starred.rs index 8ef360db8a..14e40a26d8 100644 --- a/crates/ruff_python_formatter/src/expression/expr_starred.rs +++ b/crates/ruff_python_formatter/src/expression/expr_starred.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprStarred; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprStarred { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprStarred { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_subscript.rs b/crates/ruff_python_formatter/src/expression/expr_subscript.rs index 5fe53b702c..4efa5c914c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_subscript.rs +++ b/crates/ruff_python_formatter/src/expression/expr_subscript.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprSubscript; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprSubscript { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprSubscript { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index f90c9dce4e..2b687b3787 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprTuple; @@ -10,3 +13,12 @@ impl FormatNodeRule for FormatExprTuple { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprTuple { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match default_expression_needs_parentheses(self.into(), parenthesize, source) { + Parentheses::Optional => Parentheses::Never, + parentheses => parentheses, + } + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs index 35f5ae80fe..c97bc8a56c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprUnaryOp; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprUnaryOp { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprUnaryOp { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_yield.rs b/crates/ruff_python_formatter/src/expression/expr_yield.rs index 697d851c80..0d59010bbb 100644 --- a/crates/ruff_python_formatter/src/expression/expr_yield.rs +++ b/crates/ruff_python_formatter/src/expression/expr_yield.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprYield; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprYield { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprYield { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/expr_yield_from.rs b/crates/ruff_python_formatter/src/expression/expr_yield_from.rs index 164f8fabd4..b0218e1561 100644 --- a/crates/ruff_python_formatter/src/expression/expr_yield_from.rs +++ b/crates/ruff_python_formatter/src/expression/expr_yield_from.rs @@ -1,3 +1,6 @@ +use crate::expression::parentheses::{ + default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize, +}; use crate::{verbatim_text, FormatNodeRule, PyFormatter}; use ruff_formatter::{write, Buffer, FormatResult}; use rustpython_parser::ast::ExprYieldFrom; @@ -10,3 +13,9 @@ impl FormatNodeRule for FormatExprYieldFrom { write!(f, [verbatim_text(item.range)]) } } + +impl NeedsParentheses for ExprYieldFrom { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + default_expression_needs_parentheses(self.into(), parenthesize, source) + } +} diff --git a/crates/ruff_python_formatter/src/expression/maybe_parenthesize.rs b/crates/ruff_python_formatter/src/expression/maybe_parenthesize.rs deleted file mode 100644 index aa7b4f5702..0000000000 --- a/crates/ruff_python_formatter/src/expression/maybe_parenthesize.rs +++ /dev/null @@ -1,53 +0,0 @@ -use crate::context::NodeLevel; -use crate::prelude::*; -use ruff_formatter::{format_args, write}; -use rustpython_parser::ast::Expr; - -/// Formats the passed expression. Adds parentheses if the expression doesn't fit on a line. -pub(crate) const fn maybe_parenthesize(expression: &Expr) -> MaybeParenthesize { - MaybeParenthesize { expression } -} - -pub(crate) struct MaybeParenthesize<'a> { - expression: &'a Expr, -} - -impl Format> for MaybeParenthesize<'_> { - fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { - let saved_level = f.context().node_level(); - f.context_mut().set_node_level(NodeLevel::Parenthesized); - - let result = if needs_parentheses(self.expression) { - write!( - f, - [group(&format_args![ - if_group_breaks(&text("(")), - soft_block_indent(&self.expression.format()), - if_group_breaks(&text(")")) - ])] - ) - } else { - // Don't add parentheses around expressions that have parentheses on their own (e.g. list, dict, tuple, call expression) - self.expression.format().fmt(f) - }; - - f.context_mut().set_node_level(saved_level); - - result - } -} - -const fn needs_parentheses(expr: &Expr) -> bool { - !matches!( - expr, - Expr::Tuple(_) - | Expr::List(_) - | Expr::Set(_) - | Expr::Dict(_) - | Expr::ListComp(_) - | Expr::SetComp(_) - | Expr::DictComp(_) - | Expr::GeneratorExp(_) - | Expr::Call(_) - ) -} diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 039c104d55..a8059c8b74 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -1,5 +1,9 @@ +use crate::context::NodeLevel; +use crate::expression::parentheses::{NeedsParentheses, Parentheses, Parenthesize}; use crate::prelude::*; -use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule, FormatRule}; +use ruff_formatter::{ + format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions, +}; use rustpython_parser::ast::Expr; pub(crate) mod expr_attribute; @@ -29,17 +33,30 @@ pub(crate) mod expr_tuple; pub(crate) mod expr_unary_op; pub(crate) mod expr_yield; pub(crate) mod expr_yield_from; -pub(crate) mod maybe_parenthesize; +pub(crate) mod parentheses; #[derive(Default)] -pub struct FormatExpr; +pub struct FormatExpr { + parenthesize: Parenthesize, +} + +impl FormatRuleWithOptions> for FormatExpr { + type Options = Parenthesize; + + fn with_options(mut self, options: Self::Options) -> Self { + self.parenthesize = options; + self + } +} impl FormatRule> for FormatExpr { fn fmt(&self, item: &Expr, f: &mut PyFormatter) -> FormatResult<()> { - match item { + let parentheses = item.needs_parentheses(self.parenthesize, f.context().contents()); + + let format_expr = format_with(|f| match item { Expr::BoolOp(expr) => expr.format().fmt(f), Expr::NamedExpr(expr) => expr.format().fmt(f), - Expr::BinOp(expr) => expr.format().fmt(f), + Expr::BinOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f), Expr::UnaryOp(expr) => expr.format().fmt(f), Expr::Lambda(expr) => expr.format().fmt(f), Expr::IfExp(expr) => expr.format().fmt(f), @@ -64,6 +81,72 @@ impl FormatRule> for FormatExpr { Expr::List(expr) => expr.format().fmt(f), Expr::Tuple(expr) => expr.format().fmt(f), Expr::Slice(expr) => expr.format().fmt(f), + }); + + let saved_level = f.context().node_level(); + f.context_mut().set_node_level(NodeLevel::Expression); + + let result = match parentheses { + Parentheses::Always => { + write!( + f, + [group(&format_args![ + text("("), + soft_block_indent(&format_expr), + text(")") + ])] + ) + } + // Add optional parentheses. Ignore if the item renders parentheses itself. + Parentheses::Optional => { + write!( + f, + [group(&format_args![ + if_group_breaks(&text("(")), + soft_block_indent(&format_expr), + if_group_breaks(&text(")")) + ])] + ) + } + Parentheses::Custom | Parentheses::Never => Format::fmt(&format_expr, f), + }; + + f.context_mut().set_node_level(saved_level); + + result + } +} + +impl NeedsParentheses for Expr { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses { + match self { + Expr::BoolOp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::NamedExpr(expr) => expr.needs_parentheses(parenthesize, source), + Expr::BinOp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::UnaryOp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Lambda(expr) => expr.needs_parentheses(parenthesize, source), + Expr::IfExp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Dict(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Set(expr) => expr.needs_parentheses(parenthesize, source), + Expr::ListComp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::SetComp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::DictComp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::GeneratorExp(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Await(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Yield(expr) => expr.needs_parentheses(parenthesize, source), + Expr::YieldFrom(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Compare(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Call(expr) => expr.needs_parentheses(parenthesize, source), + Expr::FormattedValue(expr) => expr.needs_parentheses(parenthesize, source), + Expr::JoinedStr(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Constant(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Attribute(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Subscript(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Starred(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Name(expr) => expr.needs_parentheses(parenthesize, source), + Expr::List(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Tuple(expr) => expr.needs_parentheses(parenthesize, source), + Expr::Slice(expr) => expr.needs_parentheses(parenthesize, source), } } } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs new file mode 100644 index 0000000000..7cebc3ebbb --- /dev/null +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -0,0 +1,93 @@ +use crate::trivia::{ + find_first_non_trivia_character_after, find_first_non_trivia_character_before, +}; +use ruff_python_ast::node::AnyNodeRef; + +pub(crate) trait NeedsParentheses { + fn needs_parentheses(&self, parenthesize: Parenthesize, source: &str) -> Parentheses; +} + +pub(super) fn default_expression_needs_parentheses( + node: AnyNodeRef, + parenthesize: Parenthesize, + source: &str, +) -> Parentheses { + debug_assert!( + node.is_expression(), + "Should only be called for expressions" + ); + + // `Optional` or `Preserve` and expression has parentheses in source code. + if !parenthesize.is_if_breaks() && is_expression_parenthesized(node, source) { + Parentheses::Always + } + // `Optional` or `IfBreaks`: Add parentheses if the expression doesn't fit on a line + else if !parenthesize.is_preserve() { + Parentheses::Optional + } else { + //`Preserve` and expression has no parentheses in the source code + Parentheses::Never + } +} + +/// Configures if the expression should be parenthesized. +#[derive(Copy, Clone, Debug, Default)] +pub enum Parenthesize { + /// Parenthesize the expression if it has parenthesis in the source. + #[default] + Preserve, + + /// Parenthesizes the expression if it doesn't fit on a line OR if the expression is parenthesized in the source code. + Optional, + + /// Parenthesizes the expression only if it doesn't fit on a line. + IfBreaks, +} + +impl Parenthesize { + const fn is_if_breaks(self) -> bool { + matches!(self, Parenthesize::IfBreaks) + } + + const fn is_preserve(self) -> bool { + matches!(self, Parenthesize::Preserve) + } +} + +/// Whether it is necessary to add parentheses around an expression. +/// This is different from [`Parenthesize`] in that it is the resolved representation: It takes into account +/// whether there are parentheses in the source code or not. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum Parentheses { + /// Always create parentheses + Always, + + /// Only add parentheses when necessary because the expression breaks over multiple lines. + Optional, + + /// Custom handling by the node's formatter implementation + Custom, + + /// Never add parentheses + Never, +} + +fn is_expression_parenthesized(expr: AnyNodeRef, contents: &str) -> bool { + use rustpython_parser::ast::Ranged; + + debug_assert!( + expr.is_expression(), + "Should only be called for expressions" + ); + + // Search backwards to avoid ambiguity with `(a, )` and because it's faster + matches!( + find_first_non_trivia_character_after(expr.end(), contents), + Some((_, ')')) + ) + // Search forwards to confirm that this is not a nested expression `(5 + d * 3)` + && matches!( + find_first_non_trivia_character_before(expr.start(), contents), + Some((_, '(')) + ) +} diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index 75de5ac303..2c3eb49b37 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -51,7 +51,7 @@ where self.fmt_node(node, f)?; self.fmt_dangling_comments(node, f)?; self.fmt_trailing_comments(node, f)?; - write!(f, [source_position(node.start())]) + write!(f, [source_position(node.end())]) } /// Formats the node without comments. Ignores any suppression comments. @@ -225,8 +225,11 @@ if True: let formatted_code = printed.as_code(); - let reformatted = - format_module(formatted_code).expect("Expected formatted code to be valid syntax"); + let reformatted = format_module(formatted_code).unwrap_or_else(|err| { + panic!( + "Formatted code resulted introduced a syntax error {err:#?}. Code:\n{formatted_code}" + ) + }); if reformatted.as_code() != formatted_code { let diff = TextDiff::from_lines(formatted_code, reformatted.as_code()) @@ -314,7 +317,7 @@ Formatted twice: let formatted_code = printed.as_code(); let reformatted = - format_module(formatted_code).expect("Expected formatted code to be valid syntax"); + format_module(formatted_code).unwrap_or_else(|err| panic!("Expected formatted code to be valid syntax but it contains syntax errors: {err}\n{formatted_code}")); if reformatted.as_code() != formatted_code { let diff = TextDiff::from_lines(formatted_code, reformatted.as_code()) @@ -378,7 +381,9 @@ other // Uncomment the `dbg` to print the IR. // Use `dbg_write!(f, []) instead of `write!(f, [])` in your formatting code to print some IR // inside of a `Format` implementation - // dbg!(formatted.document()); + // dbg!(formatted + // .document() + // .display(formatted.context().source_code())); let printed = formatted.print().unwrap(); diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__collections_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__collections_py.snap index 50f5ab1f05..9b6be2ebde 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__collections_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__collections_py.snap @@ -168,7 +168,7 @@ if True: - 2, - 3, -] -+[1, 2, 3,] ++[1, 2, 3] -division_result_tuple = (6 / 2,) +division_result_tuple = (6/2,) @@ -250,7 +250,7 @@ for x in (1,): for (x,) in (1,), (2,), (3,): pass -[1, 2, 3,] +[1, 2, 3] division_result_tuple = (6/2,) print("foo %r", (foo.bar,)) diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments_py.snap index 12a415a057..58017bff25 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments_py.snap @@ -109,7 +109,21 @@ async def wat(): ```diff --- Black +++ Ruff -@@ -19,14 +19,9 @@ +@@ -4,10 +4,12 @@ + # + # Has many lines. Many, many lines. + # Many, many, many lines. +-"""Module docstring. ++( ++ """Module docstring. + + Possibly also many, many lines. + """ ++) + + import os.path + import sys +@@ -19,9 +21,6 @@ import fast except ImportError: import slow as fast @@ -117,16 +131,9 @@ async def wat(): - -# Some comment before a function. y = 1 --( -- # some strings -- y # type: ignore --) -+# some strings -+y # type: ignore - - - def function(default=None): -@@ -93,4 +88,4 @@ + ( + # some strings +@@ -93,4 +92,4 @@ # Some closing comments. # Maybe Vim or Emacs directives for formatting. @@ -144,10 +151,12 @@ async def wat(): # # Has many lines. Many, many lines. # Many, many, many lines. -"""Module docstring. +( + """Module docstring. Possibly also many, many lines. """ +) import os.path import sys @@ -160,8 +169,10 @@ try: except ImportError: import slow as fast y = 1 -# some strings -y # type: ignore +( + # some strings + y # type: ignore +) def function(default=None): diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__expression_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__expression_py.snap index e59d18b772..41c8b644da 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__expression_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__expression_py.snap @@ -276,17 +276,7 @@ last_call() Name None True -@@ -22,119 +23,83 @@ - v1 << 2 - 1 >> v2 - 1 % finished --1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8 --((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8) -+1 + v2 - v3 * 4 ^ 5 ** v6 / 7 // 8 -+((1 + v2) - (v3 * 4)) ^ (((5 ** v6) / 7) // 8) - not great - ~great - +value +@@ -30,56 +31,39 @@ -1 ~int and not v1 ^ 123 + v2 | True (~int) and (not ((v1 ^ (123 + v2)) | True)) @@ -312,14 +302,14 @@ last_call() (str or None) if True else (str or bytes or None) str or None if (1 if True else 2) else str or bytes or None (str or None) if (1 if True else 2) else (str or bytes or None) --( + ( - (super_long_variable_name or None) - if (1 if super_long_test_name else 2) - else (str or bytes or None) --) ++ (super_long_variable_name or None) if (1 if super_long_test_name else 2) else (str or bytes or None) + ) -{"2.7": dead, "3.7": (long_live or die_hard)} -{"2.7": dead, "3.7": (long_live or die_hard), **{"3.6": verygood}} -+(super_long_variable_name or None) if (1 if super_long_test_name else 2) else (str or bytes or None) +{'2.7': dead, '3.7': (long_live or die_hard)} +{'2.7': dead, '3.7': (long_live or die_hard), **{'3.6': verygood}} {**a, **b, **c} @@ -338,7 +328,7 @@ last_call() - 2, - 3, -] -+[1, 2, 3,] ++[1, 2, 3] [*a] [*range(10)] -[ @@ -351,15 +341,14 @@ last_call() - *a, - 5, -] --[ -- this_is_a_very_long_variable_which_will_force_a_delimiter_split, -- element, -- another, -- *more, --] -+[*a, 4, 5,] -+[4, *a, 5,] -+[this_is_a_very_long_variable_which_will_force_a_delimiter_split, element, another, *more] ++[*a, 4, 5] ++[4, *a, 5] + [ + this_is_a_very_long_variable_which_will_force_a_delimiter_split, + element, +@@ -87,54 +71,44 @@ + *more, + ] {i for i in (1, 2, 3)} -{(i**2) for i in (1, 2, 3)} -{(i**2) for i, _ in ((1, "a"), (2, "b"), (3, "c"))} @@ -417,18 +406,21 @@ last_call() dict[str, int] tuple[str, ...] -tuple[str, int, float, dict[str, int]] - tuple[ +-tuple[ - str, - int, - float, - dict[str, int], ++( ++ tuple[ + str, int, float, dict[str, int] ] ++) +tuple[str, int, float, dict[str, int],] very_long_variable_name_filters: t.List[ t.Tuple[str, t.Union[str, t.List[t.Optional[str]]]], ] -@@ -144,9 +109,9 @@ +@@ -144,9 +118,9 @@ xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod( # type: ignore sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__) ) @@ -441,7 +433,7 @@ last_call() slice[0] slice[0:1] slice[0:1:2] -@@ -174,57 +139,29 @@ +@@ -174,57 +148,29 @@ numpy[:, ::-1] numpy[np.newaxis, :] (str or None) if (sys.version_info[0] > (3,)) else (str or bytes or None) @@ -450,9 +442,8 @@ last_call() +{'2.7': dead, '3.7': long_live or die_hard} +{'2.7', '3.6', '3.7', '3.8', '3.9', '4.0' if gilectomy else '3.10'} [1, 2, 3, 4, 5, 6, 7, 8, 9, 10 or A, 11 or B, 12 or C] --(SomeName) + (SomeName) SomeName -+SomeName (Good, Bad, Ugly) (i for i in (1, 2, 3)) -((i**2) for i in (1, 2, 3)) @@ -511,7 +502,7 @@ last_call() Ø = set() authors.łukasz.say_thanks() mapping = { -@@ -237,134 +174,86 @@ +@@ -237,114 +183,80 @@ def gen(): yield from outside_of_generator @@ -655,30 +646,7 @@ last_call() + ~ aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h ^ aaaaaaaaaaaaaaaa.i << aaaaaaaaaaaaaaaa.k >> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n ): return True --( -- aaaaaaaaaaaaaaaa -- + aaaaaaaaaaaaaaaa -- - aaaaaaaaaaaaaaaa -- * (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) -- / (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) --) -+aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaa * (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) / (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) - aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa --( -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -- >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -- << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa --) -+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - bbbb >> bbbb * bbbb --( -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -- ^ bbbb.a & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -- ^ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa --) -+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ^bbbb.a & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - last_call() - # standalone comment at ENDMARKER + ( ``` ## Ruff Output @@ -709,8 +677,8 @@ Name1 or Name2 and Name3 or Name4 v1 << 2 1 >> v2 1 % finished -1 + v2 - v3 * 4 ^ 5 ** v6 / 7 // 8 -((1 + v2) - (v3 * 4)) ^ (((5 ** v6) / 7) // 8) +1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8 +((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8) not great ~great +value @@ -731,7 +699,9 @@ str or None if True else str or bytes or None (str or None) if True else (str or bytes or None) str or None if (1 if True else 2) else str or bytes or None (str or None) if (1 if True else 2) else (str or bytes or None) -(super_long_variable_name or None) if (1 if super_long_test_name else 2) else (str or bytes or None) +( + (super_long_variable_name or None) if (1 if super_long_test_name else 2) else (str or bytes or None) +) {'2.7': dead, '3.7': (long_live or die_hard)} {'2.7': dead, '3.7': (long_live or die_hard), **{'3.6': verygood}} {**a, **b, **c} @@ -743,12 +713,17 @@ str or None if (1 if True else 2) else str or bytes or None (1, 2, 3) [] [1, 2, 3, 4, 5, 6, 7, 8, 9, (10 or A), (11 or B), (12 or C)] -[1, 2, 3,] +[1, 2, 3] [*a] [*range(10)] -[*a, 4, 5,] -[4, *a, 5,] -[this_is_a_very_long_variable_which_will_force_a_delimiter_split, element, another, *more] +[*a, 4, 5] +[4, *a, 5] +[ + this_is_a_very_long_variable_which_will_force_a_delimiter_split, + element, + another, + *more, +] {i for i in (1, 2, 3)} {(i ** 2) for i in (1, 2, 3)} {(i ** 2) for i, _ in ((1, 'a'), (2, 'b'), (3, 'c'))} @@ -782,9 +757,11 @@ call.me(maybe) list[str] dict[str, int] tuple[str, ...] -tuple[ +( + tuple[ str, int, float, dict[str, int] ] +) tuple[str, int, float, dict[str, int],] very_long_variable_name_filters: t.List[ t.Tuple[str, t.Union[str, t.List[t.Optional[str]]]], @@ -828,7 +805,7 @@ numpy[np.newaxis, :] {'2.7': dead, '3.7': long_live or die_hard} {'2.7', '3.6', '3.7', '3.8', '3.9', '4.0' if gilectomy else '3.10'} [1, 2, 3, 4, 5, 6, 7, 8, 9, 10 or A, 11 or B, 12 or C] -SomeName +(SomeName) SomeName (Good, Bad, Ugly) (i for i in (1, 2, 3)) @@ -936,11 +913,25 @@ if ( ~ aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h ^ aaaaaaaaaaaaaaaa.i << aaaaaaaaaaaaaaaa.k >> aaaaaaaaaaaaaaaa.l ** aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n ): return True -aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa - aaaaaaaaaaaaaaaa * (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) / (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) +( + aaaaaaaaaaaaaaaa + + aaaaaaaaaaaaaaaa + - aaaaaaaaaaaaaaaa + * (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) + / (aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa) +) aaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +( + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +) bbbb >> bbbb * bbbb -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ^bbbb.a & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +( + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ^ bbbb.a & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ^ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +) last_call() # standalone comment at ENDMARKER ``` diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__slices_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__slices_py.snap index 902e4035e2..fcfa1edd86 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__slices_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__slices_py.snap @@ -76,16 +76,51 @@ x[ ```diff --- Black +++ Ruff -@@ -56,4 +56,8 @@ +@@ -31,14 +31,17 @@ + ham[lower + offset : upper + offset] + + slice[::, ::] +-slice[ ++( ++ slice[ + # A + : + # B + : + # C + ] +-slice[ ++) ++( ++ slice[ + # A + 1: + # B +@@ -46,8 +49,10 @@ + # C + 3 + ] ++) + +-slice[ ++( ++ slice[ + # A + 1 + + 2 : +@@ -56,4 +61,11 @@ # C 4 ] -x[1:2:3] # A # B # C -+x[ ++) ++( ++ x[ + 1: # A + 2: # B + 3 # C +] ++) ``` ## Ruff Output @@ -124,14 +159,17 @@ ham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)] ham[lower + offset : upper + offset] slice[::, ::] -slice[ +( + slice[ # A : # B : # C ] -slice[ +) +( + slice[ # A 1: # B @@ -139,8 +177,10 @@ slice[ # C 3 ] +) -slice[ +( + slice[ # A 1 + 2 : @@ -149,11 +189,14 @@ slice[ # C 4 ] -x[ +) +( + x[ 1: # A 2: # B 3 # C ] +) ``` ## Black Output diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__torture_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__torture_py.snap index 95a7d91907..42d322316d 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__torture_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__torture_py.snap @@ -42,14 +42,13 @@ assert ( ```diff --- Black +++ Ruff -@@ -1,58 +1,33 @@ - importA --( -- () -- << 0 +@@ -2,18 +2,13 @@ + ( + () + << 0 - ** 101234234242352525425252352352525234890264906820496920680926538059059209922523523525 --) # -+() << 0 ** 101234234242352525425252352352525234890264906820496920680926538059059209922523523525 # ++ **101234234242352525425252352352525234890264906820496920680926538059059209922523523525 + ) # assert sort_by_dependency( { @@ -65,12 +64,7 @@ assert ( } ) == ["2a", "2b", "2", "3a", "3b", "3", "1"] - importA - 0 --0 ^ 0 # -+0^0 # - - +@@ -25,34 +20,18 @@ class A: def foo(self): for _ in range(10): @@ -120,7 +114,11 @@ assert ( ```py importA -() << 0 ** 101234234242352525425252352352525234890264906820496920680926538059059209922523523525 # +( + () + << 0 + **101234234242352525425252352352525234890264906820496920680926538059059209922523523525 +) # assert sort_by_dependency( { @@ -131,7 +129,7 @@ assert sort_by_dependency( importA 0 -0^0 # +0 ^ 0 # class A: diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__binary_expression_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__binary_expression_py.snap new file mode 100644 index 0000000000..fc76324221 --- /dev/null +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__binary_expression_py.snap @@ -0,0 +1,123 @@ +--- +source: crates/ruff_python_formatter/src/lib.rs +expression: snapshot +--- +## Input +```py +(aaaaaaaa + + # trailing operator comment + b # trailing right comment +) + + +(aaaaaaaa # trailing left comment + + # trailing operator comment + # leading right comment + b +) + + +# Black breaks the right side first for the following expressions: +aaaaaaaaaaaaaa + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(argument1, argument2, argument3) +aaaaaaaaaaaaaa + [bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee] +aaaaaaaaaaaaaa + (bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee) +aaaaaaaaaaaaaa + { key1:bbbbbbbbbbbbbbbbbbbbbb, key2: ccccccccccccccccccccc, key3: dddddddddddddddd, key4: eeeeeee } +aaaaaaaaaaaaaa + { bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee } +aaaaaaaaaaaaaa + [a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ] +aaaaaaaaaaaaaa + (a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ) +aaaaaaaaaaaaaa + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb} + +# Wraps it in parentheses if it needs to break both left and right +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + [ + bbbbbbbbbbbbbbbbbbbbbb, + ccccccccccccccccccccc, + dddddddddddddddd, + eee +] # comment + + + +# But only for expressions that have a statement parent. +not (aaaaaaaaaaaaaa + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb}) +[a + [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] in c ] + + +# leading comment +( + # comment + content + b +) +``` + + + +## Output +```py +( + aaaaaaaa + + b # trailing operator comment # trailing right comment +) + + +( + aaaaaaaa # trailing left comment + + # trailing operator comment + # leading right comment + b +) +# Black breaks the right side first for the following expressions: +( + aaaaaaaaaaaaaa + + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(argument1, argument2, argument3) +) +aaaaaaaaaaaaaa + [ + bbbbbbbbbbbbbbbbbbbbbb, + ccccccccccccccccccccc, + dddddddddddddddd, + eeeeeee, +] +( + aaaaaaaaaaaaaa + + (bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee) +) +( + aaaaaaaaaaaaaa + + { key1:bbbbbbbbbbbbbbbbbbbbbb, key2: ccccccccccccccccccccc, key3: dddddddddddddddd, key4: eeeeeee } +) +( + aaaaaaaaaaaaaa + + { bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eeeeeee } +) +( + aaaaaaaaaaaaaa + + [a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ] +) +( + aaaaaaaaaaaaaa + + (a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ) +) +( + aaaaaaaaaaaaaa + + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb} +) +# Wraps it in parentheses if it needs to break both left and right +( + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + [bbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc, dddddddddddddddd, eee] +) # comment +# But only for expressions that have a statement parent. +( + not (aaaaaaaaaaaaaa + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb}) +) +[ + a + [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] in c, +] +# leading comment +( + # comment + content + + b +) +``` + + diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__while_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__statement__while_py.snap similarity index 100% rename from crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__while_py.snap rename to crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__statement__while_py.snap diff --git a/crates/ruff_python_formatter/src/statement/stmt_expr.rs b/crates/ruff_python_formatter/src/statement/stmt_expr.rs index 691411c762..ffb86ee9df 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_expr.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_expr.rs @@ -1,3 +1,4 @@ +use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::FormatNodeRule; use rustpython_parser::ast::StmtExpr; @@ -9,6 +10,6 @@ impl FormatNodeRule for FormatStmtExpr { fn fmt_fields(&self, item: &StmtExpr, f: &mut PyFormatter) -> FormatResult<()> { let StmtExpr { value, .. } = item; - value.format().fmt(f) + value.format().with_options(Parenthesize::Optional).fmt(f) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_while.rs b/crates/ruff_python_formatter/src/statement/stmt_while.rs index 7ff55a21de..1ed71397fd 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_while.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_while.rs @@ -1,5 +1,5 @@ use crate::comments::{leading_alternate_branch_comments, trailing_comments}; -use crate::expression::maybe_parenthesize::maybe_parenthesize; +use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::FormatNodeRule; use ruff_formatter::write; @@ -33,7 +33,7 @@ impl FormatNodeRule for FormatStmtWhile { [ text("while"), space(), - maybe_parenthesize(test), + test.format().with_options(Parenthesize::IfBreaks), text(":"), trailing_comments(trailing_condition_comments), block_indent(&body.format()) diff --git a/crates/ruff_python_formatter/src/trivia.rs b/crates/ruff_python_formatter/src/trivia.rs index e1224883b0..1ffd3f39df 100644 --- a/crates/ruff_python_formatter/src/trivia.rs +++ b/crates/ruff_python_formatter/src/trivia.rs @@ -10,8 +10,8 @@ use ruff_text_size::{TextLen, TextRange, TextSize}; /// /// Returns `None` if the range is empty or only contains trivia (whitespace or comments). pub(crate) fn find_first_non_trivia_character_in_range( - code: &str, range: TextRange, + code: &str, ) -> Option<(TextSize, char)> { let rest = &code[range]; let mut char_iter = rest.chars(); @@ -40,8 +40,63 @@ pub(crate) fn find_first_non_trivia_character_in_range( None } +pub(crate) fn find_first_non_trivia_character_after( + offset: TextSize, + code: &str, +) -> Option<(TextSize, char)> { + find_first_non_trivia_character_in_range(TextRange::new(offset, code.text_len()), code) +} + +pub(crate) fn find_first_non_trivia_character_before( + offset: TextSize, + code: &str, +) -> Option<(TextSize, char)> { + let head = &code[TextRange::up_to(offset)]; + let mut char_iter = head.chars(); + + while let Some(c) = char_iter.next_back() { + match c { + c if is_python_whitespace(c) => { + continue; + } + + // Empty comment + '#' => continue, + + non_trivia_character => { + // Non trivia character but we don't know if it is a comment or not. Consume all characters + // until the start of the line and track if the last non-whitespace character was a `#`. + let mut is_comment = false; + + let first_non_trivia_offset = char_iter.as_str().text_len(); + + while let Some(c) = char_iter.next_back() { + match c { + '#' => { + is_comment = true; + } + '\n' | '\r' => { + if !is_comment { + return Some((first_non_trivia_offset, non_trivia_character)); + } + } + + c => { + if !is_python_whitespace(c) { + is_comment = false; + } + } + } + } + } + } + } + + None +} + /// Returns the number of newlines between `offset` and the first non whitespace character in the source code. -pub(crate) fn lines_before(code: &str, offset: TextSize) -> u32 { +pub(crate) fn lines_before(offset: TextSize, code: &str) -> u32 { let head = &code[TextRange::up_to(offset)]; let mut newlines = 0u32; @@ -68,7 +123,7 @@ pub(crate) fn lines_before(code: &str, offset: TextSize) -> u32 { } /// Counts the empty lines between `offset` and the first non-whitespace character. -pub(crate) fn lines_after(code: &str, offset: TextSize) -> u32 { +pub(crate) fn lines_after(offset: TextSize, code: &str) -> u32 { let rest = &code[usize::from(offset)..]; let mut newlines = 0; @@ -98,33 +153,33 @@ mod tests { #[test] fn lines_before_empty_string() { - assert_eq!(lines_before("", TextSize::new(0)), 0); + assert_eq!(lines_before(TextSize::new(0), ""), 0); } #[test] fn lines_before_in_the_middle_of_a_line() { - assert_eq!(lines_before("a = 20", TextSize::new(4)), 0); + assert_eq!(lines_before(TextSize::new(4), "a = 20"), 0); } #[test] fn lines_before_on_a_new_line() { - assert_eq!(lines_before("a = 20\nb = 10", TextSize::new(7)), 1); + assert_eq!(lines_before(TextSize::new(7), "a = 20\nb = 10"), 1); } #[test] fn lines_before_multiple_leading_newlines() { - assert_eq!(lines_before("a = 20\n\r\nb = 10", TextSize::new(9)), 2); + assert_eq!(lines_before(TextSize::new(9), "a = 20\n\r\nb = 10"), 2); } #[test] fn lines_before_with_comment_offset() { - assert_eq!(lines_before("a = 20\n# a comment", TextSize::new(8)), 0); + assert_eq!(lines_before(TextSize::new(8), "a = 20\n# a comment"), 0); } #[test] fn lines_before_with_trailing_comment() { assert_eq!( - lines_before("a = 20 # some comment\nb = 10", TextSize::new(22)), + lines_before(TextSize::new(22), "a = 20 # some comment\nb = 10"), 1 ); } @@ -132,40 +187,40 @@ mod tests { #[test] fn lines_before_with_comment_only_line() { assert_eq!( - lines_before("a = 20\n# some comment\nb = 10", TextSize::new(22)), + lines_before(TextSize::new(22), "a = 20\n# some comment\nb = 10"), 1 ); } #[test] fn lines_after_empty_string() { - assert_eq!(lines_after("", TextSize::new(0)), 0); + assert_eq!(lines_after(TextSize::new(0), ""), 0); } #[test] fn lines_after_in_the_middle_of_a_line() { - assert_eq!(lines_after("a = 20", TextSize::new(4)), 0); + assert_eq!(lines_after(TextSize::new(4), "a = 20"), 0); } #[test] fn lines_after_before_a_new_line() { - assert_eq!(lines_after("a = 20\nb = 10", TextSize::new(6)), 1); + assert_eq!(lines_after(TextSize::new(6), "a = 20\nb = 10"), 1); } #[test] fn lines_after_multiple_newlines() { - assert_eq!(lines_after("a = 20\n\r\nb = 10", TextSize::new(6)), 2); + assert_eq!(lines_after(TextSize::new(6), "a = 20\n\r\nb = 10"), 2); } #[test] fn lines_after_before_comment_offset() { - assert_eq!(lines_after("a = 20 # a comment\n", TextSize::new(7)), 0); + assert_eq!(lines_after(TextSize::new(7), "a = 20 # a comment\n"), 0); } #[test] fn lines_after_with_comment_only_line() { assert_eq!( - lines_after("a = 20\n# some comment\nb = 10", TextSize::new(6)), + lines_after(TextSize::new(6), "a = 20\n# some comment\nb = 10"), 1 ); } diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 09cadf47cf..be3edffe05 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -23,6 +23,8 @@ pub struct Scope<'a> { bindings: FxHashMap<&'a str, BindingId>, /// A map from binding ID to binding ID that it shadows. shadowed_bindings: HashMap>, + /// A list of all names that have been deleted in this scope. + deleted_symbols: Vec<&'a str>, /// Index into the globals arena, if the scope contains any globally-declared symbols. globals_id: Option, } @@ -36,6 +38,7 @@ impl<'a> Scope<'a> { star_imports: Vec::default(), bindings: FxHashMap::default(), shadowed_bindings: IntMap::default(), + deleted_symbols: Vec::default(), globals_id: None, } } @@ -48,6 +51,7 @@ impl<'a> Scope<'a> { star_imports: Vec::default(), bindings: FxHashMap::default(), shadowed_bindings: IntMap::default(), + deleted_symbols: Vec::default(), globals_id: None, } } @@ -67,14 +71,23 @@ impl<'a> Scope<'a> { } } - /// Returns `true` if this scope defines a binding with the given name. - pub fn defines(&self, name: &str) -> bool { + /// Removes the binding with the given name. + pub fn delete(&mut self, name: &'a str) -> Option { + self.deleted_symbols.push(name); + self.bindings.remove(name) + } + + /// Returns `true` if this scope has a binding with the given name. + pub fn has(&self, name: &str) -> bool { self.bindings.contains_key(name) } - /// Removes the binding with the given name - pub fn remove(&mut self, name: &str) -> Option { - self.bindings.remove(name) + /// Returns `true` if the scope declares a symbol with the given name. + /// + /// Unlike [`Scope::has`], the name may no longer be bound to a value (e.g., it could be + /// deleted). + pub fn declares(&self, name: &str) -> bool { + self.has(name) || self.deleted_symbols.contains(&name) } /// Returns the ids of all bindings defined in this scope. diff --git a/docs/tutorial.md b/docs/tutorial.md index b2dac8d196..d32edc5861 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -242,7 +242,7 @@ This tutorial has focused on Ruff's command-line interface, but Ruff can also be ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.0.270 + rev: v0.0.271 hooks: - id: ruff ``` diff --git a/docs/usage.md b/docs/usage.md index 812ed466fa..0e901a9674 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -22,7 +22,7 @@ Ruff can also be used as a [pre-commit](https://pre-commit.com) hook: ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.0.270 + rev: v0.0.271 hooks: - id: ruff ``` @@ -32,7 +32,7 @@ Or, to enable autofix: ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.0.270 + rev: v0.0.271 hooks: - id: ruff args: [ --fix, --exit-non-zero-on-fix ] diff --git a/pyproject.toml b/pyproject.toml index 77118d4e10..add2cda545 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "maturin" [project] name = "ruff" -version = "0.0.270" +version = "0.0.271" description = "An extremely fast Python linter, written in Rust." authors = [{ name = "Charlie Marsh", email = "charlie.r.marsh@gmail.com" }] maintainers = [{ name = "Charlie Marsh", email = "charlie.r.marsh@gmail.com" }]