Question

Given a simple code like below:

def func(flag):
    if flag:
        seq = range(3)
    return seq[1]

pep8 and flake8 both think this piece of code is good. Why could this happen? seq is only defined under if clause.

Was it helpful?

Solution

I don't have any specific knowledge about the design and implementation of pep8, but detecting conditionally-undefined variables in general is a tricky business. For example, consider the following code:

def func(value):
    if value < 2:
        raise ValueError()
    for p in generate_primes():
        if value % p == 0:
            smallest_prime_factor = p
            break
    do_something_with(smallest_prime_factor)

Now, the programmer knows that as long as you pass in an integer, one of the tests will pass. The static analysis tool doesn't know that. So what is your tool going to do?

  • require an else clause on the loop (in which case you won't get full code coverage for your tests unless you inject a mock version of generate_primes that doesn't satisfy the contract of generate_primes)
  • require initialization with something like smallest_prime_factor = 1 before the loop (which misleadingly implies to a reader of the code that there's a case to deal with where the result isn't initialized in the loop).
  • disable the check just for this variable (https://github.com/cburroughs/pep8.py/issues/12)
  • don't diagnose conditionally-undefined variables (which means you'll miss some errors).

It looks to me as though pep8 has gone for the simple and conservative option: it doesn't diagnose something that might not be incorrect. The fix is not to rely solely on static analysis tools, and certainly not to rely solely on tools that are designed primarily to detect issues of style rather than of code correctness. Write tests with full code coverage -- then for your code the test where flag is false will throw, diagnosing your error.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top