Skip to content

Fix #5112: Prevent used-before-assignment if named expression found first in container #5812

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

Merged

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Feb 16, 2022

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Prevent a false positive for used-before-assignment when a named expression (walrus operator) is found first in a container.

For instance, this passed:

assert (x := True)

But this didn't (it's a tuple):

assert (x := True), x

Notice this should not pass:

assert x, (x := True)

Closes #5112

Of course, this can wait for 2.14, no problem.

@coveralls
Copy link

coveralls commented Feb 16, 2022

Pull Request Test Coverage Report for Build 1860771448

  • 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
  • 26 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.009%) to 93.982%

Files with Coverage Reduction New Missed Lines %
pylint/lint/parallel.py 2 92.86%
pylint/checkers/variables.py 24 96.44%
Totals Coverage Status
Change from base Build 1840800912: -0.009%
Covered Lines: 14929
Relevant Lines: 15885

πŸ’› - Coveralls

@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Feb 16, 2022
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about where to put this. On one hand this is a bug fix, it feels wrong not to put it in if the work is done. On the other hand we have a LOT of thing in 2.13 already and we need to focus on actually releasing at some point. We can probably put this in 2.13 as I took the time to review now.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's been a new issue opened regarding this problem, I think we could add this regression test, what do you think ?

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.


def expression_in_ternary_operator_inside_container_wrong_position():
"""2-element list where named expression comes too late"""
return [val3, val3 if (val3 := 'something') else 'anything'] # [used-before-assignment]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pff... Man that's convoluted. Can't we raise an don't-write-code-like-this warning here? πŸ˜…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I had to take 2-3mn and run the code to understand myself πŸ˜„ But do we really want to make the kind of person that would write this, flock to our bug tracker to complain πŸ˜‰ ?

@Pierre-Sassoulas Pierre-Sassoulas merged commit 3253f4b into pylint-dev:main Feb 17, 2022
@jacobtylerwalls jacobtylerwalls deleted the named-expr-first-in-container branch February 17, 2022 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid used-before-assignment
4 participants