Repository: serge-sans-paille/gast

Issue:
gast doesn't seem to parse assignment expressions correctly
It looks like `gast` doesn't correctly parse [assignment expressions](https://docs.python.org/3/whatsnew/3.8.html#assignment-expressions) (the `:=` operator):

`ast`:

```
In [1]: import ast

In [2]: expr = "(x := 1)"

In [3]: ast.dump(ast.parse(expr))
Out[3]: "Module(body=[Expr(value=NamedExpr(target=Name(id='x', ctx=Store()), value=Constant(value=1, kind=None)))], type_ignores=[])"
```

`gast`:

```
In [4]: import gast

In [5]: expr = "(x := 1)"

In [6]: gast.dump(gast.parse(expr))
Out[6]: 'Module(body=[Expr(value=None)], type_ignores=[])'
```

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index d26f2d7..0fead9d 100644
--- a/README.rst
+++ b/README.rst
@@ -186,6 +186,7 @@ ASDL
 
               -- BoolOp() can use left & right?
         expr = BoolOp(boolop op, expr* values)
+             | NamedExpr(expr target, expr value)
              | BinOp(expr left, operator op, expr right)
              | UnaryOp(unaryop op, expr operand)
              | Lambda(arguments args, expr body)
diff --git a/gast/ast3.py b/gast/ast3.py
index 6febbe6..a1e2436 100644
--- a/gast/ast3.py
+++ b/gast/ast3.py
@@ -20,7 +20,8 @@ class Ast3ToGAst(AstToGAst):
 
         def visit_ExtSlice(self, node):
             new_node = gast.Tuple(self._visit(node.dims), gast.Load())
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_Index(self, node):
             return self._visit(node.value)
@@ -49,7 +50,8 @@ class Ast3ToGAst(AstToGAst):
                 node.n,
                 None,
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_Ellipsis(self, node):
             new_node = gast.Constant(
@@ -65,14 +67,16 @@ class Ast3ToGAst(AstToGAst):
                 node.s,
                 None,
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_Bytes(self, node):
             new_node = gast.Constant(
                 node.s,
                 None,
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_FunctionDef(self, node):
             new_node = gast.FunctionDef(
@@ -83,7 +87,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.returns),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncFunctionDef(self, node):
             new_node = gast.AsyncFunctionDef(
@@ -94,7 +99,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.returns),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_For(self, node):
             new_node = gast.For(
@@ -104,7 +110,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.orelse),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncFor(self, node):
             new_node = gast.AsyncFor(
@@ -114,7 +121,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.orelse),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_With(self, node):
             new_node = gast.With(
@@ -122,7 +130,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.body),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncWith(self, node):
             new_node = gast.AsyncWith(
@@ -130,7 +139,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.body),
                 None,  # type_comment
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_Call(self, node):
             if sys.version_info.minor < 5:
@@ -156,7 +166,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.args) + starred,
                 self._visit(node.keywords) + kwargs,
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_NameConstant(self, node):
             if node.value is None:
@@ -165,7 +176,8 @@ class Ast3ToGAst(AstToGAst):
                 new_node = gast.Constant(True, None)
             elif node.value is False:
                 new_node = gast.Constant(False, None)
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
         def visit_arguments(self, node):
             new_node = gast.arguments(
@@ -177,30 +189,33 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.kwarg),
                 self._visit(node.defaults),
             )
-            return gast.copy_location(new_node, node)
+            gast.copy_location(new_node, node)
+            return new_node
 
     def visit_Name(self, node):
         new_node = gast.Name(
-            node.id,  # micro-optimization here, don't call self._visit
+            self._visit(node.id),
             self._visit(node.ctx),
             None,
             None,
         )
-        return ast.copy_location(new_node, node)
+        ast.copy_location(new_node, node)
+        return new_node
 
     def visit_arg(self, node):
         if sys.version_info.minor < 8:
-            extra_arg = None
+            extra_args = [None]
         else:
