[pylint] Implement unnecessary-lambda (W0108)

crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_lambda.py

@@ -0,0 +1,55 @@
+_ = lambda: print()  # [unnecessary-lambda]
+_ = lambda x, y: min(x, y) # [unnecessary-lambda]
+
+_ = lambda *args: f(*args) # [unnecessary-lambda]
+_ = lambda **kwargs: f(**kwargs) # [unnecessary-lambda]
+_ = lambda *args, **kwargs: f(*args, **kwargs) # [unnecessary-lambda]
+_ = lambda x, y, z, *args, **kwargs: f(x, y, z, *args, **kwargs) # [unnecessary-lambda]
+
+_ = lambda x: f(lambda x: x)(x) # [unnecessary-lambda]
+_ = lambda x, y: f(lambda x, y: x + y)(x, y) # [unnecessary-lambda]
+
+# default value in lambda parameters
+_ = lambda x=42: print(x)
+
+# lambda body is not a call
+_ = lambda x: x
+
+# ignore chained calls
+_ = lambda: chained().call()
+
+# lambda has kwargs but not the call
+_ = lambda **kwargs: f()
+
+# lambda has kwargs and the call has kwargs named differently
+_ = lambda **kwargs: f(**dict([('forty-two', 42)]))
+
+# lambda has kwargs and the call has unnamed kwargs
+_ = lambda **kwargs: f(**{1: 2})
+
+# lambda has starred parameters but not the call
+_ = lambda *args: f()
+
+# lambda has starred parameters and the call has starargs named differently
+_ = lambda *args: f(*list([3, 4]))
+# lambda has starred parameters and the call has unnamed starargs
+_ = lambda *args: f(*[3, 4])
+
+# lambda has parameters but not the call
+_ = lambda x: f()
+_ = lambda *x: f()
+_ = lambda **x: f()
+
+# lambda parameters and call args are not the same length
+_ = lambda x, y: f(x)
+
+# lambda parameters and call args are not in the same order
+_ = lambda x, y: f(y, x)
+
+# lambda parameters and call args are not the same
+_ = lambda x: f(y)
+
+# the call uses the lambda parameters
+_ = lambda x: x(x)
+_ = lambda x, y: x(x, y)
+_ = lambda x: z(lambda y: x + y)(x)

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

@@ -0,0 +1,21 @@
+use crate::checkers::ast::Checker;
+use crate::codes::Rule;
+use crate::rules::pylint;
+use ruff_python_ast::{self as ast, Expr};
+
+pub(crate) fn lambda(expr: &Expr, checker: &mut Checker) {
+    match expr {
+        Expr::Lambda(
+            lambda @ ast::ExprLambda {
+                parameters: _,
+                body: _,
+                range: _,
+            },
+        ) => {
+            if checker.enabled(Rule::UnnecessaryLambda) {
+                pylint::rules::unnecessary_lambda(checker, lambda);
+            }
+        }
+        _ => unreachable!("Expected Expr::Lambda"),
+    }
+}

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

@@ -5,6 +5,7 @@ pub(super) use deferred_scopes::deferred_scopes;
 pub(super) use definitions::definitions;
 pub(super) use except_handler::except_handler;
 pub(super) use expression::expression;
+pub(super) use lambda::lambda;
 pub(super) use module::module;
 pub(super) use parameter::parameter;
 pub(super) use parameters::parameters;
@@ -19,6 +20,7 @@ mod deferred_scopes;
 mod definitions;
 mod except_handler;
 mod expression;
+mod lambda;
 mod module;
 mod parameter;
 mod parameters;

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

@@ -52,8 +52,8 @@ use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind};
 use ruff_python_semantic::analyze::{typing, visibility};
 use ruff_python_semantic::{
     BindingFlags, BindingId, BindingKind, Exceptions, Export, FromImport, Globals, Import, Module,
-    ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, StarImport,
-    SubmoduleImport,
+    ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, Snapshot,
+    StarImport, SubmoduleImport,
 };
 use ruff_python_stdlib::builtins::{BUILTINS, MAGIC_GLOBALS};
 use ruff_source_file::Locator;
