C2901 & C2902: New checks for unnecessary lambda expression usage

[jpy-git]

Type of Changes

	Type
✓	✨ New feature

Description

Closes #5976.

Adds new check unnecessary-lambda-assignment: Lambda expression assigned to a variable. Define a function using the "def" keyword instead.

From PEP8:

Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier:
# Correct:
def f(x): return 2*x
# Wrong:
f = lambda x: 2*x

This is equivalent to E731 in flake8 but additionally addresses several false negatives that are missed by that implementation: PyCQA/pycodestyle#1061

Also adds a new check unnecessary-direct-lambda-call: Lambda expression called directly. Execute the expression inline instead.
i.e. a**2 not (lambda x: x**2)(a)
This prevents a weird redundant usage of lambda functions that I've seen some junior devs do before.

[jpy-git]

There were a lot of existing tests using that I had to add ignores to in that last commit, so look at the commit before that for easier reviewing

[coveralls]

Pull Request Test Coverage Report for Build 2269685819

• 27 of 27 (100.0%) changed or added relevant lines in 1 file are covered.
• 10 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.007%) to 95.185%

---

Files with Coverage Reduction	New Missed Lines	%
pylint/checkers/stdlib.py	10	94.4%

Totals
Change from base Build 2266524332:	0.007%
Covered Lines:	15855
Relevant Lines:	16657

---

💛 - Coveralls

[jacobtylerwalls]

This is hilariously unlikely to happen, but what do you think about attempting to catch this case? (It doesn't emit a warning even if you remove the disable):
https://github.com/PyCQA/pylint/blob/edf753e6864224ac0144c0ac12c8d89baa93e097/tests/functional/a/assignment/assignment_expression.py#L44

[jpy-git]

@jacobtylerwalls I've extended the check to flag assignment via named expressions as well 👍
Currently the tests for python<=3.7 are failing because walrus operators were only introduced in 3.8. Is there a way to only run some lines/files for certain versions?

[Pierre-Sassoulas]

Currently the tests for python<=3.7 are failing because walrus operators were only introduced in 3.8. Is there a way to only run some lines/files for certain versions?

Yes, look at a .rc files in tests/functional/ there should be examples of use for min_version

[jpy-git]

Thanks!

[jpy-git]

(sorry about the closing and reopening btw, some of the checks keep timing out for no reason (seems like a GHA thing rather than the code changes and closing/reopening is the easiest way to kick it off again)

[Pierre-Sassoulas]

seems like a GHA thing rather than the code changes and closing/reopening is the easiest way to kick it off again

Yes, the python 3.8 + coverage job regularly takes more than 15mn. It's more frequent than an actual failed tests right now.

[DanielNoord]

@Pierre-Sassoulas Not sure if this is an option, but as a collaborator I can re-run actions. Can we allow this for contributors as well? That might make this more workable.

[Pierre-Sassoulas]

Can we allow this for contributors as well?

I don't think so, but the trick to open/close the issue should do most of the time.

[jpy-git]

is there a reason why we don't increase the timeout-minutes settings in the yaml file?

[Pierre-Sassoulas]

From what I understand the pipeline that are stuck are stuck because the runner used by github has only one core and we try to use two (and we can't ask for multiple core so it's 'random'). So it will fail even if we wait one hour. 15mn permit to free the worker sooner and have overall better availability.

[Pierre-Sassoulas]

LGTM, but I think it would be a nice check to inaugurate the false positive github action primer when it's available. It's a default checker and it's going to be run in a lot of code once it lands in pylint.

[DanielNoord]

@Pierre-Sassoulas @jpy-git For some reason the latest merge of main broke this. Could either of you look into this?

After that, I think we can go ahead and merge this as we shouldn't let this be blocked by the eventual update to the primer imo.

[Pierre-Sassoulas]

Sure let me fix this, I should have merged that a long time ago. I think the issue is only with the checker number that has been claimed by another new checker since.

[Pierre-Sassoulas]

@DanielNoord pypy has column/line variation compared to cpython. This is something we do not control and we're probably never going to change the parsing to have the right column in both. Should we still take the column/line into account under PYPY ?

[DanielNoord]

@DanielNoord pypy has column/line variation compared to cpython. This is something we do not control and we're probably never going to change the parsing to have the right column in both. Should we still take the column/line into account under PYPY ?

Let's ignore this test on PyPy. This is a very rare (and only) case where the col_offset (and not end_col_offset) differ. Actually I think the column offset of PyPy is more "correct", but that's just my opinion 😄 . I don't think we should stop checking col_offset on PyPy all together as this is the only case where it differs.

[DanielNoord]

Thanks @jpy-git for contributing the new checker and thanks to all reviewers 🎉