## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements a new semantic syntax error where annotated name
can't be global
example
```
x: int = 1
def f():
global x
x: str = "foo" # SyntaxError: annotated name 'x' can't be global
```
## Test Plan
<!-- How was it tested? -->
I have written tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## Summary
This PR re-implements [return-in-generator
(B901)](https://docs.astral.sh/ruff/rules/return-in-generator/#return-in-generator-b901)
for async generators as a semantic syntax error. This is not a syntax
error for sync generators, so we'll need to preserve both the lint rule
and the syntax error in this case.
It also updates B901 and the new implementation to catch cases where the
generator's `yield` or `yield from` expression is part of another
statement, as in:
```py
def foo():
return (yield)
```
These were previously not caught because we only looked for
`Stmt::Expr(Expr::Yield)` in `visit_stmt` instead of visiting `yield`
expressions directly. I think this modification is within the spirit of
the rule and safe to try out since the rule is in preview.
## Test Plan
<!-- How was it tested? -->
I have written tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
This PR implements syntax error where a default type parameter is
followed by a non-default type parameter.
https://github.com/astral-sh/ruff/issues/17412#issuecomment-3584088217
## Test Plan
I have written inline tests as directed in #17412
---------
Signed-off-by: 11happy <bhuminjaysoni@gmail.com>
Signed-off-by: 11happy <soni5happy@gmail.com>
## Summary
Fixes#21750 and a related bug in `PLE1142`. We were not properly
considering generators to be valid `await` contexts, which caused the
`F704` issue. One of the tests I added for this also uncovered an issue
in `PLE1142` for comprehensions nested within async generators because
we were only checking the current scope rather than traversing the
nested context.
## Test Plan
Both of these rules are implemented as semantic syntax errors, so I
added tests (and fixes) in both Ruff and ty.
## Summary
Fixes a panic when parsing IPython escape commands with `Help` kind
(`?`) in expression contexts. The parser now reports an error instead of
panicking.
Fixes#21465.
## Problem
The parser panicked with `unreachable!()` in
`parse_ipython_escape_command_expression` when encountering escape
commands with `Help` kind (`?`) in expression contexts, where only
`Magic` (`%`) and `Shell` (`!`) are allowed.
## Approach
Replaced the `unreachable!()` panic with error handling that adds a
`ParseErrorType::OtherError` and continues parsing, returning a valid
AST node with the error attached.
## Test Plan
Added `test_ipython_escape_command_in_with_statement` and
`test_ipython_help_escape_command_as_expression` to verify the fix.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Summary
--
Fixes#21360 by using the union of names instead of overwriting them, as
Micha suggested originally on #21104.
This avoids overwriting the `n` name in the `Subscript` by the empty set
of names visited in the nested OR pattern before visiting the other arm
of the outer OR pattern.
Test Plan
--
A new inline test case taken from the issue
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR ports PLE0117 as a semantic syntax error.
## Test Plan
<!-- How was it tested? -->
Tests previously written
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
The parser currently uses single quotes to wrap tokens. This is
inconsistent with the rest of ruff/ty, which use backticks.
For example, see the inconsistent diagnostics produced in this simple
example: https://play.ty.dev/0a9d6eab-6599-4a1d-8e40-032091f7f50f
Consistently wrapping tokens in backticks produces uniform diagnostics.
Following the style decision of #723, in #2889 some quotes were already
switched into backticks.
This is also in line with Rust's guide on diagnostics
(https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure):
> When code or an identifier must appear in a message or label, it
should be surrounded with backticks
## Summary
Fixes#21101 by storing the child visitor's names in the parent visitor.
This makes sure that `visitor.names` on line 1818 isn't empty after we
visit a nested OR pattern.
## Test Plan
New inline test cases derived from the issue,
[playground](https://play.ruff.rs/7b6439ac-ee8f-4593-9a3e-c2aa34a595d0)
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements a new semantic syntax error where name is parameter &
global.
## Test Plan
<!-- How was it tested? -->
I have written inline test as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Summary
--
This PR fixes the issue I added in #20867 and noticed in #20930. Cases
like this
cause an error on any Python version:
```py
f"{1:""}"
```
which gave me a false sense of security before. Cases like this are
still
invalid only before 3.12 and weren't flagged after the changes in
#20867:
```py
f'{1: abcd "{'aa'}" }'
# ^ reused quote
f'{1: abcd "{"\n"}" }'
# ^ backslash
```
I didn't recognize these as nested interpolations that also need to be
checked
for invalid expressions, so filtering out the whole format spec wasn't
quite
right. And `elements.interpolations()` only iterates over the outermost
interpolations, not the nested ones.
There's basically no code change in this PR, I just moved the existing
check
from `parse_interpolated_string`, which parses the entire string, to
`parse_interpolated_element`. This kind of seems more natural anyway and
avoids
having to try to recursively visit nested elements after the fact in
`parse_interpolated_string`. So viewing the diff with something like
```
git diff --color-moved --ignore-space-change --color-moved-ws=allow-indentation-change main
```
should make this more clear.
Test Plan
--
New tests
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements semantic syntax error where alternative patterns bind
different names
## Test Plan
<!-- How was it tested? -->
I have written inline tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements `F702`
https://docs.astral.sh/ruff/rules/continue-outside-loop/ as semantic
syntax error.
## Test Plan
<!-- How was it tested? -->
Tests are already previously written in F702
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Summary
--
Fixes#20844 by refining the unsupported syntax error check for [PEP
701]
f-strings before Python 3.12 to allow backslash escapes and escaped
outer quotes
in the format spec part of f-strings. These are only disallowed within
the
f-string expression part on earlier versions. Using the examples from
the PR:
```pycon
>>> f"{1:\x64}"
'1'
>>> f"{1:\"d\"}"
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier '"d"' for object of type 'int'
```
Note that the second case is a runtime error, but this is actually
avoidable if
you override `__format__`, so despite being pretty weird, this could
actually be
a valid use case.
```pycon
>>> class C:
... def __format__(*args, **kwargs): return "<C>"
...
>>> f"{C():\"d\"}"
'<C>'
```
At first I thought narrowing the range we check to exclude the format
spec would
only work for escapes, but it turns out that cases like `f"{1:""}"` are
already
covered by an existing `ParseError`, so we can just narrow the range of
both our
escape and quote checks.
Our comment check also seems to be working correctly because it's based
on the
actual tokens. A case like
[this](https://play.ruff.rs/9f1c2ff2-cd8e-4ad7-9f40-56c0a524209f):
```python
f"""{1:# }"""
```
doesn't include a comment token, instead the `#` is part of an
`InterpolatedStringLiteralElement`.
Test Plan
--
New inline parser tests
[PEP 701]: https://peps.python.org/pep-0701/
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements https://docs.astral.sh/ruff/rules/break-outside-loop/
(F701) as a semantic syntax error.
## Test Plan
<!-- How was it tested? -->
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This PR resolves the issue noticed in
https://github.com/astral-sh/ruff/pull/20777#discussion_r2417233227.
Namely, cases like this were being flagged as syntax errors despite
being perfectly valid on Python 3.8:
```pycon
Python 3.8.20 (default, Oct 2 2024, 16:34:12)
[Clang 18.1.8 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> with (open("foo.txt", "w")): ...
...
Ellipsis
>>> with (open("foo.txt", "w")) as f: print(f)
...
<_io.TextIOWrapper name='foo.txt' mode='w' encoding='UTF-8'>
```
The second of these was already allowed but not the first:
```shell
> ruff check --target-version py38 --ignore ALL - <<EOF
with (open("foo.txt", "w")): ...
with (open("foo.txt", "w")) as f: print(f)
EOF
invalid-syntax: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
--> -:1:6
|
1 | with (open("foo.txt", "w")): ...
| ^
2 | with (open("foo.txt", "w")) as f: print(f)
|
Found 1 error.
```
There was some discussion of related cases in
https://github.com/astral-sh/ruff/pull/16523#discussion_r1984657793, but
it seems I overlooked the single-element case when flagging tuples. As
suggested in the other thread, we can just check if there's more than
one element or a trailing comma, which will cause the tuple parsing on
<=3.8 and avoid the false positives.
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements
https://docs.astral.sh/ruff/rules/future-feature-not-defined/ (F407) as
a semantic syntax error.
## Test Plan
<!-- How was it tested? -->
I have written inline tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR implements
https://docs.astral.sh/ruff/rules/multiple-starred-expressions/ as a
semantic syntax error
## Test Plan
I have added inline tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR implements F406
https://docs.astral.sh/ruff/rules/undefined-local-with-nested-import-star-usage/
as a semantic syntax error
## Test Plan
I have written inline tests as directed in #17412
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR implements
https://docs.astral.sh/ruff/rules/yield-from-in-async-function/ as a
syntax semantic error
## Test Plan
<!-- How was it tested? -->
I have written a simple inline test as directed in
[https://github.com/astral-sh/ruff/issues/17412](https://github.com/astral-sh/ruff/issues/17412)
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
Removes the `module_ptr` field from `AstNodeRef` in release mode, and
change `NodeIndex` to a `NonZeroU32` to reduce the size of
`Option<AstNodeRef<_>>` fields.
I believe CI runs in debug mode, so this won't show up in the memory
report, but this reduces memory by ~2% in release mode.
As of [this cpython PR](https://github.com/python/cpython/pull/135996),
it is not allowed to concatenate t-strings with non-t-strings,
implicitly or explicitly. Expressions such as `"foo" t"{bar}"` are now
syntax errors.
This PR updates some AST nodes and parsing to reflect this change.
The structural change is that `TStringPart` is no longer needed, since,
as in the case of `BytesStringLiteral`, the only possibilities are that
we have a single `TString` or a vector of such (representing an implicit
concatenation of t-strings). This removes a level of nesting from many
AST expressions (which is what all the snapshot changes reflect), and
simplifies some logic in the implementation of visitors, for example.
The other change of note is in the parser. When we meet an implicit
concatenation of string-like literals, we now count the number of
t-string literals. If these do not exhaust the total number of
implicitly concatenated pieces, then we emit a syntax error. To recover
from this syntax error, we encode any t-string pieces as _invalid_
string literals (which means we flag them as invalid, record their
range, and record the value as `""`). Note that if at least one of the
pieces is an f-string we prefer to parse the entire string as an
f-string; otherwise we parse it as a string.
This logic is exactly the same as how we currently treat
`BytesStringLiteral` parsing and error recovery - and carries with it
the same pros and cons.
Finally, note that I have not implemented any changes in the
implementation of the formatter. As far as I can tell, none are needed.
I did change a few of the fixtures so that we are always concatenating
t-strings with t-strings.
Parsing the (invalid) expression `f"{\t"i}"` caused a panic because the
`TStringMiddle` character was "unreachable" due the way the parser
recovered from the line continuation (it ate the t-string start).
The cause of the issue is as follows:
The parser begins parsing the f-string and expects to see a list of
objects, essentially alternating between _interpolated elements_ and
ordinary strings. It is happy to see the first left brace, but then
there is a lexical error caused by the line-continuation character. So
instead of the parser seeing a list of elements with just one member, it
sees a list that starts like this:
- Interpolated element with an invalid token, stored as a `Name`
- Something else built from tokens beginning with `TStringStart` and
`TStringMiddle`
When it sees the `TStringStart` error recovery says "that's a list
element I don't know what to do with, let's skip it". When it sees
`TStringMiddle` it says "oh, that looks like the middle of _some
interpolated string_ so let's try to parse it as one of the literal
elements of my `FString`". Unfortunately, the function being used to
parse individual list elements thinks (arguably correctly) that it's not
possible to have a `TStringMiddle` sitting in your `FString`, and hits
`unreachable`.
Two potential ways (among many) to solve this issue are:
1. Allow a `TStringMiddle` as a valid "literal" part of an f-string
during parsing (with the hope/understanding that this would only occur
in an invalid context)
2. Skip the `TStringMiddle` as an "unexpected/invalid list item" in the
same way that we skipped `TStringStart`.
I have opted for the second approach since it seems somehow more morally
correct, even though it loses more information. To implement this, the
recovery context needs to know whether we are in an f-string or t-string
- hence the changes to that enum. As a bonus we get slightly more
specific error messages in some cases.
Closes#18860
## Summary
This PR is a collaboration with @AlexWaygood from our pairing session
last Friday.
The main goal here is removing `ruff_linter::message::OldDiagnostic` in
favor of
using `ruff_db::diagnostic::Diagnostic` directly. This involved a few
major steps:
- Transferring the fields
- Transferring the methods and trait implementations, where possible
- Converting some constructor methods to free functions
- Moving the `SecondaryCode` struct
- Updating the method names
I'm hoping that some of the methods, especially those in the
`expect_ruff_*`
family, won't be necessary long-term, but I avoided trying to replace
them
entirely for now to keep the already-large diff a bit smaller.
### Related refactors
Alex and I noticed a few refactoring opportunities while looking at the
code,
specifically the very similar implementations for
`create_parse_diagnostic`,
`create_unsupported_syntax_diagnostic`, and
`create_semantic_syntax_diagnostic`.
We combined these into a single generic function, which I then copied
into
`ruff_linter::message` with some small changes and a TODO to combine
them in the
future.
I also deleted the `DisplayParseErrorType` and `TruncateAtNewline` types
for
reporting parse errors. These were added in #4124, I believe to work
around the
error messages from LALRPOP. Removing these didn't affect any tests, so
I think
they were unnecessary now that we fully control the error messages from
the
parser.
On a more minor note, I factored out some calls to the
`OldDiagnostic::filename`
(now `Diagnostic::expect_ruff_filename`) function to avoid repeatedly
allocating
`String`s in some places.
### Snapshot changes
The `show_statistics_syntax_errors` integration test changed because the
`OldDiagnostic::name` method used `syntax-error` instead of
`invalid-syntax`
like in ty. I think this (`--statistics`) is one of the only places we
actually
use this name for syntax errors, so I hope this is okay. An alternative
is to
use `syntax-error` in ty too.
The other snapshot changes are from removing this code, as discussed on
[Discord](https://discord.com/channels/1039017663004942429/1228460843033821285/1388252408848847069):
34052a1185/crates/ruff_linter/src/message/mod.rs (L128-L135)
I think both of these are technically breaking changes, but they only
affect
syntax errors and are very narrow in scope, while also pretty
substantially
simplifying the refactor, so I hope they're okay to include in a patch
release.
## Test plan
Existing tests, with the adjustments mentioned above
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Setting `TY_MEMORY_REPORT=full` will generate and print a memory usage
report to the CLI after a `ty check` run:
```
=======SALSA STRUCTS=======
`Definition` metadata=7.24MB fields=17.38MB count=181062
`Expression` metadata=4.45MB fields=5.94MB count=92804
`member_lookup_with_policy_::interned_arguments` metadata=1.97MB fields=2.25MB count=35176
...
=======SALSA QUERIES=======
`File -> ty_python_semantic::semantic_index::SemanticIndex`
metadata=11.46MB fields=88.86MB count=1638
`Definition -> ty_python_semantic::types::infer::TypeInference`
metadata=24.52MB fields=86.68MB count=146018
`File -> ruff_db::parsed::ParsedModule`
metadata=0.12MB fields=69.06MB count=1642
...
=======SALSA SUMMARY=======
TOTAL MEMORY USAGE: 577.61MB
struct metadata = 29.00MB
struct fields = 35.68MB
memo metadata = 103.87MB
memo fields = 409.06MB
```
Eventually, we should integrate these numbers into CI in some form. The
one limitation currently is that heap allocations in salsa structs (e.g.
interned values) are not tracked, but memoized values should have full
coverage. We may also want a peak memory usage counter (that accounts
for non-salsa memory), but that is relatively simple to profile manually
(e.g. `time -v ty check`) and would require a compile-time option to
avoid runtime overhead.
Closes#18671
Note that while this has, I believe, always been invalid syntax, it was
reported as a different syntax error until Python 3.12:
Python 3.11:
```pycon
>>> x = 1
>>> f"{x! s}"
File "<stdin>", line 1
f"{x! s}"
^
SyntaxError: f-string: invalid conversion character: expected 's', 'r', or 'a'
```
Python 3.12:
```pycon
>>> x = 1
>>> f"{x! s}"
File "<stdin>", line 1
f"{x! s}"
^^^
SyntaxError: f-string: conversion type must come right after the exclamanation mark
```
## Summary
Garbage collect ASTs once we are done checking a given file. Queries
with a cross-file dependency on the AST will reparse the file on demand.
This reduces ty's peak memory usage by ~20-30%.
The primary change of this PR is adding a `node_index` field to every
AST node, that is assigned by the parser. `ParsedModule` can use this to
create a flat index of AST nodes any time the file is parsed (or
reparsed). This allows `AstNodeRef` to simply index into the current
instance of the `ParsedModule`, instead of storing a pointer directly.
The indices are somewhat hackily (using an atomic integer) assigned by
the `parsed_module` query instead of by the parser directly. Assigning
the indices in source-order in the (recursive) parser turns out to be
difficult, and collecting the nodes during semantic indexing is
impossible as `SemanticIndex` does not hold onto a specific
`ParsedModuleRef`, which the pointers in the flat AST are tied to. This
means that we have to do an extra AST traversal to assign and collect
the nodes into a flat index, but the small performance impact (~3% on
cold runs) seems worth it for the memory savings.
Part of https://github.com/astral-sh/ty/issues/214.
This PR implements template strings (t-strings) in the parser and
formatter for Ruff.
Minimal changes necessary to compile were made in other parts of the code (e.g. ty, the linter, etc.). These will be covered properly in follow-up PRs.
This is analogous to the existing `Tokens::after` method. Its
implementation is almost identical.
We plan to use this for looking at the tokens immediately before the
cursor when fetching completions.
Fixes#17867
## Summary
The CPython parser does not allow generator expressions which are the
sole arguments in an argument list to have a trailing comma.
With this change, we start flagging such instances.
## Test Plan
Added new inline tests.
## Summary
Part of #17412
Starred expressions cannot be used as values in assignment expressions.
Add a new semantic syntax error to catch such instances.
Note that we already have
`ParseErrorType::InvalidStarredExpressionUsage` to catch some starred
expression errors during parsing, but that does not cover top level
assignment expressions.
## Test Plan
- Added new inline tests for the new rule
- Found some examples marked as "valid" in existing tests (`_ = *data`),
which are not really valid (per this new rule) and updated them
- There was an existing inline test - `assign_stmt_invalid_value_expr`
which had instances of `*` expression which would be deemed invalid by
this new rule. Converted these to tuples, so that they do not trigger
this new rule.
## Summary
Part of #17412
Add a new compile-time syntax error for detecting `nonlocal`
declarations at a module level.
## Test Plan
- Added new inline tests for the syntax error
- Updated existing tests for `nonlocal` statement parsing to be inside a
function scope
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
While adding semantic error support to red-knot, I noticed duplicate
diagnostics for code like this:
```py
# error: [invalid-syntax] "cannot use an asynchronous comprehension outside of an asynchronous function on Python 3.9 (syntax was added in 3.11)"
# error: [invalid-syntax] "`asynchronous comprehension` outside of an asynchronous function"
[reveal_type(x) async for x in AsyncIterable()]
```
Beyond the duplication, the first error message doesn't make much sense
because this syntax is _not_ allowed on Python 3.11 either.
To fix this, this PR renames the
`async-comprehension-outside-async-function` semantic syntax error to
`async-comprehension-in-sync-comprehension` and fixes the rule to avoid
applying outside of sync comprehensions at all.
## Test Plan
New linter test demonstrating the false positive. The mdtests from my red-knot
PR also reflect this change.
Status
--
This is a pretty minor change, but it was breaking a red-knot mdtest
until #17463 landed. Now this should close#11934 as the last syntax
error being tracked there!
Summary
--
Moves `Parser::validate_parameters` to
`SemanticSyntaxChecker::duplicate_parameter_name`.
Test Plan
--
Existing tests, with `## Errors` replaced with `## Semantic Syntax
Errors`.
Summary
--
This PR extends semantic syntax error detection to red-knot. The main
changes here are:
1. Adding `SemanticSyntaxChecker` and `Vec<SemanticSyntaxError>` fields
to the `SemanticIndexBuilder`
2. Calling `SemanticSyntaxChecker::visit_stmt` and `visit_expr` in the
`SemanticIndexBuilder`'s `visit_stmt` and `visit_expr` methods
3. Implementing `SemanticSyntaxContext` for `SemanticIndexBuilder`
4. Adding new mdtests to test the context implementation and show
diagnostics
(3) is definitely the trickiest and required (I think) a minor addition
to the `SemanticIndexBuilder`. I tried to look around for existing code
performing the necessary checks, but I definitely could have missed
something or misused the existing code even when I found it.
There's still one TODO around `global` statement handling. I don't think
there's an existing way to look this up, but I'm happy to work on that
here or in a separate PR. This currently only affects detection of one
error (`LoadBeforeGlobalDeclaration` or
[PLE0118](https://docs.astral.sh/ruff/rules/load-before-global-declaration/)
in ruff), so it's not too big of a problem even if we leave the TODO.
Test Plan
--
New mdtests, as well as new errors for existing mdtests
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Bhuminjay Soni
Micha Reiser
Brent Westbrook
Dan Parizher
Bhuminjay Soni
Luca Chiodini
Ibraheem Ahmed
Dylan
Jack O'Connor
Andrew Gallant
Max Mynter
Abhijeet Prasad Bodas