-            extra_arg = self._visit(node.type_comment)
+            extra_args = [self._visit(node.type_comment)]
 
         new_node = gast.Name(
-            node.arg,  # micro-optimization here, don't call self._visit
+            self._visit(node.arg),
             gast.Param(),
             self._visit(node.annotation),
-            extra_arg  # type_comment
+            *extra_args  # type_comment
         )
-        return ast.copy_location(new_node, node)
+        ast.copy_location(new_node, node)
+        return new_node
 
     def visit_ExceptHandler(self, node):
         if node.name:
@@ -208,7 +223,8 @@ class Ast3ToGAst(AstToGAst):
                 self._visit(node.type),
                 gast.Name(node.name, gast.Store(), None, None),
                 self._visit(node.body))
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
         else:
             return self.generic_visit(node)
 
@@ -258,7 +274,8 @@ class GAstToAst3(GAstToAst):
                 new_slice,
                 self._visit(node.ctx),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
     def visit_Assign(self, node):
         new_node = ast.Assign(
@@ -266,7 +283,8 @@ class GAstToAst3(GAstToAst):
             self._visit(node.value),
         )
 
-        return ast.copy_location(new_node, node)
+        ast.copy_location(new_node, node)
+        return new_node
 
     if sys.version_info.minor < 8:
 
@@ -287,7 +305,8 @@ class GAstToAst3(GAstToAst):
                 new_node = ast.Str(node.value)
             else:
                 new_node = ast.Bytes(node.value)
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
     def _make_arg(self, node):
         if node is None:
@@ -310,7 +329,8 @@ class GAstToAst3(GAstToAst):
             self._visit(node.id),
             self._visit(node.ctx),
         )
-        return ast.copy_location(new_node, node)
+        ast.copy_location(new_node, node)
+        return new_node
 
     def visit_ExceptHandler(self, node):
         if node.name:
@@ -346,7 +366,8 @@ class GAstToAst3(GAstToAst):
                 self._visit(starargs),
                 self._visit(kwargs),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_ClassDef(self, node):
             self.generic_visit(node)
