Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stronger typing to nodes.py #1783

Closed
rwbarton opened this issue Jul 1, 2016 · 7 comments
Closed

Add stronger typing to nodes.py #1783

rwbarton opened this issue Jul 1, 2016 · 7 comments
Labels
priority-2-low refactoring Changing mypy's internals

Comments

@rwbarton
Copy link
Contributor

rwbarton commented Jul 1, 2016

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.

@rwbarton rwbarton added the refactoring Changing mypy's internals label Jul 1, 2016
@JukkaL
Copy link
Collaborator

JukkaL commented Jul 1, 2016

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

rwbarton added a commit to rwbarton/mypy that referenced this issue Jul 1, 2016
This doesn't actually do anything yet, as Statement and Expression are
just synonyms for Node. But it's part of the work that will need to be
done for python#1783, and for now it serves as (unchecked) documentation.
rwbarton added a commit to rwbarton/mypy that referenced this issue Jul 1, 2016
This doesn't actually do anything yet, as Statement and Expression are
just synonyms for Node. But it's part of the work that will need to be
done for python#1783, and for now it serves as (unchecked) documentation.
gvanrossum pushed a commit that referenced this issue Jul 1, 2016
This doesn't actually do anything yet, as Statement and Expression are
just synonyms for Node. But it's part of the work that will need to be
done for #1783, and for now it serves as (unchecked) documentation.
@gvanrossum gvanrossum added this to the Future milestone Jul 7, 2016
@gvanrossum
Copy link
Member

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

@elazarg
Copy link
Contributor

elazarg commented Nov 1, 2016

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

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

@rwbarton
Copy link
Contributor Author

rwbarton commented Nov 1, 2016

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

@elazarg
Copy link
Contributor

elazarg commented Nov 1, 2016

We need an ExpressionVisitor.

@gvanrossum gvanrossum removed this from the Future milestone Mar 29, 2017
@elazarg
Copy link
Contributor

elazarg commented Apr 3, 2017

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

@gvanrossum
Copy link
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
Labels
priority-2-low refactoring Changing mypy's internals
Projects
None yet
Development

No branches or pull requests

4 participants