[ruff] Guard against use of default_factory as a keyword argument (RUF026)

crates/ruff_linter/resources/test/fixtures/ruff/RUF026.py

@@ -0,0 +1,70 @@
+# Violation cases: RUF026
+
+from collections import defaultdict
+
+
+def func():
+    defaultdict(default_factory=None)  # RUF026
+
+def func():
+    defaultdict(default_factory=int)  # RUF026
+
+def func():
+    defaultdict(default_factory=float)  # RUF026
+
+def func():
+    defaultdict(default_factory=dict)  # RUF026
+
+def func():
+    defaultdict(default_factory=list)  # RUF026
+
+def func():
+    defaultdict(default_factory=tuple)  # RUF026
+
+def func():
+    def foo():
+        pass
+    defaultdict(default_factory=foo)  # RUF026
+
+
+def func():
+    defaultdict(default_factory=lambda: 1)  # RUF026
+
+def func():
+    from collections import deque
+    defaultdict(default_factory=deque)  # RUF026
+
+def func():
+    class MyCallable:
+        def __call__(self):
+            pass
+    defaultdict(default_factory=MyCallable())  # RUF026
+
+
+# Non-violation cases: RUF026
+
+def func():
+    defaultdict(default_factory=1)  # OK
+
+def func():
+    defaultdict(default_factory="wdefwef")  # OK
+
+def func():
+    defaultdict(default_factory=[1,2,3])  # OK
+
+def func():
+    defaultdict()  # OK
+
+def func():
+    defaultdict(int)  # OK
+
+def func():
+    defaultdict(list)  # OK
+
+def func():
+    defaultdict(dict)  # OK
+
+def func():
+    def constant_factory(value):
+        return lambda: value
+    defaultdict(constant_factory('<missing>'))

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -983,6 +983,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::UnsortedDunderAll) {
                 ruff::rules::sort_dunder_all_extend_call(checker, call);
             }
