nedbat / coveragepy

The code coverage tool for Python
https://coverage.readthedocs.io
Apache License 2.0
3.01k stars 432 forks source link

Code after return not reported as not covered #1059

Open mschoettle opened 3 years ago

mschoettle commented 3 years ago

Describe the bug I accidentally had code after a return statement that would obviously never execute. However, coverage never reported these lines as uncovered.

To Reproduce

example.py

def main(arg):
    if arg == 'bar':
        return 'foo'

        print('covered?')
    else:
        pass

main('bar')
main('foobar')

command

coverage run --branch example.py && coverage report

Expected behavior coverage returns the line(s) after return as uncovered.

Additional context Without the if else block it reports the line correctly as uncovered.

nedbat commented 3 years ago

Coverage understands that the line is impossible to run (it has no green edge to it):

image

Coverage is trying to tell you about gaps in your test suite. There's nothing you can do in your test suite to execute that line. This is a job for a linter.

mschoettle commented 3 years ago

I agree that a linter should report that line. If I remove the if else block the line is reported as missing though:

Screen Shot 2020-11-26 at 20 46 56
Zeckie commented 1 year ago

I also agree that the job of finding unreachable lines is not really the responsibility of coverage. However, different tools determine reachability in different ways, for example mypy can use type information to determine that both of the print statements in the following code are unreachable:

def main(arg: str) -> str:
    if isinstance(arg, str):
        return 'foo'

        print('covered?')  # unreachable as after `return`
    else:
        print('covered?')  # unreachable as `arg` is always a `str`
        pass

main('bar')
main('foobar')

I think that it would be useful to show which lines that coverage has determined are unreachable, to make it clear why coverage isn't reporting a line that appears to be executable as run or missing. This could be done by adding another category (unreachable), and treat it in a similar way to how excluded is treated.

jonkiparsky commented 1 year ago

@nedbat if you like the idea of adding an "unreachable" category, then this issue might turn out to be suitable for the sprint, depending on what's going on in the guts of the code at this point (I haven't looked)

If you don't like that idea, then this might be a won't-fix issue.