-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
unnecessary-lambda false positive (expression variable reassigned) #8192
Comments
This is an interesting one. Actually it has nothing to with class attributes. Instead, the issue is when variables are looked up. I suppose Pylint is suggesting that f = lambda x: foo.get(x) # false positive unnecessary-lambda
# an object is created and given the name 'foo'
foo = {1: 2}
assert f(1) == 2
# a new object is created and given the name 'foo'; first object is lost
foo = {1: 3}
assert f(1) == 3
# the name 'foo' is deleted; second object is lost
del foo
assert f(1) == 3 # NameError: name 'foo' is not defined Changing the lambda expression to a full function definition does not change this behavior. The current |
The message for In this case, I wonder if it would be hard for the checker to suppress the error if any variable referenced in the expression is reassigned. |
This is my first PR and I'm new at rust, so feel free to ask me to rewrite everything if needed ;) The rule must be called after deferred lambas have been visited because of the last check (whether the lambda parameters are used in the body of the function that's being called). I didn't know where to do it, so I did what I could to be able to work on the rule itself: - added a `ruff_linter::checkers::ast::analyze::lambda` module - build a vec of visited lambdas in `visit_deferred_lambdas` - call `analyze::lambda` on the vec after they all have been visited Building that vec of visited lambdas was necessary so that bindings could be properly resolved in the case of nested lambdas. Note that there is an open issue in pylint for some false positives, do we need to fix that before merging the rule? pylint-dev/pylint#8192 Also, I did not provide any fixes (yet), maybe we want do avoid merging new rules without fixes? ## Summary Checks for lambdas whose body is a function call on the same arguments as the lambda itself. ### Bad ```python df.apply(lambda x: str(x)) ``` ### Good ```python df.apply(str) ``` ## Test Plan Added unit test and snapshot. Manually compared pylint and ruff output on pylint's test cases. ## References - [pylint documentation](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/unnecessary-lambda.html) - [pylint implementation](https://github.com/pylint-dev/pylint/blob/main/pylint/checkers/base/basic_checker.py#L521-L587) - #970
Bug description
pylint
unnecessary-lambda
doesn't seem to understand that reassignment of a variablecontaining a class instance affects bound methodscan necessitate use of a lambda:Configuration
No response
Command used
Pylint output
Expected behavior
no warning, since the variable holding the class instance (in this case, a dict) used in the lambda expression is reassigned locally
Pylint version
OS / Environment
No response
Additional dependencies
No response
The text was updated successfully, but these errors were encountered: