Reject more syntactically invalid Python programs (#8524)

## Summary

This commit adds some additional error checking to the parser such that
assignments that are invalid syntax are rejected. This covers the
obvious cases like `5 = 3` and some not so obvious cases like `x + y =
42`.

This does add an additional recursive call to the parser for the cases
handling assignments. I had initially been concerned about doing this,
but `set_context` is already doing recursion during assignments, so I
didn't feel as though this was changing any fundamental performance
characteristics of the parser. (Also, in practice, I would expect any
such recursion here to be quite shallow since the recursion is done on
the target of an assignment. Such things are rarely nested much in
practice.)

Fixes #6895

## Test Plan

I've added unit tests covering every case that is detected as invalid on
an `Expr`.
This commit is contained in:
Andrew Gallant 2023-11-07 07:16:06 -05:00 committed by GitHub
parent c3d6d5d006
commit 6a1fa4778f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 1432 additions and 148 deletions

View file

@ -13,6 +13,7 @@ use crate::{
context::set_context,
string::{StringType, concatenate_strings, parse_fstring_middle, parse_string_literal},
token::{self, StringKind},
invalid,
};
use lalrpop_util::ParseError;
@ -108,12 +109,12 @@ DelStatement: ast::Stmt = {
};
ExpressionStatement: ast::Stmt = {
<location:@L> <expression:TestOrStarExprList> <suffix:AssignSuffix*> <end_location:@R> => {
<location:@L> <expression:TestOrStarExprList> <suffix:AssignSuffix*> <end_location:@R> =>? {
// Just an expression, no assignment:
if suffix.is_empty() {
ast::Stmt::Expr(
Ok(ast::Stmt::Expr(
ast::StmtExpr { value: Box::new(expression.into()), range: (location..end_location).into() }
)
))
} else {
let mut targets = vec![set_context(expression.into(), ast::ExprContext::Store)];
let mut values = suffix;
@ -123,25 +124,27 @@ ExpressionStatement: ast::Stmt = {
for target in values {
targets.push(set_context(target.into(), ast::ExprContext::Store));
}
ast::Stmt::Assign(
invalid::assignment_targets(&targets)?;
Ok(ast::Stmt::Assign(
ast::StmtAssign { targets, value, range: (location..end_location).into() }
)
))
}
},
<location:@L> <target:TestOrStarExprList> <op:AugAssign> <rhs:TestListOrYieldExpr> <end_location:@R> => {
ast::Stmt::AugAssign(
<location:@L> <target:TestOrStarExprList> <op:AugAssign> <rhs:TestListOrYieldExpr> <end_location:@R> =>? {
invalid::assignment_target(&target.expr)?;
Ok(ast::Stmt::AugAssign(
ast::StmtAugAssign {
target: Box::new(set_context(target.into(), ast::ExprContext::Store)),
op,
value: Box::new(rhs.into()),
range: (location..end_location).into()
},
)
))
},
<location:@L> <target:Test<"all">> ":" <annotation:Test<"all">> <rhs:AssignSuffix?> <end_location:@R> => {
<location:@L> <target:Test<"all">> ":" <annotation:Test<"all">> <rhs:AssignSuffix?> <end_location:@R> =>? {
let simple = target.expr.is_name_expr();
ast::Stmt::AnnAssign(
invalid::assignment_target(&target.expr)?;
Ok(ast::Stmt::AnnAssign(
ast::StmtAnnAssign {
target: Box::new(set_context(target.into(), ast::ExprContext::Store)),
annotation: Box::new(annotation.into()),
@ -149,7 +152,7 @@ ExpressionStatement: ast::Stmt = {
simple,
range: (location..end_location).into()
},
)
))
},
};