@@ -371,7 +392,8 @@ class GAstToAst3(GAstToAst):
                 self._visit(node.decorator_list),
                 self._visit(node.returns),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncFunctionDef(self, node):
             new_node = ast.AsyncFunctionDef(
@@ -381,7 +403,8 @@ class GAstToAst3(GAstToAst):
                 self._visit(node.decorator_list),
                 self._visit(node.returns),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_For(self, node):
             new_node = ast.For(
@@ -390,7 +413,8 @@ class GAstToAst3(GAstToAst):
                 self._visit(node.body),
                 self._visit(node.orelse),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncFor(self, node):
             new_node = ast.AsyncFor(
@@ -400,21 +424,24 @@ class GAstToAst3(GAstToAst):
                 self._visit(node.orelse),
                 None,  # type_comment
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_With(self, node):
             new_node = ast.With(
                 self._visit(node.items),
                 self._visit(node.body),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_AsyncWith(self, node):
             new_node = ast.AsyncWith(
                 self._visit(node.items),
                 self._visit(node.body),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
         def visit_Call(self, node):
             new_node = ast.Call(
@@ -422,7 +449,8 @@ class GAstToAst3(GAstToAst):
                 self._visit(node.args),
                 self._visit(node.keywords),
             )
-            return ast.copy_location(new_node, node)
+            ast.copy_location(new_node, node)
+            return new_node
 
     def visit_arguments(self, node):
         extra_args = [self._make_arg(node.vararg),
diff --git a/gast/astn.py b/gast/astn.py
index d04775a..4406c4f 100644
--- a/gast/astn.py
+++ b/gast/astn.py
@@ -7,29 +7,24 @@ def _generate_translators(to):
     class Translator(ast.NodeTransformer):
 
         def _visit(self, node):
-            if isinstance(node, ast.AST):
-                return self.visit(node)
-            elif isinstance(node, list):
+            if isinstance(node, list):
                 return [self._visit(n) for n in node]
+            elif isinstance(node, ast.AST):
+                return self.visit(node)
             else:
                 return node
 
         def generic_visit(self, node):
             cls = type(node).__name__
-            try:
-                new_node = getattr(to, cls)()
-            except AttributeError:
-                # handle nodes that are not part of the AST
+            # handle nodes that are not part of the AST
+            if not hasattr(to, cls):
                 return
-
+            new_node = getattr(to, cls)()
             for field in node._fields:
                 setattr(new_node, field, self._visit(getattr(node, field)))
-
-            for attr in node._attributes:
-                try:
+            for attr in getattr(node, '_attributes'):
+                if hasattr(node, attr):
                     setattr(new_node, attr, getattr(node, attr))
-                except AttributeError:
-                    pass
             return new_node
 
     return Translator
diff --git a/gast/gast.py b/gast/gast.py
index 11b248a..7ebc26d 100644
--- a/gast/gast.py
+++ b/gast/gast.py
@@ -12,27 +12,25 @@ except ImportError:
 
 
 def _make_node(Name, Fields, Attributes, Bases):
-    NBFields = len(Fields)
-
     def create_node(self, *args, **kwargs):
-        if args:
-            if len(args) != NBFields:
-                raise TypeError(
-                    "{} constructor takes either 0 or {} positional arguments".
-                    format(Name, NBFields))
-            for argname, argval in zip(Fields, args):
-                setattr(self, argname, argval)
-        if kwargs:
-            for argname, argval in kwargs.items():
-                setattr(self, argname, argval)
+        nbparam = len(args) + len(kwargs)
+        assert nbparam in (0, len(Fields)), \
+            "Bad argument number for {}: {}, expecting {}".\
+            format(Name, nbparam, len(Fields))
+        self._fields = Fields
+        self._attributes = Attributes
+        for argname, argval in zip(self._fields, args):
+            setattr(self, argname, argval)
+        for argname, argval in kwargs.items():
+            assert argname in Fields, \
+                    "Invalid Keyword argument for {}: {}".format(Name, argname)
+            setattr(self, argname, argval)
 
     setattr(_sys.modules[__name__],
             Name,
             type(Name,
                  Bases,
-                 {'__init__': create_node,
-                  '_fields': Fields,
-                  '_attributes': Attributes}))
+                 {'__init__': create_node}))
 
 
 _nodes = (
@@ -133,6 +131,9 @@ _nodes = (
     ('BoolOp', (('op', 'values',),
                 ('lineno', 'col_offset', 'end_lineno', 'end_col_offset',),
                 (expr,))),
+    ('NamedExpr', (('target', 'value',),
+                ('lineno', 'col_offset', 'end_lineno', 'end_col_offset',),
+                (expr,))),
     ('BinOp', (('left', 'op', 'right',),
                ('lineno', 'col_offset', 'end_lineno', 'end_col_offset',),
                (expr,))),


Test patch:
diff --git a/tests/test_compat.py b/tests/test_compat.py
index c04c433..bca7683 100644
--- a/tests/test_compat.py
+++ b/tests/test_compat.py
@@ -112,6 +112,16 @@ class CompatTestCase(unittest.TestCase):
                         "], type_ignores=[])")
                 self.assertEqual(gast.dump(tree), norm)
 
+            def test_NamedExpr(self):
+                code = '(x := 1) '
+                tree = gast.parse(code)
+                compile(gast.gast_to_ast(tree), '<test>', 'exec')
+                norm = ("Module(body=[Expr(value=NamedExpr(target=Name(id='x',"
+                        " ctx=Store(), annotation=None, type_comment=None), "
+                        "value=Constant(value=1, kind=None)))], type_ignores="
+                        "[])")
+                self.assertEqual(gast.dump(tree), norm)
+
         else:
 
             def test_Bytes(self):