From 4b8c815b279dc407da55b106e86edcb3f267805a Mon Sep 17 00:00:00 2001 From: InSync Date: Tue, 10 Dec 2024 15:39:46 +0700 Subject: [PATCH] [`flake8-bugbear`] `itertools.batched()` without explicit `strict` (`B911`) (#14408) ## Summary Resolves #14387. ## Test Plan `cargo nextest run` and `cargo insta test`. --------- Co-authored-by: Micha Reiser --- .../test/fixtures/flake8_bugbear/B911.py | 59 ++++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_bugbear/mod.rs | 1 + .../rules/batched_without_explicit_strict.rs | 91 ++++++++++ .../src/rules/flake8_bugbear/rules/mod.rs | 2 + .../rules/zip_without_explicit_strict.rs | 6 +- ...__flake8_bugbear__tests__B911_B911.py.snap | 169 ++++++++++++++++++ ruff.schema.json | 2 + 9 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B911.py create mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/rules/batched_without_explicit_strict.rs create mode 100644 crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B911_B911.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B911.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B911.py new file mode 100644 index 0000000000..44a027a88a --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B911.py @@ -0,0 +1,59 @@ +from itertools import batched, count, cycle, repeat + + +# Errors +batched(range(3), 1) +batched("abc", 2) +batched([i for i in range(42)], some_n) +batched((foo for foo in cycle())) +batched(itertools.batched([1, 2, 3], strict=True)) + +# Errors (limited iterators). +batched(repeat(1, 1)) +batched(repeat(1, times=4)) + +# No fix +batched([], **kwargs) + +# No errors +batched() +batched(range(3), 0, strict=True) +batched(["a", "b"], count, strict=False) +batched(("a", "b", "c"), zip(repeat()), strict=True) + +# No errors (infinite iterators) +batched(cycle("ABCDEF"), 3) +batched(count(), qux + lorem) +batched(repeat(1), ipsum // 19 @ 0x1) +batched(repeat(1, None)) +batched(repeat(1, times=None)) + + +import itertools + +# Errors +itertools.batched(range(3), 1) +itertools.batched("abc", 2) +itertools.batched([i for i in range(42)], some_n) +itertools.batched((foo for foo in cycle())) +itertools.batched(itertools.batched([1, 2, 3], strict=True)) + +# Errors (limited iterators). +itertools.batched(repeat(1, 1)) +itertools.batched(repeat(1, times=4)) + +# No fix +itertools.batched([], **kwargs) + +# No errors +itertools.batched() +itertools.batched(range(3), 0, strict=True) +itertools.batched(["a", "b"], count, strict=False) +itertools.batched(("a", "b", "c"), zip(repeat()), strict=True) + +# No errors (infinite iterators) +itertools.batched(cycle("ABCDEF"), 3) +itertools.batched(count(), qux + lorem) +itertools.batched(repeat(1), ipsum // 19 @ 0x1) +itertools.batched(repeat(1, None)) +itertools.batched(repeat(1, times=None)) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index b86389c74d..10f56f3f99 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1099,6 +1099,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::DotlessPathlibWithSuffix) { flake8_use_pathlib::rules::dotless_pathlib_with_suffix(checker, call); } + if checker.enabled(Rule::BatchedWithoutExplicitStrict) { + flake8_bugbear::rules::batched_without_explicit_strict(checker, call); + } } Expr::Dict(dict) => { if checker.any_enabled(&[ diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 7a87494341..c673a0987d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -358,6 +358,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept), (Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict), (Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation), + (Flake8Bugbear, "911") => (RuleGroup::Preview, rules::flake8_bugbear::rules::BatchedWithoutExplicitStrict), // flake8-blind-except (Flake8BlindExcept, "001") => (RuleGroup::Stable, rules::flake8_blind_except::rules::BlindExcept), diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs index afe9d349f4..b254847c38 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/mod.rs @@ -66,6 +66,7 @@ mod tests { #[test_case(Rule::ReturnInGenerator, Path::new("B901.py"))] #[test_case(Rule::LoopIteratorMutation, Path::new("B909.py"))] #[test_case(Rule::MutableContextvarDefault, Path::new("B039.py"))] + #[test_case(Rule::BatchedWithoutExplicitStrict, Path::new("B911.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/batched_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/batched_without_explicit_strict.rs new file mode 100644 index 0000000000..34893b109f --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/batched_without_explicit_strict.rs @@ -0,0 +1,91 @@ +use crate::checkers::ast::Checker; +use crate::rules::flake8_bugbear::rules::is_infinite_iterable; +use crate::settings::types::PythonVersion; +use ruff_diagnostics::{Diagnostic, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_ast::ExprCall; + +/// ## What it does +/// Checks for `itertools.batched` calls without an explicit `strict` parameter. +/// +/// ## Why is this bad? +/// By default, if the length of the iterable is not divisible by +/// the second argument to `itertools.batched`, the last batch +/// will be shorter than the rest. +/// +/// In Python 3.13, a `strict` parameter was added which allows controlling if the batches must be of uniform length. +/// Pass `strict=True` to raise a `ValueError` if the batches are of non-uniform length. +/// Otherwise, pass `strict=False` to make the intention explicit. +/// +/// ## Example +/// ```python +/// itertools.batched(iterable, n) +/// ``` +/// +/// Use instead if the batches must be of uniform length: +/// ```python +/// itertools.batched(iterable, n, strict=True) +/// ``` +/// +/// Or if the batches can be of non-uniform length: +/// ```python +/// itertools.batched(iterable, n, strict=False) +/// ``` +/// +/// ## Known deviations +/// Unlike the upstream `B911`, this rule will not report infinite iterators +/// (e.g., `itertools.cycle(...)`). +/// +/// ## Options +/// - `target-version` +/// +/// ## References +/// - [Python documentation: `batched`](https://docs.python.org/3/library/itertools.html#batched) +#[derive(ViolationMetadata)] +pub(crate) struct BatchedWithoutExplicitStrict; + +impl Violation for BatchedWithoutExplicitStrict { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + "`itertools.batched()` without an explicit `strict` parameter".to_string() + } + + fn fix_title(&self) -> Option { + Some("Add an explicit `strict` parameter".to_string()) + } +} + +/// B911 +pub(crate) fn batched_without_explicit_strict(checker: &mut Checker, call: &ExprCall) { + if checker.settings.target_version < PythonVersion::Py313 { + return; + } + + let semantic = checker.semantic(); + let (func, arguments) = (&call.func, &call.arguments); + + let Some(qualified_name) = semantic.resolve_qualified_name(func) else { + return; + }; + + if !matches!(qualified_name.segments(), ["itertools", "batched"]) { + return; + } + + if arguments.find_keyword("strict").is_some() { + return; + } + + let Some(iterable) = arguments.find_positional(0) else { + return; + }; + + if is_infinite_iterable(iterable, semantic) { + return; + } + + let diagnostic = Diagnostic::new(BatchedWithoutExplicitStrict, call.range); + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs index b4af7755dd..8fcca6883e 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs @@ -2,6 +2,7 @@ pub(crate) use abstract_base_class::*; pub(crate) use assert_false::*; pub(crate) use assert_raises_exception::*; pub(crate) use assignment_to_os_environ::*; +pub(crate) use batched_without_explicit_strict::*; pub(crate) use cached_instance_method::*; pub(crate) use duplicate_exceptions::*; pub(crate) use duplicate_value::*; @@ -40,6 +41,7 @@ mod abstract_base_class; mod assert_false; mod assert_raises_exception; mod assignment_to_os_environ; +mod batched_without_explicit_strict; mod cached_instance_method; mod duplicate_exceptions; mod duplicate_value; diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs index 272ea7a596..9d512b577b 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs @@ -17,7 +17,7 @@ use crate::fix::edits::add_argument; /// iterable. This can lead to subtle bugs. /// /// Pass `strict=True` to raise a `ValueError` if the iterables are of -/// non-uniform length. Alternatively, if the iterables are deliberately +/// non-uniform length. Alternatively, if the iterables are deliberately of /// different lengths, pass `strict=False` to make the intention explicit. /// /// ## Example @@ -61,7 +61,7 @@ pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::Exp .arguments .args .iter() - .any(|arg| is_infinite_iterator(arg, semantic)) + .any(|arg| is_infinite_iterable(arg, semantic)) { checker.diagnostics.push( Diagnostic::new(ZipWithoutExplicitStrict, call.range()).with_fix(Fix::applicable_edit( @@ -89,7 +89,7 @@ pub(crate) fn zip_without_explicit_strict(checker: &mut Checker, call: &ast::Exp /// Return `true` if the [`Expr`] appears to be an infinite iterator (e.g., a call to /// `itertools.cycle` or similar). -fn is_infinite_iterator(arg: &Expr, semantic: &SemanticModel) -> bool { +pub(crate) fn is_infinite_iterable(arg: &Expr, semantic: &SemanticModel) -> bool { let Expr::Call(ast::ExprCall { func, arguments: Arguments { args, keywords, .. }, diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B911_B911.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B911_B911.py.snap new file mode 100644 index 0000000000..850a173070 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B911_B911.py.snap @@ -0,0 +1,169 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs +snapshot_kind: text +--- +B911.py:5:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +4 | # Errors +5 | batched(range(3), 1) + | ^^^^^^^^^^^^^^^^^^^^ B911 +6 | batched("abc", 2) +7 | batched([i for i in range(42)], some_n) + | + = help: Add an explicit `strict` parameter + +B911.py:6:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +4 | # Errors +5 | batched(range(3), 1) +6 | batched("abc", 2) + | ^^^^^^^^^^^^^^^^^ B911 +7 | batched([i for i in range(42)], some_n) +8 | batched((foo for foo in cycle())) + | + = help: Add an explicit `strict` parameter + +B911.py:7:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +5 | batched(range(3), 1) +6 | batched("abc", 2) +7 | batched([i for i in range(42)], some_n) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +8 | batched((foo for foo in cycle())) +9 | batched(itertools.batched([1, 2, 3], strict=True)) + | + = help: Add an explicit `strict` parameter + +B911.py:8:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +6 | batched("abc", 2) +7 | batched([i for i in range(42)], some_n) +8 | batched((foo for foo in cycle())) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +9 | batched(itertools.batched([1, 2, 3], strict=True)) + | + = help: Add an explicit `strict` parameter + +B911.py:9:1: B911 `itertools.batched()` without an explicit `strict` parameter + | + 7 | batched([i for i in range(42)], some_n) + 8 | batched((foo for foo in cycle())) + 9 | batched(itertools.batched([1, 2, 3], strict=True)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +10 | +11 | # Errors (limited iterators). + | + = help: Add an explicit `strict` parameter + +B911.py:12:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +11 | # Errors (limited iterators). +12 | batched(repeat(1, 1)) + | ^^^^^^^^^^^^^^^^^^^^^ B911 +13 | batched(repeat(1, times=4)) + | + = help: Add an explicit `strict` parameter + +B911.py:13:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +11 | # Errors (limited iterators). +12 | batched(repeat(1, 1)) +13 | batched(repeat(1, times=4)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +14 | +15 | # No fix + | + = help: Add an explicit `strict` parameter + +B911.py:16:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +15 | # No fix +16 | batched([], **kwargs) + | ^^^^^^^^^^^^^^^^^^^^^ B911 +17 | +18 | # No errors + | + = help: Add an explicit `strict` parameter + +B911.py:35:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +34 | # Errors +35 | itertools.batched(range(3), 1) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +36 | itertools.batched("abc", 2) +37 | itertools.batched([i for i in range(42)], some_n) + | + = help: Add an explicit `strict` parameter + +B911.py:36:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +34 | # Errors +35 | itertools.batched(range(3), 1) +36 | itertools.batched("abc", 2) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +37 | itertools.batched([i for i in range(42)], some_n) +38 | itertools.batched((foo for foo in cycle())) + | + = help: Add an explicit `strict` parameter + +B911.py:37:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +35 | itertools.batched(range(3), 1) +36 | itertools.batched("abc", 2) +37 | itertools.batched([i for i in range(42)], some_n) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +38 | itertools.batched((foo for foo in cycle())) +39 | itertools.batched(itertools.batched([1, 2, 3], strict=True)) + | + = help: Add an explicit `strict` parameter + +B911.py:38:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +36 | itertools.batched("abc", 2) +37 | itertools.batched([i for i in range(42)], some_n) +38 | itertools.batched((foo for foo in cycle())) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +39 | itertools.batched(itertools.batched([1, 2, 3], strict=True)) + | + = help: Add an explicit `strict` parameter + +B911.py:39:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +37 | itertools.batched([i for i in range(42)], some_n) +38 | itertools.batched((foo for foo in cycle())) +39 | itertools.batched(itertools.batched([1, 2, 3], strict=True)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +40 | +41 | # Errors (limited iterators). + | + = help: Add an explicit `strict` parameter + +B911.py:42:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +41 | # Errors (limited iterators). +42 | itertools.batched(repeat(1, 1)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +43 | itertools.batched(repeat(1, times=4)) + | + = help: Add an explicit `strict` parameter + +B911.py:43:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +41 | # Errors (limited iterators). +42 | itertools.batched(repeat(1, 1)) +43 | itertools.batched(repeat(1, times=4)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +44 | +45 | # No fix + | + = help: Add an explicit `strict` parameter + +B911.py:46:1: B911 `itertools.batched()` without an explicit `strict` parameter + | +45 | # No fix +46 | itertools.batched([], **kwargs) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B911 +47 | +48 | # No errors + | + = help: Add an explicit `strict` parameter diff --git a/ruff.schema.json b/ruff.schema.json index 85e2c08b91..a464a0b142 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2891,6 +2891,8 @@ "B904", "B905", "B909", + "B91", + "B911", "BLE", "BLE0", "BLE00",