pycodestyle icon indicating copy to clipboard operation
pycodestyle copied to clipboard

Add checks for unconsistent returns

Open SylvainDe opened this issue 8 years ago • 22 comments

Added in PEP 8 : """ Be consistent in return statements. Either all return statements in a function should return an expression, or none of them should. If any return statement returns an expression, any return statements where no value is returned should explicitly state this as return None, and an explicit return statement should be present at the end of the function (if reachable). """

Checking for unconsistent returns corresponds to implementing a check based on ast tree. Given an AST tree, one can easily collect return statements and check whether they return an explicit value or not. Also, from the AST tree, one can try to check whether the end of the function is reachable.

Warning W740 is added for explicit inconsistent returns.

Warning W741 is added for implicit inconsistent returns : when a functions explicitly returns a value at some point but does not at the (reachable) end of the function. If the end of a function is not reachable but warning is triggered, one might simply add a "return None" or an "assert False" : as one said : "Explicit is better than implicit.".

Regarding the code : Implementation has been made as easy to understand as possible. The new check itself has been implemented in a new class UnconsistentReturns which uses yet another class FlowAnalysis which serves as an holder for various helper methods. Also, I took this chance to change a few details so that AST-tree checks fit more easily. This changes a few APIs. I don't know if anyone is relying on those.

Regarding the tests : Adding the first AST-tree based check leads to most of the (incorrect) test code to be parsed which leads to many SyntaxError either related to a single version of Python or to all of them. A A new symbol has been to be able to convey the fact that an error is expecting only for such or such version of Python. I've fixed all issues related to SyntaxError so that they are all passing all right. I hope I haven't changed what is actually tested.

SylvainDe avatar Aug 25 '15 19:08 SylvainDe