From 91f651ceec4f117b4880e0ff63bd7c22907b9828 Mon Sep 17 00:00:00 2001 From: Drew O'Meara <35852900+drew-512@users.noreply.github.com> Date: Sun, 8 Nov 2026 16:32:39 -0600 Subject: [PATCH] parser: fix dotted decorator names A decorator with a dotted name, e.g. a.b.c(x), was parsed into a single Name whose Id was the whole string "a.b.c", so it failed at runtime with NameError: name 'a.b.c' is not defined. Build the decorator expression as CPython 3.4 does in ast_for_dotted_name: a Name for the first component and one Attribute (Load) per further component, all at the start of the dotted name, wrapped in a Call when arguments are given. Plain decorators now also carry the name's position rather than the '@', as in CPython. The new dottedName helper lives in parser.go so that grammar.y keeps its line numbers: the regenerated y.go changes by one line and y.output does not change. Enable test_dotted and test_eval_order in vm/tests/decorators.py and expect AttributeError for the nullval.attr decorator, as in CPython's test_decorators.py. Add dotted decorator cases to the parser tests. --- parser/grammar.y | 2 +- parser/grammar_data_test.go | 4 ++ parser/make_grammar_test.py | 22 ++++++++ parser/parser.go | 18 +++++++ parser/y.go | 2 +- vm/tests/decorators.py | 105 ++++++++++++++++++------------------ 6 files changed, 98 insertions(+), 55 deletions(-) diff --git a/parser/grammar.y b/parser/grammar.y index 5ad69751..60afd918 100644 --- a/parser/grammar.y +++ b/parser/grammar.y @@ -332,7 +332,7 @@ optional_arglist_call: decorator: '@' dotted_name optional_arglist_call NEWLINE { - fn := &ast.Name{ExprBase: ast.ExprBase{Pos: $$}, Id: ast.Identifier($2), Ctx: ast.Load} + fn := dottedName($2, $2) if $3 == nil { $$ = fn } else { diff --git a/parser/grammar_data_test.go b/parser/grammar_data_test.go index b0b53eeb..179f8ecd 100644 --- a/parser/grammar_data_test.go +++ b/parser/grammar_data_test.go @@ -320,6 +320,10 @@ var grammarTestData = []struct { {"@dec(a,b,c=d,*args,**kwargs)\ndef fn():\n pass\n", "exec", "Module(body=[FunctionDef(name='fn', args=arguments(args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Pass()], decorator_list=[Call(func=Name(id='dec', ctx=Load()), args=[Name(id='a', ctx=Load()), Name(id='b', ctx=Load())], keywords=[keyword(arg='c', value=Name(id='d', ctx=Load()))], starargs=Name(id='args', ctx=Load()), kwargs=Name(id='kwargs', ctx=Load()))], returns=None)])", nil, ""}, {"@dec1\n@dec2()\n@dec3(a)\n@dec4(a,b)\ndef fn():\n pass\n", "exec", "Module(body=[FunctionDef(name='fn', args=arguments(args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Pass()], decorator_list=[Name(id='dec1', ctx=Load()), Call(func=Name(id='dec2', ctx=Load()), args=[], keywords=[], starargs=None, kwargs=None), Call(func=Name(id='dec3', ctx=Load()), args=[Name(id='a', ctx=Load())], keywords=[], starargs=None, kwargs=None), Call(func=Name(id='dec4', ctx=Load()), args=[Name(id='a', ctx=Load()), Name(id='b', ctx=Load())], keywords=[], starargs=None, kwargs=None)], returns=None)])", nil, ""}, {"@dec1\n@dec2()\n@dec3(a)\n@dec4(a,b)\nclass A(B):\n pass\n", "exec", "Module(body=[ClassDef(name='A', bases=[Name(id='B', ctx=Load())], keywords=[], starargs=None, kwargs=None, body=[Pass()], decorator_list=[Name(id='dec1', ctx=Load()), Call(func=Name(id='dec2', ctx=Load()), args=[], keywords=[], starargs=None, kwargs=None), Call(func=Name(id='dec3', ctx=Load()), args=[Name(id='a', ctx=Load())], keywords=[], starargs=None, kwargs=None), Call(func=Name(id='dec4', ctx=Load()), args=[Name(id='a', ctx=Load()), Name(id='b', ctx=Load())], keywords=[], starargs=None, kwargs=None)])])", nil, ""}, + {"@a.b\ndef fn():\n pass\n", "exec", "Module(body=[FunctionDef(name='fn', args=arguments(args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Pass()], decorator_list=[Attribute(value=Name(id='a', ctx=Load()), attr='b', ctx=Load())], returns=None)])", nil, ""}, + {"@a.b.c(d,e=f)\ndef fn():\n pass\n", "exec", "Module(body=[FunctionDef(name='fn', args=arguments(args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Pass()], decorator_list=[Call(func=Attribute(value=Attribute(value=Name(id='a', ctx=Load()), attr='b', ctx=Load()), attr='c', ctx=Load()), args=[Name(id='d', ctx=Load())], keywords=[keyword(arg='e', value=Name(id='f', ctx=Load()))], starargs=None, kwargs=None)], returns=None)])", nil, ""}, + {"@a.b\n@c.d()\n@e\nclass A(B):\n pass\n", "exec", "Module(body=[ClassDef(name='A', bases=[Name(id='B', ctx=Load())], keywords=[], starargs=None, kwargs=None, body=[Pass()], decorator_list=[Attribute(value=Name(id='a', ctx=Load()), attr='b', ctx=Load()), Call(func=Attribute(value=Name(id='c', ctx=Load()), attr='d', ctx=Load()), args=[], keywords=[], starargs=None, kwargs=None), Name(id='e', ctx=Load())])])", nil, ""}, + {"@a.\ndef fn():\n pass\n", "exec", "", py.SyntaxError, "invalid syntax"}, {"", "single", "", py.SyntaxError, "unexpected EOF while parsing"}, {"\n", "single", "", py.SyntaxError, "unexpected EOF while parsing"}, {"pass\n", "single", "Interactive(body=[Pass()])", nil, ""}, diff --git a/parser/make_grammar_test.py b/parser/make_grammar_test.py index d354e43a..00324368 100755 --- a/parser/make_grammar_test.py +++ b/parser/make_grammar_test.py @@ -470,6 +470,28 @@ def fn(): class A(B): pass """, "exec"), + ("""\ +@a.b +def fn(): + pass +""", "exec"), + ("""\ +@a.b.c(d,e=f) +def fn(): + pass +""", "exec"), + ("""\ +@a.b +@c.d() +@e +class A(B): + pass +""", "exec"), + ("""\ +@a. +def fn(): + pass +""", "exec", SyntaxError), # single input ("", "single", SyntaxError), diff --git a/parser/parser.go b/parser/parser.go index 4c4cd808..14b3bbd2 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -9,3 +9,21 @@ //go:generate goyacc -v y.output grammar.y package parser + +import ( + "strings" + + "github.com/go-python/gpython/ast" +) + +// dottedName returns the expression for a dotted name such as "a.b.c": a +// Name for the first part and an Attribute for each further part, all at +// pos, as ast_for_dotted_name does in CPython. +func dottedName(pos ast.Pos, name string) ast.Expr { + parts := strings.Split(name, ".") + var expr ast.Expr = &ast.Name{ExprBase: ast.ExprBase{Pos: pos}, Id: ast.Identifier(parts[0]), Ctx: ast.Load} + for _, part := range parts[1:] { + expr = &ast.Attribute{ExprBase: ast.ExprBase{Pos: pos}, Value: expr, Attr: ast.Identifier(part), Ctx: ast.Load} + } + return expr +} diff --git a/parser/y.go b/parser/y.go index 39027c98..21507ed3 100644 --- a/parser/y.go +++ b/parser/y.go @@ -1167,7 +1167,7 @@ yydefault: yyDollar = yyS[yypt-4 : yypt+1] //line grammar.y:334 { - fn := &ast.Name{ExprBase: ast.ExprBase{Pos: yyVAL.pos}, Id: ast.Identifier(yyDollar[2].str), Ctx: ast.Load} + fn := dottedName(yyDollar[2].pos, yyDollar[2].str) if yyDollar[3].call == nil { yyVAL.expr = fn } else { diff --git a/vm/tests/decorators.py b/vm/tests/decorators.py index b7e2d703..72bd3f32 100644 --- a/vm/tests/decorators.py +++ b/vm/tests/decorators.py @@ -107,12 +107,11 @@ def notamethod(x): self.assertRaises(TypeError, notamethod, 1) doc="test_dotted" -# FIXME class decorator -# decorators = MiscDecorators() -# @decorators.author('Cleese') -# def foo(): return 42 -# self.assertEqual(foo(), 42) -# self.assertEqual(foo.author, 'Cleese') +decorators = MiscDecorators() +@decorators.author('Cleese') +def foo(): return 42 +self.assertEqual(foo(), 42) +self.assertEqual(foo.author, 'Cleese') doc="test_argforms" def noteargs(*args, **kwds): @@ -198,7 +197,7 @@ def unimp(func): for expr, exc in [ ("undef", NameError), ("nullval", TypeError), - ("nullval.attr", NameError), # FIXME ("nullval.attr", AttributeError), + ("nullval.attr", AttributeError), ("unimp", NotImplementedError)]: codestr = "@%s\ndef f(): pass\nassert f() is None" % expr code = compile(codestr, "test", "exec") @@ -244,53 +243,53 @@ def foo(): return 42 # performed in the above order for each decorator, but we should # iterate through the decorators in the reverse of the order they # appear in the source. -# FIXME class decorator -# actions = [] -# -# def make_decorator(tag): -# actions.append('makedec' + tag) -# def decorate(func): -# actions.append('calldec' + tag) -# return func -# return decorate -# -# class NameLookupTracer (object): -# def __init__(self, index): -# self.index = index -# -# def __getattr__(self, fname): -# if fname == 'make_decorator': -# opname, res = ('evalname', make_decorator) -# elif fname == 'arg': -# opname, res = ('evalargs', str(self.index)) -# else: -# assert False, "Unknown attrname %s" % fname -# actions.append('%s%d' % (opname, self.index)) -# return res -# -# c1, c2, c3 = map(NameLookupTracer, [ 1, 2, 3 ]) -# -# expected_actions = [ 'evalname1', 'evalargs1', 'makedec1', -# 'evalname2', 'evalargs2', 'makedec2', -# 'evalname3', 'evalargs3', 'makedec3', -# 'calldec3', 'calldec2', 'calldec1' ] -# -# actions = [] -# @c1.make_decorator(c1.arg) -# @c2.make_decorator(c2.arg) -# @c3.make_decorator(c3.arg) -# def foo(): return 42 -# self.assertEqual(foo(), 42) -# -# self.assertEqual(actions, expected_actions) + +actions = [] + +def make_decorator(tag): + actions.append('makedec' + tag) + def decorate(func): + actions.append('calldec' + tag) + return func + return decorate + +class NameLookupTracer (object): + def __init__(self, index): + self.index = index + + def __getattr__(self, fname): + if fname == 'make_decorator': + opname, res = ('evalname', make_decorator) + elif fname == 'arg': + opname, res = ('evalargs', str(self.index)) + else: + assert False, "Unknown attrname %s" % fname + actions.append('%s%d' % (opname, self.index)) + return res + +c1, c2, c3 = map(NameLookupTracer, [ 1, 2, 3 ]) + +expected_actions = [ 'evalname1', 'evalargs1', 'makedec1', + 'evalname2', 'evalargs2', 'makedec2', + 'evalname3', 'evalargs3', 'makedec3', + 'calldec3', 'calldec2', 'calldec1' ] + +actions = [] +@c1.make_decorator(c1.arg) +@c2.make_decorator(c2.arg) +@c3.make_decorator(c3.arg) +def foo(): return 42 +self.assertEqual(foo(), 42) + +self.assertEqual(actions, expected_actions) + +# Test the equivalence claim in chapter 7 of the reference manual. # -# # Test the equivalence claim in chapter 7 of the reference manual. -# # -# actions = [] -# def bar(): return 42 -# bar = c1.make_decorator(c1.arg)(c2.make_decorator(c2.arg)(c3.make_decorator(c3.arg)(bar))) -# self.assertEqual(bar(), 42) -# self.assertEqual(actions, expected_actions) +actions = [] +def bar(): return 42 +bar = c1.make_decorator(c1.arg)(c2.make_decorator(c2.arg)(c3.make_decorator(c3.arg)(bar))) +self.assertEqual(bar(), 42) +self.assertEqual(actions, expected_actions) doc="test_simple" def plain(x):