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

Parsing does not raise error for code assigning to a function call #916

Closed
DRMacIver opened this issue Apr 28, 2023 · 5 comments
Closed

Comments

@DRMacIver
Copy link

LibCST will happily parse the following code:

import libcst
libcst.parse_statement('max() = 1')

However this is a syntax error, as you can't assign to function calls, and ideally would be raised as such.

(Tested on libcst 0.4.9)

@Zac-HD Zac-HD mentioned this issue Apr 29, 2023
@jakkdl
Copy link
Contributor

jakkdl commented May 22, 2023

So, I'm looking at whether to place this validation in BaseAssignTargetExpression, AssignTarget, and/or Assign/AnnAssign/AugAssign.

class BaseAssignTargetExpression(BaseExpression, ABC):
"""
An expression that's valid on the left side of an assignment. That assignment may
be part an :class:`Assign` node, or it may be part of a number of other control
structures that perform an assignment, such as a :class:`For` loop.
Python's grammar defines all expression as valid in this position, but the AST
compiler further restricts the allowed types, which is what this type attempts to
express.
This is similar to a :class:`BaseDelTargetExpression`, but it also includes
:class:`StarredElement` as a valid node.
The set of valid nodes are defined as part of `CPython's AST context computation
<https://github.com/python/cpython/blob/v3.8.0a4/Python/ast.c#L1120>`_.
"""

since 3.9 the python grammar does define the valid assignment targets

 assignment:
    | NAME ':' expression ['=' annotated_rhs ] 
    | ('(' single_target ')' 
         | single_subscript_attribute_target) ':' expression ['=' annotated_rhs ] 
    | (star_targets '=' )+ (yield_expr | star_expressions) !'=' [TYPE_COMMENT] 
    | single_target augassign ~ (yield_expr | star_expressions) 

so I thought I'd add a _validate to BaseAssignTargetExpression that checks whether it's a Name, Subscript or StarredElement. But then I noticed that a lot of other classes inherits from it, including Name, Annotation, Attribute - which all quickly starts breaking if I restrict what BaseAssignTargetExpression can be. Is there a reason why all of these don't just inherit from BaseExpression, or can I change them to do so? Or should I place the validation in AssignTarget instead?

@jakkdl
Copy link
Contributor

jakkdl commented May 24, 2023

@zsol any thoughts on the above?

@zsol
Copy link
Member

zsol commented May 25, 2023

The native parser correctly raises a syntax error here. So to work around you can set the LIBCST_PARSER_TYPE environment variable to native before calling libcst, or wait for the next release which will have this behavior by default due to #929

@jakkdl
Copy link
Contributor

jakkdl commented May 26, 2023

Oh, I totally thought I tested it with native - but apparently not. Sweet!

@zsol
Copy link
Member

zsol commented May 26, 2023

Gonna close this out as native parser is enabled by default in 1.0.0

@zsol zsol closed this as completed May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants