no warning for possibly undefined scope

Bug #1497348 reported by jonathan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Pyflakes
Won't Fix
Undecided
Unassigned

Bug Description

Apologies if this is known -- I didn't see a bug or a skipped test for it.

This should generate a warning but does not:

    a = None
    if True or False:
        b = None
    ab = a + b

b is undefined when the `if` fails, and the code is invalid Python.

    a = None
    if False:
        b = None
    ab = a + b

    Traceback (most recent call last):
      File "/Users/jvanasco/Desktop/scoping.py", line 4, in <module>
        ab = a + b
    NameError: name 'b' is not defined

However pyflakes does not catch this error at all.

Ian Cordasco (icordasc)
Changed in pyflakes:
status: New → Confirmed
Revision history for this message
Phil Frost (bitglue) wrote :

What about:

i = randint(0, 10)
if is_even(i):
    b = "foo"
if is_odd(i):
    b = "bar"
print b

This works all the time, but is there a way for Pyflakes to know that?

Revision history for this message
jonathan (jvanasco) wrote :

I don't think Pyflakes (or developers) need to know if something would necessarily trigger an error -- just that it could.

Other linters/debug tools I've used (none python) triggered a warning in similar situations (not a confirmed error) that a variable could potentially be undefined when executed. With that warning noted, one might edit the (bitglue) example to read-

 i = randint(0, 10)
 if is_even(i):
  b = "foo"
 else:
  b = "bar"
 print b

or even...

 i = randint(0, 10)
 b = None
 if is_even(i):
  b = "foo"
 if is_odd:
  b = "bar"
 print b

In any event, in both the original(jonathan) and (bitglue) examples, a variable is declared within a conditional block BUT unconditionally used outside of it. While this isn't a guaranteed error, it's a potential source -- which is why we use tools like pyflakes [right?].

Revision history for this message
asmeurer (asmeurer) wrote :

One of pyflakes' goals is to avoid false positives (see https://github.com/pyflakes/pyflakes#design-principles). I personally use pyflakes in some test suites so that tests don't pass unless pyflakes gives no errors. Even the few false positives that pyflakes does give (usually surrounding compatibility imports) are irksome.

Perhaps a new tool (possibly built on top of pyflakes) that checks for these more advanced sorts of errors and doesn't mind giving false positives would be in order. They're not really style issues, so I don't think pep8 is a good place for them (they also aren't part of PEP 8).

And I'm pretty sure this has all been discussed extensively in an another issue here.

Revision history for this message
Phil Frost (bitglue) wrote :

> While this isn't a guaranteed error, it's a potential
> source -- which is why we use tools like pyflakes [right?].

I think maybe not. Pyflakes has a few design principles[1] which set it apart from similar tools like pylint. One of them is that Pyflakes avoids false positives even if that means admitting the possibility a false negative. This gives Pyflakes a couple nice properties:

Firstly, you can run Pyflakes against any code, even really bad code, and be confident that 99.9% of what it tells you are actual problems: code which is either unnecessary, never executed, or a bug.

Another is that there's never any need to frob a bunch of configuration options to match personal style preferences (though people who like to follow PEP8 use pyflakes as part of flake8).

Personally, I lean very heavily on unit testing to catch errors in my programs. Pyflakes catches some things a unit test won't catch, like an unused variable or import. Since the unit tests catch most of the errors I really like that I don't have to fight with my linter to get it to give me relevant information.

So it sounds like we have here is just a difference in expectations so I'm going to close the ticket. But thanks for reporting the bug anyway, it's always good to get feedback. I'd encourage you to check out pylint: I don't know if it does precisely what you want, but you might find in general it does more what you want.

[1] https://github.com/pyflakes/pyflakes#design-principles

Changed in pyflakes:
status: Confirmed → Won't Fix
Revision history for this message
jonathan (jvanasco) wrote :

Respectfully: there is *nothing* in the documentation that explicitly states or otherwise suggests the extent to which false-positives are avoided includes allowing false-negatives. "Trying very, very hard to avoid false- positives" is decidedly not the same as knowingly not testing for scenarios that could produce false positives.

It is your decision to embrace that design decision - and I'm not going to question that. I would, however, like to point out that there is a moral responsibility to explicitly and adequately note that decision. A lot of people have placed trust in pyflakes directly (or via flake8), as part of their code auditing and build/deploy routines; there honestly should be a list of potential bugs that this library has decided not to test against [because of the potential for false-negatives] so that end-users can decide how to proceed.

FYI- We rely heavily on unit testing as-well-as integrated testing. All of our custom tests passed; pyflakes (via flake8) showed no errors on any checkins or the build/deploy log... but one particular edge case kept popping up in the production logs. It took several hours of debugging to determine what happened was a variant of the above-reported issue.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.