<!--
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 refactors semantic error tests in each seperate file
## Test Plan
<!-- How was it tested? -->
## CC
- @ntBre
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## 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 unifies the two different ways Ruff and ty construct syntax
errors. Ruff has been storing the primary message in the diagnostic
itself, while ty attached the message to the primary annotation:
```
> ruff check try.py
invalid-syntax: name capture `x` makes remaining patterns unreachable
--> try.py:2:10
|
1 | match 42:
2 | case x: ...
| ^
3 | case y: ...
|
Found 1 error.
> uvx ty check try.py
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
Checking ------------------------------------------------------------ 1/1 files
error[invalid-syntax]
--> try.py:2:10
|
1 | match 42:
2 | case x: ...
| ^ name capture `x` makes remaining patterns unreachable
3 | case y: ...
|
Found 1 diagnostic
```
I think there are benefits to both approaches, and I do like ty's
version, but I feel like we should pick one (and it might help with
#20901 eventually). I slightly prefer Ruff's version, so I went with
that. Hopefully this isn't too controversial, but I'm happy to close
this if it is.
Note that this shouldn't change any other diagnostic formats in ty
because
[`Diagnostic::primary_message`](98d27c4128/crates/ruff_db/src/diagnostic/mod.rs (L177))
was already falling back to the primary annotation message if the
diagnostic message was empty. As a result, I think this change will
partially resolve the FIXME therein.
Test Plan
--
Existing tests with updated snapshots
Summary
--
Closes#19467 and also removes the warning about using Python 3.14
without
preview enabled.
I also bumped `PythonVersion::default` to 3.9 because it reaches EOL
this month,
but we could also defer that for now if we wanted.
The first three commits are related to the `latest` bump to 3.14; the
fourth commit
bumps the default to 3.10.
Note that this PR also bumps the default Python version for ty to 3.10
because
there was a test asserting that it stays in sync with
`ast::PythonVersion`.
Test Plan
--
Existing tests
I spot-checked the ecosystem report, and I believe these are all
expected. Inbits doesn't specify a target Python version, so I guess
we're applying the default. UP007, UP035, and UP045 all use the new
default value to emit new diagnostics.
- Convert panics to diagnostics with id `Panic`, severity `Fatal`, and
the error as the diagnostic message, annotated with a `Span` with empty
code block and no range.
- Updates the post-linting message diagnostic handling to track the
maximum severity seen, and then prints the "report a bug in ruff"
message only if the max severity was `Fatal`
This depends on the sorting changes since it creates diagnostics with no
range specified.
<!--
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
--
To take advantage of the new diagnostics, we need to update our caching
model to include all of the information supported by `ruff_db`'s
diagnostic type. Instead of trying to serialize all of this information,
Micha suggested simply not caching files with diagnostics, like we
already do for files with syntax errors. This PR is an attempt at that
approach.
This has the added benefit of trimming down our `Rule` derives since
this was the last place the `FromStr`/`strum_macros::EnumString`
implementation was used, as well as the (de)serialization macros and
`CacheKey`.
Test Plan
--
Existing tests, with their input updated not to include a diagnostic,
plus a new test showing that files with lint diagnostics are not cached.
Benchmarks
--
In addition to tests, we wanted to check that this doesn't degrade
performance too much. I posted part of this new analysis in
https://github.com/astral-sh/ruff/issues/18198#issuecomment-3175048672,
but I'll duplicate it here. In short, there's not much difference
between `main` and this branch for projects with few diagnostics
(`home-assistant`, `airflow`), as expected. The difference for projects
with many diagnostics (`cpython`) is quite a bit bigger (~300 ms vs ~220
ms), but most projects that run ruff regularly are likely to have very
few diagnostics, so this may not be a problem practically.
I guess GitHub isn't really rendering this as I intended, but the extra
separator line is meant to separate the benchmarks on `main` (above the
line) from this branch (below the line).
| Command | Mean [ms] | Min [ms] | Max [ms] |
|:--------------------------------------------------------------|----------:|---------:|---------:|
| `ruff check cpython --no-cache --isolated --exit-zero` | 322.0 | 317.5
| 326.2 |
| `ruff check cpython --isolated --exit-zero` | 217.3 | 209.8 | 237.9 |
| `ruff check home-assistant --no-cache --isolated --exit-zero` | 279.5
| 277.0 | 283.6 |
| `ruff check home-assistant --isolated --exit-zero` | 37.2 | 35.7 |
40.6 |
| `ruff check airflow --no-cache --isolated --exit-zero` | 133.1 | 130.4
| 146.4 |
| `ruff check airflow --isolated --exit-zero` | 34.7 | 32.9 | 41.6 |
|:--------------------------------------------------------------|----------:|---------:|---------:|
| `ruff check cpython --no-cache --isolated --exit-zero` | 330.1 | 324.5
| 333.6 |
| `ruff check cpython --isolated --exit-zero` | 309.2 | 306.1 | 314.7 |
| `ruff check home-assistant --no-cache --isolated --exit-zero` | 288.6
| 279.4 | 302.3 |
| `ruff check home-assistant --isolated --exit-zero` | 39.8 | 36.9 |
42.4 |
| `ruff check airflow --no-cache --isolated --exit-zero` | 134.5 | 131.3
| 140.6 |
| `ruff check airflow --isolated --exit-zero` | 39.1 | 37.2 | 44.3 |
I had Claude adapt one of the
[scripts](https://github.com/sharkdp/hyperfine/blob/master/scripts/plot_whisker.py)
from the hyperfine repo to make this plot, so it's not quite perfect,
but maybe it's still useful. The table is probably more reliable for
close comparisons. I'll put more details about the benchmarks below for
the sake of future reproducibility.
<img width="4472" height="2368" alt="image"
src="https://github.com/user-attachments/assets/1c42d13e-818a-44e7-b34c-247340a936d7"
/>
<details><summary>Benchmark details</summary>
<p>
The versions of each project:
- CPython: 6322edd260e8cad4b09636e05ddfb794a96a0451, the 3.10 branch
from the contributing docs
- `home-assistant`: 5585376b406f099fb29a970b160877b57e5efcb0
- `airflow`: 29a1cb0cfde9d99b1774571688ed86cb60123896
The last two are just the main branches at the time I cloned the repos.
I don't think our Ruff config should be applied since I used
`--isolated`, but these are cloned into my copy of Ruff at
`crates/ruff_linter/resources/test`, and I trimmed the
`./target/release/` prefix from each of the commands, but these are
builds of Ruff in release mode.
And here's the script with the `hyperfine` invocation:
```shell
#!/bin/bash
cargo build --release --bin ruff
# git clone --depth 1 https://github.com/home-assistant/core crates/ruff_linter/resources/test/home-assistant
# git clone --depth 1 https://github.com/apache/airflow crates/ruff_linter/resources/test/airflow
bin=./target/release/ruff
resources=./crates/ruff_linter/resources/test
cpython=$resources/cpython
home_assistant=$resources/home-assistant
airflow=$resources/airflow
base=${1:-bench}
hyperfine --warmup 10 --export-json $base.json --export-markdown $base.md \
"$bin check $cpython --no-cache --isolated --exit-zero" \
"$bin check $cpython --isolated --exit-zero" \
"$bin check $home_assistant --no-cache --isolated --exit-zero" \
"$bin check $home_assistant --isolated --exit-zero" \
"$bin check $airflow --no-cache --isolated --exit-zero" \
"$bin check $airflow --isolated --exit-zero"
```
I ran this once on `main` (`baseline` in the graph, top half of the
table) and once on this branch (`nocache` and bottom of the table).
</p>
</details>
Summary
--
I noticed while reviewing #19390 that in `check_tokens` we were still
passing
around an extra `LinterSettings`, despite all of the same functions also
receiving a `LintContext` with its own settings.
This PR adds the `LintContext::settings` method and calls that instead
of using
the separate `LinterSettings`.
Test Plan
--
Existing tests
Small rewording to indicate that core development is done but that we
may add breaking changes.
Feel free to bikeshed!
Test:
```console
❯ echo "t''" | cargo run -p ruff -- check --no-cache --isolated --target-version py314 -
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.13s
Running `target/debug/ruff check --no-cache --isolated --target-version py314 -`
warning: Support for Python 3.14 is in preview and may undergo breaking changes. Enable `preview` to remove this warning.
All checks passed!
```
## Summary
This PR addresses some additional feedback on #19053:
- Renaming the `syntax_error` methods to `invalid_syntax` to match the
lint id
- Moving the standalone `diagnostic_from_violation` function to
`Violation::into_diagnostic`
- Removing the `Ord` and `PartialOrd` implementations from `Diagnostic`
in favor of `Diagnostic::start_ordering`
## Test Plan
Existing tests
## Additional Follow-ups
Besides these, I also put the following comments on my todo list, but
they seemed like they might be big enough to have their own PRs:
- [Use `LintId::IOError` for IO
errors](https://github.com/astral-sh/ruff/pull/19053#discussion_r2189425922)
- [Move `Fix` and
`Edit`](https://github.com/astral-sh/ruff/pull/19053#discussion_r2189448647)
- [Avoid so many
unwraps](https://github.com/astral-sh/ruff/pull/19053#discussion_r2189465980)
## 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
I think this should be the last step before combining `OldDiagnostic`
and `ruff_db::Diagnostic`. We can't store a `NoqaCode` on
`ruff_db::Diagnostic`, so I converted the `noqa_code` field to an
`Option<String>` and then propagated this change to all of the callers.
I tried to use `&str` everywhere it was possible, so I think the
remaining `to_string` calls are necessary. I spent some time trying to
convert _everything_ to `&str` but ran into lifetime issues, especially
in the `FixTable`. Maybe we can take another look at that if it causes a
performance regression, but hopefully these paths aren't too hot. We
also avoid some `to_string` calls, so it might even out a bit too.
## Test Plan
Existing tests
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
This PR removes the last two places we were using `NoqaCode::rule` in
`linter.rs` (see
https://github.com/astral-sh/ruff/pull/18391#discussion_r2154637329 and
https://github.com/astral-sh/ruff/pull/18391#discussion_r2154649726) by
checking whether fixes are actually desired before adding them to a
`DiagnosticGuard`. I implemented this by storing a `Violation`'s `Rule`
on the `DiagnosticGuard` so that we could check if it was enabled in the
embedded `LinterSettings` when trying to set a fix.
All of the corresponding `set_fix` methods on `OldDiagnostic` were now
unused (except in tests where I just set `.fix` directly), so I moved
these to the guard instead of keeping both sets.
The very last place where we were using `NoqaCode::rule` was in the
cache. I just reverted this to parsing the `Rule` from the name. I had
forgotten to update the comment there anyway. Hopefully this doesn't
cause too much of a perf hit.
In terms of binary size, we're back down almost to where `main` was two
days ago
(https://github.com/astral-sh/ruff/pull/18391#discussion_r2155034320):
```
41,559,344 bytes for main 2 days ago
41,669,840 bytes for #18391
41,653,760 bytes for main now (after #18391 merged)
41,602,224 bytes for this branch
```
Only 43 kb up, but that shouldn't all be me this time :)
## Test Plan
Existing tests and benchmarks on this PR
## Summary
This PR avoids one of the three calls to `NoqaCode::rule` from
https://github.com/astral-sh/ruff/pull/18391 by applying per-file
ignores in the `LintContext`. To help with this, it also replaces all
direct uses of `LinterSettings.rules.enabled` with a
`LintContext::enabled` (or `Checker::enabled`, which defers to its
context) method. There are still some direct accesses to
`settings.rules`, but as far as I can tell these are not in a part of
the code where we can really access a `LintContext`. I believe all of
the code reachable from `check_path`, where the replaced per-file ignore
code was, should be converted to the new methods.
## Test Plan
Existing tests, with a single snapshot updated for RUF100, which I think
actually shows a more accurate diagnostic message now.
Summary
--
This PR unifies the remaining differences between `OldDiagnostic` and
`Message` (`OldDiagnostic` was only missing an optional `noqa_offset`
field) and
replaces `Message` with `OldDiagnostic`.
The biggest functional difference is that the combined `OldDiagnostic`
kind no
longer implements `AsRule` for an infallible conversion to `Rule`. This
was
pretty easy to work around with `is_some_and` and `is_none_or` in the
few places
it was needed. In `LintContext::report_diagnostic_if_enabled` we can
just use
the new `Violation::rule` method, which takes care of most cases.
Most of the interesting changes are in [this
range](8156992540)
before I started renaming.
Test Plan
--
Existing tests
Future Work
--
I think it's time to start shifting some of these fields to the new
`Diagnostic`
kind. I believe we want `Fix` for sure, but I'm less sure about the
others. We
may want to keep a thin wrapper type here anyway to implement a `rule`
method,
so we could leave some of these fields on that too.
## Summary
This PR avoids the `Vec::retain` call in `check_tokens` by checking if
rules are enabled as their diagnostics are constructed.
2a425e43fd/crates/ruff_linter/src/checkers/tokens.rs (L174-L176)
Since `LintContext::report_diagnostic_if_enabled` required a
`LinterSettings`, I added a `settings` field to the context itself
instead of trying to pass it everywhere. This also turned
`LogicalLinesContext` into a trivial wrapper around `LintContext`, so I
just removed it in favor of using `LintContext` directly too.
The diff is a bit smaller with whitespace hidden since many blocks got
moved into something like this:
```rust
if let Some(mut diagnostic) = context.report_diagnostic.enabled(...) {
// old code
}
```
## Test Plan
Existing tests
## Summary
As the title says, this PR removes the `Message::to_rule` method by
replacing related uses of `Rule` with `NoqaCode` (or the rule's name in
the case of the cache). Where it seemed a `Rule` was really needed, we
convert back to the `Rule` by parsing either the rule name (with
`str::parse`) or the `NoqaCode` (with `Rule::from_code`).
I thought this was kind of like cheating and that it might not resolve
this part of Micha's
[comment](https://github.com/astral-sh/ruff/pull/18391#issuecomment-2933764275):
> because we can't add Rule to Diagnostic or **have it anywhere in our
shared rendering logic**
but after looking again, the only remaining `Rule` conversion in
rendering code is for the SARIF output format. The other two non-test
`Rule` conversions are for caching and writing a fix summary, which I
don't think fall into the shared rendering logic. That leaves the SARIF
format as the only real problem, but maybe we can delay that for now.
The motivation here is that we won't be able to store a `Rule` on the
new `Diagnostic` type, but we should be able to store a `NoqaCode`,
likely as a string.
## Test Plan
Existing tests
##
[Benchmarks](https://codspeed.io/astral-sh/ruff/branches/brent%2Fremove-to-rule)
Almost no perf regression, only -1% on
`linter/default-rules[large/dataset.py]`.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
Summary
--
This is the last main difference between the `OldDiagnostic` and
`Message`
types, so attaching a `SourceFile` to `OldDiagnostic` should make
combining the
two types almost trivial.
Initially I updated the remaining rules without access to a `Checker` to
take a
`&SourceFile` directly, but after Micha's suggestion in
https://github.com/astral-sh/ruff/pull/18356#discussion_r2113281552, I
updated all of these calls to take a
`LintContext` instead. This new type is a thin wrapper around a
`RefCell<Vec<OldDiagnostic>>`
and a `SourceFile` and now has the `report_diagnostic` method returning
a `DiagnosticGuard` instead of `Checker`.
This allows the same `Drop`-based implementation to be used in cases
without a `Checker` and also avoids a lot of intermediate allocations of
`Vec<OldDiagnostic>`s.
`Checker` now also contains a `LintContext`, which it defers to for its
`report_diagnostic` methods, which I preserved for convenience.
Test Plan
--
Existing tests
Summary
--
It's a bit late in the refactoring process, but I think there are still
a couple of PRs left before getting rid of this type entirely, so I
thought it would still be worth doing.
This PR is just a quick rename with no other changes.
Test Plan
--
Existing tests
## Summary
This PR unifies the ruff `Message` enum variants for syntax errors and
rule violations into a single `Message` struct consisting of a shared
`db::Diagnostic` and some additional, optional fields used for some rule
violations.
This version of `Message` is nearly a drop-in replacement for
`ruff_diagnostics::Diagnostic`, which is the next step I have in mind
for the refactor.
I think this is also a useful checkpoint because we could possibly add
some of these optional fields to the new `Diagnostic` type. I think
we've previously discussed wanting support for `Fix`es, but the other
fields seem less relevant, so we may just need to preserve the `Message`
wrapper for a bit longer.
## Test plan
Existing tests
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
This PR deletes the `DiagnosticKind` type by inlining its three fields
(`name`, `body`, and `suggestion`) into three other diagnostic types:
`Diagnostic`, `DiagnosticMessage`, and `CacheMessage`.
Instead of deferring to an internal `DiagnosticKind`, both `Diagnostic`
and `DiagnosticMessage` now have their own macro-generated `AsRule`
implementations.
This should make both https://github.com/astral-sh/ruff/pull/18051 and
another follow-up PR changing the type of `name` on `CacheMessage`
easier since its type will be able to change separately from
`Diagnostic` and `DiagnosticMessage`.
## Test Plan
Existing tests
Re: #17526
## Summary
Add integration test for semantic syntax for `IrrefutableCasePattern`,
`SingleStarredAssignment`, `WriteToDebug`, and `InvalidExpression`.
## Notes
- Following @ntBre's suggestion, I will keep the test coming in batches
like this over the next few days in separate PRs to keep the review load
per PR manageable while also not spamming too many.
- I did not add a test for `del __debug__` which is one of the examples
in `crates/ruff_python_parser/src/semantic_errors.rs:1051`.
For python version `<= 3.8` there is no error and for `>=3.9` the error
is not `WriteToDebug` but `SyntaxError: cannot delete __debug__ on
Python 3.9 (syntax was removed in 3.9)`.
- The `blacken-docs` bypass is necessary because otherwise the test does
not pass pre-commit checks; but we want to check for this faulty syntax.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
This is a test.
## Summary
This PR partially addresses #16418 via the following:
- `LinterSettings::unresolved_python_version` is now a `TargetVersion`,
which is a thin wrapper around an `Option<PythonVersion>`
- `Checker::target_version` now calls `TargetVersion::linter_version`
internally, which in turn uses `unwrap_or_default` to preserve the
current default behavior
- Calls to the parser now call `TargetVersion::parser_version`, which
calls `unwrap_or_else(PythonVersion::latest)`
- The `Checker`'s implementation of
`SemanticSyntaxContext::python_version` also uses
`TargetVersion::parser_version` to use `PythonVersion::latest` for
semantic errors
In short, all lint rule behavior should be unchanged, but we default to
the latest Python version for the new syntax errors, which should
minimize confusing version-related syntax errors for users without a
version configured.
## Test Plan
Existing tests, which showed no changes (except for printing default
settings).
<!--
Thank you for contributing to Ruff! 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?
- Does this pull request include references to any relevant issues?
-->
Re: #17526
## Summary
Add test fixtures for `AwaitOutsideAsync` and
`AsyncComprehensionOutsideAsyncFunction` errors.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
This is a test.
<!-- How was it tested? -->
Re: #17526
## Summary
Add integration tests for Python Semantic Syntax for
`InvalidStarExpression`, `DuplicateMatchKey`, and
`DuplicateMatchClassAttribute`.
## Note
- Red knot integration tests for `DuplicateMatchKey` exist already in
line 89-101.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
This is a test.
<!-- How was it tested? -->
Re: #17526
## Summary
Adds tests to red knot and `linter.rs` for the semantic syntax.
Specifically add tests for `ReboundComprehensionVariable`,
`DuplicateTypeParameter`, and `MultipleCaseAssignment`.
Refactor the `test_async_comprehension_in_sync_comprehension` →
`test_semantic_error` to be more general for all semantic syntax test
cases.
## Test Plan
This is a test.
## Question
I'm happy to contribute more tests the coming days.
Should that happen here or should we merge this PR such that the
refactor `test_async_comprehension_in_sync_comprehension` →
`test_semantic_error` is available on main and others can chime in, too?
A small PR that just updates the various settings/configurations to
allow Python 3.14. At the moment selecting that target version will
have no impact compared to Python 3.13 - except that a warning
is emitted if the user does so with `preview` disabled.
Summary
--
This PR resolves https://github.com/astral-sh/ruff/issues/9761 by adding
a linter configuration option to disable
`typing_extensions` imports. As mentioned [here], it would be ideal if
we could
detect whether or not `typing_extensions` is available as a dependency
automatically, but this seems like a much easier fix in the meantime.
The default for the new option, `typing-extensions`, is `true`,
preserving the current behavior. Setting it to `false` will bail out of
the new
`Checker::typing_importer` method, which has been refactored from the
`Checker::import_from_typing` method in
https://github.com/astral-sh/ruff/pull/17340),
with `None`, which is then handled specially by each rule that calls it.
I considered some alternatives to a config option, such as checking if
`typing_extensions` has been imported or checking for a `TYPE_CHECKING`
block we could use, but I think defaulting to allowing
`typing_extensions` imports and allowing the user to disable this with
an option is both simple to implement and pretty intuitive.
[here]:
https://github.com/astral-sh/ruff/issues/9761#issuecomment-2790492853
Test Plan
--
New linter tests exercising several combinations of Python versions and
the new config option for PYI019. I also added tests for the other
affected rules, but only in the case where the new config option is
enabled. The rules' existing tests also cover the default case.
This PR collects all behavior gated under preview into a new module
`ruff_linter::preview` that exposes functions like
`is_my_new_feature_enabled` - just as is done in the formatter crate.
## 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.
Summary
--
This PR reimplements [yield-outside-function
(F704)](https://docs.astral.sh/ruff/rules/yield-outside-function/) as a
semantic syntax error. Despite the name, this rule covers `yield from`
and `await` in addition to `yield`.
Test Plan
--
New linter tests, along with the existing F704 test.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Summary
--
Detect async comprehensions nested in sync comprehensions in async
functions before Python 3.11, when this was [changed].
The actual logic of this rule is very straightforward, but properly
tracking the async scopes took a bit of work. An alternative to the
current approach is to offload the `in_async_context` check into the
`SemanticSyntaxContext` trait, but that actually required much more
extensive changes to the `TestContext` and also to ruff's semantic
model, as you can see in the changes up to
31554b473507034735bd410760fde6341d54a050. This version has the benefit
of mostly centralizing the state tracking in `SemanticSyntaxChecker`,
although there was some subtlety around deferred function body traversal
that made the changes to `Checker` more intrusive too (hence the new
linter test).
The `Checkpoint` struct/system is obviously overkill for now since it's
only tracking a single `bool`, but I thought it might be more useful
later.
[changed]: https://github.com/python/cpython/issues/77527
Test Plan
--
New inline tests and a new linter integration test.
## Summary
This PR implements the "greeter" approach for checking the AST for
syntax errors emitted by the CPython compiler. It introduces two main
infrastructural changes to support all of the compile-time errors:
1. Adds a new `semantic_errors` module to the parser crate with public
`SemanticSyntaxChecker` and `SemanticSyntaxError` types
2. Embeds a `SemanticSyntaxChecker` in the `ruff_linter::Checker` for
checking these errors in ruff
As a proof of concept, it also implements detection of two syntax
errors:
1. A reimplementation of
[`late-future-import`](https://docs.astral.sh/ruff/rules/late-future-import/)
(`F404`)
2. Detection of rebound comprehension iteration variables
(https://github.com/astral-sh/ruff/issues/14395)
## Test plan
Existing F404 tests, new inline tests in the `ruff_python_parser` crate,
and a linter CLI test showing an example of the `Message` output.
I also tested in VS Code, where `preview = false` and turning off syntax
errors both disable the new errors:
And on the playground, where `preview = false` also disables the errors:
Fixes#14395
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
The single flag `has_syntax_error` on `LinterResult` is replaced with
two (private) flags: `has_valid_syntax` and
`has_no_unsupported_syntax_errors`, which record whether there are
`ParseError`s or `UnsupportedSyntaxError`s, respectively. Only the
former is used to prevent a `FixAll` action.
An attempt has been made to make consistent the usage of the phrases
"valid syntax" (which seems to be used to refer only to _parser_ errors)
and "syntax error" (which refers to both _parser_ errors and
version-specific syntax errors).
Closes#16841
Summary
--
This PR updates `check_path` in the `ruff_linter` crate to return a
`Vec<Message>` instead of a `Vec<Diagnostic>`. The main motivation for
this is to make it easier to convert semantic syntax errors directly
into `Message`s rather than `Diagnostic`s in #16106. However, this also
has the benefit of keeping the preview check on unsupported syntax
errors in `check_path`, as suggested in
https://github.com/astral-sh/ruff/pull/16429#discussion_r1974748024.
All of the interesting changes are in the first commit. The second
commit just renames variables like `diagnostics` to `messages`, and the
third commit is a tiny import fix.
I also updated the `ExpandedMessage::location` field name, which caused
a few extra commits tidying up the playground code. I thought it was
nicely symmetric with `end_location`, but I'm happy to revert that too.
Test Plan
--
Existing tests. I also tested the playground and server manually.
Summary
--
This is a follow up addressing the comments on #16425. As @dhruvmanila
pointed out, the naming is a bit tricky. I went with `has_no_errors` to
try to differentiate it from `is_valid`. It actually ends up negated in
most uses, so it would be more convenient to have `has_any_errors` or
`has_errors`, but I thought it would sound too much like the opposite of
`is_valid` in that case. I'm definitely open to suggestions here.
Test Plan
--
Existing tests.
## Summary
This PR builds on the changes in #16220 to pass a target Python version
to the parser. It also adds the `Parser::unsupported_syntax_errors` field, which
collects version-related syntax errors while parsing. These syntax
errors are then turned into `Message`s in ruff (in preview mode).
This PR only detects one syntax error (`match` statement before Python
3.10), but it has been pretty quick to extend to several other simple
errors (see #16308 for example).
## Test Plan
The current tests are CLI tests in the linter crate, but these could be
supplemented with inline parser tests after #16357.
I also tested the display of these syntax errors in VS Code:
---------
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
## Summary
This PR is another step in preparing to detect syntax errors in the
parser. It introduces the new `per-file-target-version` top-level
configuration option, which holds a mapping of compiled glob patterns to
Python versions. I intend to use the
`LinterSettings::resolve_target_version` method here to pass to the
parser:
f50849aeef/crates/ruff_linter/src/linter.rs (L491-L493)
## Test Plan
I added two new CLI tests to show that the `per-file-target-version` is
respected in both the formatter and the linter.
Micha Reiser
Bhuminjay Soni
Brent Westbrook
Amethyst Reese
Dylan
Robsdedude
Max Mynter
Vasco Schiavo