[`flake8-bugbear`] Implement `re-sub-positional-args` (`B034`) (#5669)

## Summary

Needed to do some coding to end the day.

Closes #5665.

crates/ruff/resources/test/fixtures/flake8_bugbear/B034.py

@@ -0,0 +1,27 @@
+import re
+from re import sub
+
+# B034
+re.sub("a", "b", "aaa", re.IGNORECASE)
+re.sub("a", "b", "aaa", 5)
+re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+re.subn("a", "b", "aaa", re.IGNORECASE)
+re.subn("a", "b", "aaa", 5)
+re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+re.split(" ", "a a a a", re.I)
+re.split(" ", "a a a a", 2)
+re.split(" ", "a a a a", 2, re.I)
+sub("a", "b", "aaa", re.IGNORECASE)
+
+# OK
+re.sub("a", "b", "aaa")
+re.sub("a", "b", "aaa", flags=re.IGNORECASE)
+re.sub("a", "b", "aaa", count=5)
+re.sub("a", "b", "aaa", count=5, flags=re.IGNORECASE)
+re.subn("a", "b", "aaa")
+re.subn("a", "b", "aaa", flags=re.IGNORECASE)
+re.subn("a", "b", "aaa", count=5)
+re.subn("a", "b", "aaa", count=5, flags=re.IGNORECASE)
+re.split(" ", "a a a a", flags=re.I)
+re.split(" ", "a a a a", maxsplit=2)
+re.split(" ", "a a a a", maxsplit=2, flags=re.I)

crates/ruff/src/checkers/ast/mod.rs

@@ -2426,12 +2426,14 @@ where
                 }
                 pandas_vet::rules::attr(self, attr, value, expr);
             }
-            Expr::Call(ast::ExprCall {
+            Expr::Call(
-                func,
+                call @ ast::ExprCall {
-                args,
+                    func,
-                keywords,
+                    args,
-                range: _,
+                    keywords,
-            }) => {
+                    range: _,
+                },
+            ) => {
                 if let Expr::Name(ast::ExprName { id, ctx, range: _ }) = func.as_ref() {
                     if id == "locals" && matches!(ctx, ExprContext::Load) {
                         let scope = self.semantic.scope_mut();
@@ -2597,6 +2599,9 @@ where
                 ]) {
                     flake8_bandit::rules::suspicious_function_call(self, expr);
                 }
+                if self.enabled(Rule::ReSubPositionalArgs) {
+                    flake8_bugbear::rules::re_sub_positional_args(self, call);
+                }
                 if self.enabled(Rule::UnreliableCallableCheck) {
                     flake8_bugbear::rules::unreliable_callable_check(self, expr, func, args);
                 }

crates/ruff/src/codes.rs

@@ -266,6 +266,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bugbear, "031") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ReuseOfGroupbyGenerator),
         (Flake8Bugbear, "032") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::UnintentionalTypeAnnotation),
         (Flake8Bugbear, "033") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::DuplicateValue),
+        (Flake8Bugbear, "034") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ReSubPositionalArgs),
         (Flake8Bugbear, "904") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
         (Flake8Bugbear, "905") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
 

crates/ruff/src/rules/flake8_bugbear/mod.rs