@@ -1844,6 +1844,7 @@ impl<'a> Checker<'a> {
 
     fn visit_deferred_lambdas(&mut self) {
         let snapshot = self.semantic.snapshot();
+        let mut visited_lambdas: Vec<(&Expr, Snapshot)> = vec![];
         while !self.deferred.lambdas.is_empty() {
             let lambdas = std::mem::take(&mut self.deferred.lambdas);
             for (expr, snapshot) in lambdas {
@@ -1859,11 +1860,21 @@ impl<'a> Checker<'a> {
                         self.visit_parameters(parameters);
                     }
                     self.visit_expr(body);
+
+                    visited_lambdas.push((expr, snapshot));
                 } else {
                     unreachable!("Expected Expr::Lambda");
                 }
             }
         }
+
+        // all deferred lambdas must be visited before we can analyze them, because they might
+        // be nested
+        for (expr, snapshot) in visited_lambdas {
+            self.semantic.restore(snapshot);
+            analyze::lambda(expr, self);
+        }
+
         self.semantic.restore(snapshot);
     }
 
@@ -1980,8 +1991,8 @@ pub(crate) fn check_ast(
     checker.visit_deferred_type_param_definitions();
     let allocator = typed_arena::Arena::new();
     checker.visit_deferred_string_type_definitions(&allocator);
-    checker.visit_exports();
 
+    checker.visit_exports();
     // Check docstrings, bindings, and unresolved references.
     analyze::deferred_for_loops(&mut checker);
     analyze::definitions(&mut checker);

crates/ruff_linter/src/codes.rs

@@ -254,6 +254,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R5501") => (RuleGroup::Unspecified, rules::pylint::rules::CollapsibleElseIf),
         #[allow(deprecated)]
         (Pylint, "R6301") => (RuleGroup::Nursery, rules::pylint::rules::NoSelfUse),
+        (Pylint, "W0108") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryLambda),
         (Pylint, "W0120") => (RuleGroup::Unspecified, rules::pylint::rules::UselessElseOnLoop),
         (Pylint, "W0127") => (RuleGroup::Unspecified, rules::pylint::rules::SelfAssigningVariable),
         (Pylint, "W0129") => (RuleGroup::Unspecified, rules::pylint::rules::AssertOnStringLiteral),

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

@@ -134,6 +134,7 @@ mod tests {
     )]
     #[test_case(Rule::BadDunderMethodName, Path::new("bad_dunder_method_name.py"))]
     #[test_case(Rule::NoSelfUse, Path::new("no_self_use.py"))]
+    #[test_case(Rule::UnnecessaryLambda, Path::new("unnecessary_lambda.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/pylint/rules/mod.rs

@@ -51,6 +51,7 @@ pub(crate) use type_name_incorrect_variance::*;
 pub(crate) use type_param_name_mismatch::*;
 pub(crate) use unexpected_special_method_signature::*;
 pub(crate) use unnecessary_direct_lambda_call::*;
+pub(crate) use unnecessary_lambda::*;
 pub(crate) use useless_else_on_loop::*;
 pub(crate) use useless_import_alias::*;
 pub(crate) use useless_return::*;
@@ -110,6 +111,7 @@ mod type_name_incorrect_variance;
 mod type_param_name_mismatch;
 mod unexpected_special_method_signature;
 mod unnecessary_direct_lambda_call;
+mod unnecessary_lambda;
 mod useless_else_on_loop;
 mod useless_import_alias;
 mod useless_return;

crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs

