Skip to content

Commit

Permalink
Reject more syntactically invalid Python programs (#8524)
Browse files Browse the repository at this point in the history
## 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`.
  • Loading branch information
BurntSushi authored Nov 7, 2023
1 parent c3d6d5d commit 6a1fa47
Show file tree
Hide file tree
Showing 20 changed files with 1,432 additions and 148 deletions.
8 changes: 6 additions & 2 deletions crates/ruff_linter/src/rules/eradicate/detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,15 @@ mod tests {
"# ( user_content_type , _ )= TimelineEvent.objects.using(db_alias).get_or_create(",
&[]
));
assert!(comment_contains_code(
assert!(comment_contains_code("# )", &[]));

// This used to return true, but our parser has gotten a bit better
// at rejecting invalid Python syntax. And indeed, this is not valid
// Python code.
assert!(!comment_contains_code(
"# app_label=\"core\", model=\"user\"",
&[]
));
assert!(comment_contains_code("# )", &[]));

// TODO(charlie): This should be `true` under aggressive mode.
assert!(!comment_contains_code("#def foo():", &[]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ def foo():
pass


(yield a, b) = (1, 2)

# some comment
for e in l : yield e # some comment

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ def foo():
pass
(yield a, b) = (1, 2)
# some comment
for e in l : yield e # some comment
Expand Down Expand Up @@ -151,8 +149,6 @@ def foo():
# comment
pass
(yield a, b) = (1, 2)
# some comment
for e in l:
yield e # some comment
Expand Down
Loading

0 comments on commit 6a1fa47

Please sign in to comment.