@@ -19,7 +19,6 @@ mod tests {
     #[test_case(Rule::AssertRaisesException, Path::new("B017.py"))]
     #[test_case(Rule::AssignmentToOsEnviron, Path::new("B003.py"))]
     #[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
-    #[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
     #[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"))]
     #[test_case(Rule::DuplicateTryBlockException, Path::new("B025.py"))]
     #[test_case(Rule::DuplicateValue, Path::new("B033.py"))]
@@ -35,7 +34,9 @@ mod tests {
     #[test_case(Rule::LoopVariableOverridesIterator, Path::new("B020.py"))]
     #[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
     #[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
+    #[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
     #[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"))]
+    #[test_case(Rule::ReSubPositionalArgs, Path::new("B034.py"))]
     #[test_case(Rule::RedundantTupleInExceptionHandler, Path::new("B013.py"))]
     #[test_case(Rule::ReuseOfGroupbyGenerator, Path::new("B031.py"))]
     #[test_case(Rule::SetAttrWithConstant, Path::new("B009_B010.py"))]

crates/ruff/src/rules/flake8_bugbear/rules/mod.rs

@@ -17,6 +17,7 @@ pub(crate) use mutable_argument_default::*;
 pub(crate) use no_explicit_stacklevel::*;
 pub(crate) use raise_literal::*;
 pub(crate) use raise_without_from_inside_except::*;
+pub(crate) use re_sub_positional_args::*;
 pub(crate) use redundant_tuple_in_exception_handler::*;
 pub(crate) use reuse_of_groupby_generator::*;
 pub(crate) use setattr_with_constant::*;
@@ -50,6 +51,7 @@ mod mutable_argument_default;
 mod no_explicit_stacklevel;
 mod raise_literal;
 mod raise_without_from_inside_except;
+mod re_sub_positional_args;
 mod redundant_tuple_in_exception_handler;
 mod reuse_of_groupby_generator;
 mod setattr_with_constant;

crates/ruff/src/rules/flake8_bugbear/rules/re_sub_positional_args.rs

@@ -0,0 +1,112 @@
+use std::fmt;
+
+use rustpython_parser::ast::{self, Ranged};
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for calls to `re.sub`, `re.subn`, and `re.split` that pass `count`,
+/// `maxsplit`, or `flags` as positional arguments.
+///
+/// ## Why is this bad?
+/// Passing `count`, `maxsplit`, or `flags` as positional arguments to
+/// `re.sub`, re.subn`, or `re.split` can lead to confusion, as most methods in
+/// the `re` module accepts `flags` as the third positional argument, while
+/// `re.sub`, `re.subn`, and `re.split` have different signatures.
+///
+/// Instead, pass `count`, `maxsplit`, and `flags` as keyword arguments.
+///
+/// ## Example
+/// ```python
+/// import re
+///
+/// re.split("pattern", "replacement", 1)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// import re
+///
+/// re.split("pattern", "replacement", maxsplit=1)
+/// ```
+///
+/// ## References
+/// - [Python documentation: `re.sub`](https://docs.python.org/3/library/re.html#re.sub)
+/// - [Python documentation: `re.subn`](https://docs.python.org/3/library/re.html#re.subn)
+/// - [Python documentation: `re.split`](https://docs.python.org/3/library/re.html#re.split)
+#[violation]
+pub struct ReSubPositionalArgs {
+    method: Method,
+}
+
+impl Violation for ReSubPositionalArgs {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let ReSubPositionalArgs { method } = self;
+        let param_name = method.param_name();
+        format!(
+            "`{method}` should pass `{param_name}` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions"
+        )
+    }
+}
+
+/// B034
+pub(crate) fn re_sub_positional_args(checker: &mut Checker, call: &ast::ExprCall) {
+    let Some(method) = checker
+        .semantic()
+        .resolve_call_path(&call.func)
+        .and_then(|call_path| match call_path.as_slice() {
+            ["re", "sub"] => Some(Method::Sub),
+            ["re", "subn"] => Some(Method::Subn),
+            ["re", "split"] => Some(Method::Split),
+            _ => None,
+        })
+    else {
+        return;
+    };
+
+    if call.args.len() > method.num_args() {
+        checker.diagnostics.push(Diagnostic::new(
+            ReSubPositionalArgs { method },
+            call.range(),
+        ));
+    }
+}
+
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+enum Method {
+    Sub,
+    Subn,
+    Split,
+}
+
+impl fmt::Display for Method {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::Sub => fmt.write_str("re.sub"),
+            Self::Subn => fmt.write_str("re.subn"),
+            Self::Split => fmt.write_str("re.split"),
+        }
+    }
+}
+
+impl Method {
+    const fn param_name(self) -> &'static str {
+        match self {
+            Self::Sub => "count",
+            Self::Subn => "count",
+            Self::Split => "maxsplit",
+        }
+    }
+
+    const fn num_args(self) -> usize {
+        match self {
+            Self::Sub => 3,
+            Self::Subn => 3,
+            Self::Split => 2,
+        }
+    }
+}

crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B034_B034.py.snap

