Skip to content

Add stronger typing to nodes.py #1783

Closed

Description

@rwbarton

We should have a subclass Expression of Node, make all the *Expr node types inherit from it, and make all the fields of Nodes that are intended to be expressions into Expressions. Similarly for Statement, and possibly Var and related types.

NodeVisitor should be parameterized on at least two types, namely the return type on expressions and the return type on other things (mostly statements). Node's accept method should be moved into Expression and Statement (etc.), returning different type parameters of the NodeVisitor. Then TypeChecker can become a NodeVisitor[Type, None], meaning that it returns a type on an expression but returns nothing on a statement.

Then TypeChecker's visit_*_stmt methods wouldn't all be returning None where a Type was expected. So this is pretty important for getting mypy to type check itself under strict optional checking, which was my original motivation, but I think this refactoring is a good idea anyways: more specific types for the fields of Node subtypes give more information about what values are actually legal, which is helpful when either producing or consuming those nodes.

I don't want to do this refactoring right now as it's fairly large, and the parser is one of the things heavily affected, and we currently have two of those. Better to wait until after the old parser is gone.

Activity

  1. JukkaL commented on Jul 1, 2016

    @JukkaL
    Collaborator

    This would be good. I've thought about a similar thing myself.

  2. added this to the milestone on Jul 7, 2016
  3. gvanrossum commented on Oct 31, 2016

    @gvanrossum
    Member

    Heh, can we close this now that Expression and Statement are actual classes? @elazarg @rwbarton

  4. elazarg commented on Nov 1, 2016

    @elazarg
    Contributor

    For me this is ongoing, but it is more subtle. So I'd say close.

    Some day we'll have patterns (lvalues) and stuff.

  5. rwbarton commented on Nov 1, 2016

    @rwbarton
    ContributorAuthor

    The second and third paragraphs aren't done yet AFAIK.

  6. elazarg commented on Nov 1, 2016

    @elazarg
    Contributor

    We need an ExpressionVisitor.

  7. removed this from the milestone on Mar 29, 2017
  8. elazarg commented on Apr 3, 2017

    @elazarg
    Contributor

    We have expression visitor now. Are there any additional action items here?

  9. gvanrossum commented on Apr 3, 2017

    @gvanrossum
    Member

    I'm happy to close issues that vaguely suggest refactorings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions