Discussion around a proper implementation of control-flow in pylint

[Pierre-Sassoulas]

Current problem

The main limitation regarding control flow is the fact that the variable checkers rely on NamesConsumer's to_consume/consumed states, and these are updated "all-or-nothing" per variable, with additional special-case checks here and there. To give a simpler example, pylint doesn't report any error on:

def function():
    x = 10
    del x
    print(x)

function()

Desired solution

I don't know the history of the Variables checker, but it seems to me like pylint will keep running into issues around control flow as long as this basic approach is used. I wouldn't say that this is a limitation of the AST visitor pattern though. I think improvements could be made by doing any of (in order of increasing code complexity and running time, but also correctness IMO):

• Modifying NamesConsumer so that variables can be processed more than once.
• Using astroid's variable lookup functionality, which handles control flow better. [1]
• Building a control flow graph and then running VariablesChecker checks on the graph rather than the AST itself.

But all of these would be really significant changes so I'm not exactly advocating for them, just thought I'd mention. Personally I'm still very happy using pylint with the current functionality!

[1] Example:

code = """
def function():
    x = 10
    del x
    print(x)
"""
import astroid
ast = astroid.parse(code)
n = [n for n in ast.nodes_of_class(astroid.Name) if n.name == "x"][0]
print(n.lookup('x'))  # Correctly doesn't return any AssignNames

Additional context

This issue is taken entirely from a comment by @david-yz-liu here and will serve as the reference for discussion regarding control-flow implementation.

[jacobtylerwalls]

Just noting for anyone following the discussion I've proposed baby steps in #5402 (and eventually #5384) following approach 1, above:

Modifying NamesConsumer so that variables can be processed more than once.