@@ -0,0 +1,102 @@
+---
+source: crates/ruff/src/rules/flake8_bugbear/mod.rs
+---
+B034.py:5:1: B034 `re.sub` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+  |
+4 | # B034
+5 | re.sub("a", "b", "aaa", re.IGNORECASE)
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+6 | re.sub("a", "b", "aaa", 5)
+7 | re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+  |
+
+B034.py:6:1: B034 `re.sub` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+  |
+4 | # B034
+5 | re.sub("a", "b", "aaa", re.IGNORECASE)
+6 | re.sub("a", "b", "aaa", 5)
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+7 | re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+8 | re.subn("a", "b", "aaa", re.IGNORECASE)
+  |
+
+B034.py:7:1: B034 `re.sub` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+  |
+5 | re.sub("a", "b", "aaa", re.IGNORECASE)
+6 | re.sub("a", "b", "aaa", 5)
+7 | re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+8 | re.subn("a", "b", "aaa", re.IGNORECASE)
+9 | re.subn("a", "b", "aaa", 5)
+  |
+
+B034.py:8:1: B034 `re.subn` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+ 6 | re.sub("a", "b", "aaa", 5)
+ 7 | re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+ 8 | re.subn("a", "b", "aaa", re.IGNORECASE)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+ 9 | re.subn("a", "b", "aaa", 5)
+10 | re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+   |
+
+B034.py:9:1: B034 `re.subn` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+ 7 | re.sub("a", "b", "aaa", 5, re.IGNORECASE)
+ 8 | re.subn("a", "b", "aaa", re.IGNORECASE)
+ 9 | re.subn("a", "b", "aaa", 5)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+10 | re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+11 | re.split(" ", "a a a a", re.I)
+   |
+
+B034.py:10:1: B034 `re.subn` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+ 8 | re.subn("a", "b", "aaa", re.IGNORECASE)
+ 9 | re.subn("a", "b", "aaa", 5)
+10 | re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+11 | re.split(" ", "a a a a", re.I)
+12 | re.split(" ", "a a a a", 2)
+   |
+
+B034.py:11:1: B034 `re.split` should pass `maxsplit` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+ 9 | re.subn("a", "b", "aaa", 5)
+10 | re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+11 | re.split(" ", "a a a a", re.I)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+12 | re.split(" ", "a a a a", 2)
+13 | re.split(" ", "a a a a", 2, re.I)
+   |
+
+B034.py:12:1: B034 `re.split` should pass `maxsplit` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+10 | re.subn("a", "b", "aaa", 5, re.IGNORECASE)
+11 | re.split(" ", "a a a a", re.I)
+12 | re.split(" ", "a a a a", 2)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+13 | re.split(" ", "a a a a", 2, re.I)
+14 | sub("a", "b", "aaa", re.IGNORECASE)
+   |
+
+B034.py:13:1: B034 `re.split` should pass `maxsplit` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+11 | re.split(" ", "a a a a", re.I)
+12 | re.split(" ", "a a a a", 2)
+13 | re.split(" ", "a a a a", 2, re.I)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+14 | sub("a", "b", "aaa", re.IGNORECASE)
+   |
+
+B034.py:14:1: B034 `re.sub` should pass `count` and `flags` as keyword arguments to avoid confusion due to unintuitive argument positions
+   |
+12 | re.split(" ", "a a a a", 2)
+13 | re.split(" ", "a a a a", 2, re.I)
+14 | sub("a", "b", "aaa", re.IGNORECASE)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B034
+15 | 
+16 | # OK
+   |
+
+

crates/ruff/src/rules/pylint/rules/type_name_incorrect_variance.rs

@@ -66,14 +66,14 @@ impl Violation for TypeNameIncorrectVariance {
 /// PLC0105
 pub(crate) fn type_name_incorrect_variance(checker: &mut Checker, value: &Expr) {
     let Expr::Call(ast::ExprCall {
-                       func,
+        func,
-                       args,
+        args,
-                       keywords,
+        keywords,
-                       ..
+        ..
-                   }) = value
+    }) = value
-        else {
+    else {
-            return;
+        return;
-        };
+    };
 
     let Some(param_name) = type_param_name(args, keywords) else {
         return;
@@ -121,9 +121,9 @@ pub(crate) fn type_name_incorrect_variance(checker: &mut Checker, value: &Expr)
                 None
             }
         })
-        else {
+    else {
-            return;
+        return;
-        };
+    };
 
     let variance = variance(covariant, contravariant);
     let name_root = param_name

ruff.schema.json

@@ -1712,6 +1712,7 @@
         "B031",
         "B032",
         "B033",
+        "B034",
         "B9",
         "B90",
         "B904",