PyCQA / PyCQA/pyflakes

Proposed changes to pyflakes's traversal code.

Open
#816 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.5k
Forks
190
Avg merge
8m
Merged PRs (30d)
13

Description

I have spent many happy hours studying pyflakes and its tree traversal methods. This issue proposes several possible simplifications. It's fine with me if you aren't interested.

Overview

All the simplifications follow from these changes:

  • Eliminate the omit and _fieldsOrder kwargs and the _FieldsOrder class.
  • Add/change visitors to specify required traversal order.
  • Rename the visitors so that the visitor's names are the same as the corresponding Ast nodes.

With these changes, Checker.getNodeHandler becomes:

handler = getattr(self, node.__class__.__name__, self.visit)
handler(node)

There is no need for the _nodeHandlers cache because the new code creates no strings.

Only Checker.handleNode calls getNodeHandler so the code above may as well appear in-line.

Other changes

  • Eliminate iter_child_nodes.
  • Add a handleFields method:
def handleFields(self, node, fields):
    """Visit only the *given* children of node in the given order."""
    for field in fields:
        child = getattr(node, field, None)
        if isinstance(child, list):
            for item in child:
                if isinstance(item, ast.AST):
                    self.handleNode(item, node)
        elif isinstance(child, ast.AST):
            self.handleNode(child, node)

def visit(self, node):
    self.visitFields(node._fields)  # Or duplicate the code above to eliminate the call.
  • Add or change visitors to specify the required traversal order. Examples:
def arguments(self, node):
    # Visit all fields except 'defaults' and 'kw_defaults'.
    fields = ('posonlyargs', 'args', 'var arg', 'kwonlyargs', 'kwarg')
    self.handleFields(node, fields)

def DictComp(self, node):
    with self.in_scope(GeneratorScope):
        # Order matters.
        self.handleFields(node, ('generators', 'key', 'value'))
  • Eliminate Checker._unknown_handler. A new unit test would cover its absence.

Summary

These changes are merely suggestions. You can find the prototype code in the five PRs at ekr-fork-pyflakes.
I'll happily make any changes you like in a real PR.

Eliminating kwargs invariably simplifies and generalizes code. For example, it would be straightforward to subclass the new traversal code. The new visitors increase the traversal's speed and make explicit the required traversal order of the visitor's children.

Edward

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the traversal path through Checker.handleNode and Checker.getNodeHandler, then review the proposed handleFields and visitor changes in the issue. Compare the existing traversal behavior with the referenced prototype PRs, especially ordering and _unknown_handler coverage. Done means the refactor preserves traversal behavior, removes the listed mechanisms, and has tests for the changed behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.