@@ -0,0 +1,188 @@
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::visitor::Visitor;
+use ruff_python_ast::{
+    self as ast, visitor, Arguments, Expr, ExprLambda, Parameter, ParameterWithDefault,
+};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for lambdas whose body is a function call on the same arguments as the lambda itself.
+///
+/// ## Why is this bad?
+/// Such lambda expressions are in all but a few cases replaceable with the function being called
+/// in the body of the lambda.
+///
+/// ## Example
+/// ```python
+/// df.apply(lambda x: str(x))
+/// ```
+///
+/// Use instead:
+/// ```python
+/// df.apply(str)
+/// ```
+
+#[violation]
+pub struct UnnecessaryLambda;
+
+impl Violation for UnnecessaryLambda {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Lambda may not be necessary")
+    }
+}
+
+/// Identify all `Expr::Name` nodes in an AST.
+struct NameFinder<'a> {
+    /// A map from identifier to defining expression.
+    names: Vec<&'a ast::ExprName>,
+}
+
+impl<'a, 'b> Visitor<'b> for NameFinder<'a>
+where
+    'b: 'a,
+{
+    fn visit_expr(&mut self, expr: &'a Expr) {
+        if let Expr::Name(expr_name) = expr {
+            self.names.push(expr_name);
+        }
+        visitor::walk_expr(self, expr);
+    }
+}
+
+/// PLW0108
+pub(crate) fn unnecessary_lambda(checker: &mut Checker, lambda: &ExprLambda) {
+    let ExprLambda {
+        parameters,
+        body,
+        range: _,
+    } = lambda;
+    // At least one the parameters of the lambda include a default value. We can't know if the
+    // defaults provided by the lambda are the same as the defaults provided by the function
+    // being called.
+    if parameters.as_ref().map_or(false, |parameters| {
+        parameters
+            .args
+            .iter()
+            .any(|ParameterWithDefault { default, .. }| default.is_some())
+    }) {
+        return;
+    }
+    if let Expr::Call(ast::ExprCall {
+        arguments, func, ..
+    }) = body.as_ref()
+    {
+        let Arguments { args, keywords, .. } = arguments;
+
+        // don't check chained calls
+        if let Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() {
+            if let Expr::Call(_) = value.as_ref() {
+                return;
+            }
+        }
+
+        let call_starargs: Vec<&Expr> = args
+            .iter()
+            .filter_map(|arg| {
+                if let Expr::Starred(ast::ExprStarred { value, .. }) = arg {
+                    Some(value.as_ref())
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<_>>();
+
+        let call_kwargs: Vec<&Expr> = keywords.iter().map(|kw| &kw.value).collect::<Vec<_>>();
+
+        let call_ordinary_args: Vec<&Expr> = args
+            .iter()
+            .filter(|arg| !matches!(arg, Expr::Starred(_)))
+            .collect::<Vec<_>>();
+
+        if let Some(parameters) = parameters.as_ref() {
+            if let Some(kwarg) = parameters.kwarg.as_ref() {
+                if call_kwargs.is_empty()
+                    || call_kwargs.iter().any(|kw| {
+                        if let Expr::Name(ast::ExprName { id, .. }) = kw {
+                            id.as_str() != kwarg.name.as_str()
+                        } else {
+                            true
+                        }
+                    })
+                {
+                    return;
+                }
+            } else if !call_kwargs.is_empty() {
+                return;
+            }
+            if let Some(vararg) = parameters.vararg.as_ref() {
+                if call_starargs.is_empty()
+                    || call_starargs.iter().any(|arg| {
+                        if let Expr::Name(ast::ExprName { id, .. }) = arg {
+                            id.as_str() != vararg.name.as_str()
+                        } else {
+                            true
+                        }
+                    })
+                {
+                    return;
+                }
+            } else if !call_starargs.is_empty() {
+                return;
+            }
+
+            let lambda_ordinary_params: Vec<&Parameter> = parameters
+                .args
+                .iter()
+                .map(|ParameterWithDefault { parameter, .. }| parameter)
+                .collect::<Vec<_>>();
+
+            if call_ordinary_args.len() != lambda_ordinary_params.len() {
+                return;
+            }
+
+            let params_args = lambda_ordinary_params
+                .iter()
+                .zip(call_ordinary_args.iter())
+                .collect::<Vec<_>>();
+
+            for (param, arg) in params_args {
+                if let Expr::Name(ast::ExprName { id, .. }) = arg {
+                    if id.as_str() != param.name.as_str() {
+                        return;
+                    }
+                } else {
+                    return;
+                }
+            }
+        } else if !call_starargs.is_empty()
+            || !keywords.is_empty()
+            || !call_ordinary_args.is_empty()
+        {
+            return;
+        }
+
+        //  The lambda is necessary if it uses its parameter in the function it is
+        //  calling in the lambda's body
+        //  e.g. lambda foo: (func1 if foo else func2)(foo)
+
+        let mut finder = NameFinder { names: vec![] };
+        finder.visit_expr(func);
+
+        for expr_name in finder.names {
+            if let Some(binding_id) = checker.semantic().resolve_name(expr_name) {
+                let binding = checker.semantic().binding(binding_id);
+                if checker.semantic().is_current_scope(binding.scope) {
+                    return;
+                }
+            }
+        }
+
+        checker
+            .diagnostics
+            .push(Diagnostic::new(UnnecessaryLambda, lambda.range()));
+    }
+}