+
+            if checker.enabled(Rule::DefaultFactoryKwarg) {
+                ruff::rules::default_factory_kwarg(checker, call);
+            }
         }
         Expr::Dict(dict) => {
             if checker.any_enabled(&[

crates/ruff_linter/src/codes.rs

@@ -931,6 +931,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "023") => (RuleGroup::Preview, rules::ruff::rules::UnsortedDunderSlots),
         (Ruff, "024") => (RuleGroup::Preview, rules::ruff::rules::MutableFromkeysValue),
         (Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable),
+        (Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
 

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

@@ -46,6 +46,7 @@ mod tests {
     #[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
     #[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.py"))]
     #[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("RUF025.py"))]
+    #[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/default_factory_kwarg.rs

@@ -0,0 +1,233 @@
+use ast::{ExprAttribute, ExprName, Keyword};
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::{any_over_expr, is_constant};
+use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall, ExprContext};
+use ruff_python_semantic::{BindingKind, SemanticModel};
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for incorrect usages of `default_factory` as a keyword argument when
+/// initializing a `defaultdict`.
+///
+/// ## Why is this bad?
+/// The `defaultdict` constructor accepts a callable as its first argument.
+/// For example, it's common to initialize a `defaultdict` with `int` or `list`
+/// via `defaultdict(int)` or `defaultdict(list)`, to create a dictionary that
+/// returns `0` or `[]` respectively when a key is missing.
+///
+/// The default factory _must_ be provided as a positional argument, as all
+/// keyword arguments to `defaultdict` are interpreted as initial entries in
+/// the dictionary. For example, `defaultdict(foo=1, bar=2)` will create a
+/// dictionary with `{"foo": 1, "bar": 2}` as its initial entries.
+///
+/// As such, `defaultdict(default_factory=list)` will create a dictionary with
+/// `{"default_factory": list}` as its initial entry, instead of a dictionary
+/// that returns `[]` when a key is missing. Specifying a `default_factory`
+/// keyword argument is almost always a mistake, and one that type checkers
+/// can't reliably detect.
+///
+/// ## Examples
+/// ```python
+/// defaultdict(default_factory=int)
+/// defaultdict(default_factory=list)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// defaultdict(int)
+/// defaultdict(list)
+/// ```
+#[violation]
+pub struct DefaultFactoryKwarg {
+    callable_id: Option<String>,
+}
+
+impl Violation for DefaultFactoryKwarg {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        if let Some(id) = self.callable_id.as_ref() {
+            format!(
+                "Prefer using `defaultdict({id})` instead of initializing with `default_factory` keyword argument"
+            )
+        } else {
+            format!(
+                "Prefer using `defaultdict(callable)` instead of initializing with `default_factory` keyword argument"
+            )
+        }
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        match self.callable_id {
+            Some(ref id) => Some(format!(
+                "Prefer using `defaultdict({id})` instead of initializing with `default_factory` keyword argument"
+            )),
+            None => Some(format!(
+                "Prefer using `defaultdict(callable)` instead of initializing with `default_factory` keyword argument"
+            )),
+        }
+    }
+}
+
+enum ValueKind<'a> {
+    Fixable(&'a str),
+    NonFixable,
+    NonCallable,
+}
+
+/// RUF026
+pub(crate) fn default_factory_kwarg(checker: &mut Checker, default_dict: &ast::ExprCall) {
+    let ExprCall {
+        func, arguments, ..
+    } = default_dict;
+
+    if !is_defaultdict(func.as_ref()) {
+        return;
+    }
+
+    if arguments.keywords.is_empty() {
+        return;
+    }
+
+    let Some(keyword) = find_default_factory_keyword(arguments) else {
+        return;
+    };
+
+    match determine_kw_value_kind(keyword, checker.semantic()) {
+        ValueKind::Fixable(id) => {
+            let mut diagnostic = Diagnostic::new(
+                DefaultFactoryKwarg {
+                    callable_id: Some(id.to_string()),
+                },
+                default_dict.range(),
+            );
+
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                checker
+                    .generator()
+                    .expr(&fix_defaultdict_with_default_factory_as_kw_arg(
+                        &keyword.value,
+                    )),
+                default_dict.range(),
+            )));
+
+            checker.diagnostics.push(diagnostic);
+        }
+        ValueKind::NonFixable => {
+            let diagnostic = Diagnostic::new(
+                DefaultFactoryKwarg { callable_id: None },
+                default_dict.range(),
+            );
+
+            checker.diagnostics.push(diagnostic);
+        }
+        ValueKind::NonCallable => {}
+    }
+}
+
+fn is_defaultdict(func: &Expr) -> bool {
+    let id = match func {
+        Expr::Name(ExprName {
+            id,
+            ctx: _,
+            range: _,
+        }) => id,
+        Expr::Attribute(ExprAttribute { attr, .. }) => attr.as_str(),
+        _ => {
+            return false;
+        }
+    };
+    id == "defaultdict"
+}
+
+fn find_default_factory_keyword(arguments: &Arguments) -> Option<&ast::Keyword> {
+    arguments.keywords.iter().find(|&keyword| {
+        keyword
+            .arg
+            .as_ref()
+            .is_some_and(|arg| arg.as_str() == "default_factory")
+    })
+}
+
+fn determine_kw_value_kind<'a>(keyword: &'a Keyword, semantic: &'a SemanticModel) -> ValueKind<'a> {
+    if let Some(call_path) = semantic.resolve_call_path(&keyword.value) {
+        if let Some(&member) = call_path.last() {
+            if semantic.is_builtin(member) {
+                return ValueKind::Fixable(member);
+            }
+        }
+    }
+
+    if matches!(keyword.value, Expr::NoneLiteral(_)) {
+        return ValueKind::Fixable("None");
+    }
+
+    if is_non_callable_value(&keyword.value) {
+        return ValueKind::NonCallable;
+    }
+
+    match &keyword.value {
+        Expr::Name(ExprName {
+            id,
+            ctx: _,
+            range: _,
+        }) => {
+            let Some(binding_id) = semantic.lookup_symbol(id) else {
+                return ValueKind::NonFixable;
+            };
+            if matches!(
+                semantic.binding(binding_id).kind,
+                BindingKind::FunctionDefinition(_)
+            ) {
+                ValueKind::Fixable(id)
+            } else {
+                ValueKind::NonFixable
+            }
+        }
+
+        _ => ValueKind::NonFixable,
+    }
+}
+
+fn is_non_callable_value(value: &Expr) -> bool {
+    if is_constant(value) {
+        return true;
+    }
+    any_over_expr(value, &|expr| {
+        matches!(expr, |Expr::List(_)| Expr::Dict(_)
+            | Expr::Set(_)
+            | Expr::Tuple(_)
+            | Expr::Slice(_)
+            | Expr::ListComp(_)
+            | Expr::SetComp(_)
+            | Expr::DictComp(_)
+            | Expr::GeneratorExp(_)
+            | Expr::FString(_))
+    })
+}
+
+/// Generate a [`Fix`] to replace `defaultdict(default_factory=callable)` with `defaultdict(callable)`.
+///
+/// For example:
+/// - Given `defaultdict(default_factory=list)`, generate `defaultdict(list)`.
+/// - Given `def foo(): pass` `defaultdict(default_factory=foo)`, generate `defaultdict(foo)`.
+fn fix_defaultdict_with_default_factory_as_kw_arg(value: &Expr) -> Expr {
+    let args = Arguments {
+        args: vec![value.clone()],
+        keywords: vec![],
+        range: TextRange::default(),
+    };
+    Expr::Call(ExprCall {
+        func: Box::new(Expr::Name(ExprName {
+            id: "defaultdict".into(),
+            ctx: ExprContext::Load,
+            range: TextRange::default(),
+        })),
+        arguments: args,
+        range: TextRange::default(),
+    })
+}

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

@@ -2,6 +2,7 @@ pub(crate) use ambiguous_unicode_character::*;
 pub(crate) use assignment_in_assert::*;
 pub(crate) use asyncio_dangling_task::*;
 pub(crate) use collection_literal_concatenation::*;
+pub(crate) use default_factory_kwarg::*;
 pub(crate) use explicit_f_string_type_conversion::*;
 pub(crate) use function_call_in_dataclass_default::*;
 pub(crate) use implicit_optional::*;
@@ -27,6 +28,7 @@ mod assignment_in_assert;
 mod asyncio_dangling_task;
 mod collection_literal_concatenation;
 mod confusables;
+mod default_factory_kwarg;
 mod explicit_f_string_type_conversion;
 mod function_call_in_dataclass_default;
 mod helpers;