-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Produce error when NamedTuple is assigned to attribute #7662
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
Conversation
fe9ac72
to
3c062be
Compare
hi @JukkaL , could you review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Looks mostly good. I have a suggestion about tests.
f8df8c9
to
6651401
Compare
return False | ||
lvalue = s.lvalues[0] | ||
name = lvalue.name | ||
is_named_tuple, info = self.named_tuple_analyzer.check_namedtuple(s.rvalue, name, | ||
self.is_func_scope()) | ||
if not is_named_tuple: | ||
return False | ||
if isinstance(s.lvalues[0], MemberExpr): | ||
self.fail("NamedTuple type as an attribute is not supported", lvalue) | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this block to start but then a NamedTuple type as an attrib ...
error was generated on every attribute assignment statement.
as the error should raise only if RHS is NamedTuple, i think this block is better at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks good.
…me for variable (#8112) Followed the discussion in #8107 (comment), refine the code merged in #7662 by: - move testcase from `pythoneval.test` to `check-namedtuple.test` and rename it from `testNamedTupleAtRunTime` to `testAssignNamedTupleAsAttribute` - change `s.lvalues[0]` to `lvalue` to make it more concise
Fixes #7531