Skip to content

Regression used-before-assignment with try-except #5500

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

Closed
cdce8p opened this issue Dec 11, 2021 · 2 comments · Fixed by #5506
Closed

Regression used-before-assignment with try-except #5500

cdce8p opened this issue Dec 11, 2021 · 2 comments · Fixed by #5506
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check Regression

Comments

@cdce8p
Copy link
Member

cdce8p commented Dec 11, 2021

Bug description

#5402 added the used-before-assignment error in cases an assignment only happens in except blocks.
While testing, I came across the example below. It's a special case, but I believe it should still be possible to exclude it.

  • The try block should return
  • Each except block defines the variable
def func():
    try:
        value = 1 / 0
        return
    except ZeroDivisionError:
        data = 1
    except ValueError:
        data = 2
    print(data)  # used-before-assignment

/CC: @jacobtylerwalls

Configuration

No response

Command used

pylint test.py

Pylint output

E0601: Using variable 'data' before assignment (used-before-assignment)

Expected behavior

No error

Pylint version

Current main

OS / Environment

No response

Additional dependencies

No response

@cdce8p cdce8p added Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling C: used-before-assignment Issues related to 'used-before-assignment' check Regression and removed Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Dec 11, 2021
@jacobtylerwalls
Copy link
Member

Thanks for finding the case.

The try block should return

This part is simple. I'll push a PR.

Each except block defines the variable

I'm not certain we need to require this. What if one of the except blocks returns or raises? This might get quite tangled given the current implementation of the variables checker.

If we merge the PR I'm about to submit for this issue, thereby reducing false-positives, the false-negative where one except block doesn't happen to define a name could be evaluated for feasibility separately (given that we would need to check each handler for a return/raise).

jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Dec 11, 2021
…assignments in except blocks following try blocks that return
jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Dec 13, 2021
…assignments in except blocks following try blocks that return
@cdce8p
Copy link
Member Author

cdce8p commented Dec 13, 2021

Each except block defines the variable

I'm not certain we need to require this. What if one of the except blocks returns or raises? This might get quite tangled given the current implementation of the variables checker.

True.

If we merge the PR I'm about to submit for this issue, thereby reducing false-positives, the false-negative where one except block doesn't happen to define a name could be evaluated for feasibility separately (given that we would need to check each handler for a return/raise).

IMO preventing false-positives is much more important for us since those can really annoy people. false-negatives less so. I've create #5523 to add a few more test cases that should help when working on additional changes.
Furthermore I also create #5524 as a followup to track the false-negative case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check Regression
Projects
None yet
2 participants