Summary
--
Fixes#17181. The cases being tested with multiple *keys* being equal
are actually a slightly different error, more like the error for
`MatchMapping` than like the other multiple assignment errors:
```pycon
>>> match x:
... case Class(x=x, x=x): ...
...
File "<python-input-249>", line 2
case Class(x=x, x=x): ...
^
SyntaxError: attribute name repeated in class pattern: x
>>> match x:
... case {"x": 1, "x": 2}: ...
...
File "<python-input-251>", line 2
case {"x": 1, "x": 2}: ...
^^^^^^^^^^^^^^^^
SyntaxError: mapping pattern checks duplicate key ('x')
>>> match x:
... case [x, x]: ...
...
File "<python-input-252>", line 2
case [x, x]: ...
^
SyntaxError: multiple assignments to name 'x' in pattern
```
This PR just stops the false positive reported in the issue, but I will
quickly follow it up with a new rule (or possibly combined with the
mapping rule) catching the repeated attributes separately.
Test Plan
--
New inline `test_ok` and updating the `test_err` cases to have duplicate
values instead of keys.
## Summary
It turns out that `a.` isn't a list format supported by rustdoc. I
changed the documentation to use `1.`, `2.` instead.
## Test Plan
`cargo clippy`
This adds a new `Type` variant for holding an instance of a typevar
inside of a generic function or class. We don't handle specializing the
typevars yet, but this should implement most of the typing rules for
inside the generic function/class, where we don't know yet which
specific type the typevar will be specialized to.
This PR does _not_ yet handle the constraint that multiple occurrences
of the typevar must be specialized to the _same_ time. (There is an
existing test case for this in `generics/functions.md` which is still
marked as TODO.)
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Carl Meyer <carl@astral.sh>
<!--
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?
-->
## Summary
I decided to disable the new
[`needless_continue`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue)
rule because I often found the explicit `continue` more readable over an
empty block or having to invert the condition of an other branch.
## Test Plan
`cargo test`
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
This PR changes the inferred type for symbols in unreachable sections of
code to `Never` (instead of reporting them as unbound), in order to
silence false positive diagnostics. See the lengthy comment in the code
for further details.
## Test Plan
- Updated Markdown tests.
- Manually verified a couple of ecosystem diagnostic changes.
## Summary
If a package in `site-packages` had this directory structure:
```py
# bar/__init__.py
from .a import A
# bar/a.py
class A: ...
```
then we would fail to resolve the `from .a import A` import _if_ (as is
usually the case!) the `site-packages` search path was located inside a
`.venv` directory that was a subdirectory of the project's first-party
search path. The reason for this is a bug in `file_to_module` in the
module resolver. In this loop, we would identify that
`/project_root/.venv/lib/python3.13/site-packages/foo/__init__.py` can
be turned into a path relative to the first-party search path
(`/project_root`):
6e2b8f9696/crates/red_knot_python_semantic/src/module_resolver/resolver.rs (L101-L110)
but we'd then try to turn the relative path
(.venv/lib/python3.13/site-packages/foo/__init__.py`) into a module
path, realise that it wasn't a valid module path... and therefore
immediately `break` out of the loop before trying any other search paths
(such as the `site-packages` search path).
This bug was originally reported on Discord by @MatthewMckee4.
## Test Plan
I added a unit test for `file_to_module` in `resolver.rs`, and an
integration test that shows we can now resolve the import correctly in
`infer.rs`.
Summary
--
Detects duplicate literals in `match` mapping keys.
This PR also adds a `source` method to `SemanticSyntaxContext` to
display the duplicated key in the error message by slicing out its
range.
Test Plan
--
New inline tests.
Fixes#17164. Simply checking whether one type is gradually equivalent
to another is too simplistic here: `Any` is gradually equivalent to
`Todo`, but we should permit users to cast from `Todo` or `Unknown` to
`Any` without complaining about it. This changes our logic so that we
only complain about redundant casts if:
- the two types are exactly equal (when normalized) OR they are
equivalent (we'll still complain about `Any -> Any` casts, and about
`Any | str | int` -> `str | int | Any` casts, since their normalized
forms are exactly equal, even though the type is not fully static -- and
therefore does not participate in equivalence relations)
- AND the casted type does not contain `Todo`
We add support for `return` and `raise` statements in the control flow
graph: we simply add an edge to the terminal block, push the statements
to the current block, and proceed.
This implementation will have to be modified somewhat once we add
support for `try` statements - then we will need to check whether to
_defer_ the jump. But for now this will do!
Also in this PR: We fix the `unreachable` diagnostic range so that it
lumps together consecutive unreachable blocks.
## Summary
The existing signature for `str` calls had various problems, one of
which I noticed while looking at some ecosystem projects (`scrapy`,
added as a project to mypy_primer in this PR).
## Test Plan
- New tests for `str(…)` calls.
- Observed reduction of false positives in ecosystem checks
## Summary
A callable type is disjoint from other literal types. For example,
`Type::StringLiteral` must be an instance of exactly `str`, not a
subclass of `str`, and `str` is not callable. The same applies to other
literal types.
This should hopefully fix#17144, I couldn't produce any failures after
running property tests multiple times.
## Test Plan
Add test cases for disjointness check between callable and other literal
types.
Run property tests multiple times.
## Summary
Python `**` works differently to Rust `**`!
## Test Plan
Added an mdtest for various edge cases, and checked in the Python REPL
that we infer the correct type in all the new cases tested.
## Summary
This PR adds a CI job that causes GitHub to add annotations to a PR diff
when mdtest assertions fail. For example:
<details>
<summary>Screenshot</summary>
</details>
## Motivation
Debugging mdtest failures locally is currently a really nice experience:
- Errors are displayed with pretty colours, which makes them much more
readable
- If you run the test from inside an IDE, you can CTRL-click on a path
and jump directly to the line that had the failing assertion
- If you use
[`mdtest.py`](https://github.com/astral-sh/ruff/blob/main/crates/red_knot_python_semantic/mdtest.py),
you don't even need to recompile anything after changing an assertion in
an mdtest, amd the test results instantly live-update with each change
to the MarkDown file
Debugging mdtest failures in CI is much more unpleasant, however.
Sometimes an error message is just
> [static-assert-error] Argument evaluates to `False`
...which doesn't tell you very much unless you navigate to the line in
question that has the failing mdtest assertion. The line in question
might not even be touched by the PR, and even if it is, it can be hard
to find the line if the PR touches many files. Unlike locally, you can't
click on the error and jump straight to the line that contains the
failing assertion. You also don't get colourised output in CI
(https://github.com/astral-sh/ruff/issues/13939).
GitHub PR annotations should make it really easy to debug why mdtests
are failing on PRs, making PR review much easier.
## Test Plan
I opened a PR to my fork
[here](https://github.com/AlexWaygood/ruff/pull/11/files) with some
bogus changes to an mdtest to show what it looks like when there are
failures in CI and this job has been added. Scroll down to
`crates/red_knot_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md`
on the "files changed" tab for that PR to see the annotations.
## Summary
Fixes https://github.com/astral-sh/ruff/issues/17058.
Equivalent callable types were not understood as equivalent when they
appeared nested inside unions and intersections. This PR fixes that by
ensuring that `Callable` elements nested inside unions, intersections
and tuples have their representations normalized before one union type
is compared with another for equivalence, or before one intersection
type is compared with another for equivalence.
The normalizations applied to a `Callable` type are:
- the type of the default value is stripped from all parameters (only
whether the parameter _has_ a default value is relevant to whether one
`Callable` type is equivalent to another)
- The names of the parameters are stripped from positional-only
parameters, variadic parameters and keyword-variadic parameters
- Unions and intersections that are present (top-level or nested) inside
parameter annotations or return annotations are normalized.
Adding a `CallableType::normalized()` method also allows us to simplify
the implementation of `CallableType::is_equivalent_to()`.
### Should these normalizations be done eagerly as part of a
`CallableType` constructor?
I considered this. It's something that we could still consider doing in
the future; this PR doesn't rule it out as a possibility. However, I
didn't pursue it for now, for several reasons:
1. Our current `Display` implementation doesn't handle well the
possibility that a parameter might not have a name or an annotated type.
Callable types with parameters like this would be displayed as follows:
```py
(, ,) -> None: ...
```
That's fixable! It could easily become something like `(Unknown,
Unknown) -> None: ...`. But it also illustrates that we probably want to
retain the parameter names when displaying the signature of a `lambda`
function if you're hovering over a reference to the lambda in an IDE.
Currently we don't have a `LambdaType` struct for representing `lambda`
functions; if we wanted to eagerly normalize signatures when creating
`CallableType`s, we'd probably have to add a `LambdaType` struct so that
we would retain the full signature of a `lambda` function, rather than
representing it as an eagerly simplified `CallableType`.
2. In order to ensure that it's impossible to create `CallableType`s
without the parameters being normalized, I'd either have to create an
alternative `SimplifiedSignature` struct (which would duplicate a lot of
code), or move `CallableType` to a new module so that the only way of
constructing a `CallableType` instance would be via a constructor method
that performs the normalizations eagerly on the callable's signature.
Again, this isn't a dealbreaker, and I think it's still an option, but
it would be a lot of churn, and it didn't seem necessary for now. Doing
it this way, at least to start with, felt like it would create a diff
that's easier to review and felt like it would create fewer merge
conflicts for others.
## Test Plan
- Added a regression mdtest for
https://github.com/astral-sh/ruff/issues/17058
- Ran `QUICKCHECK_TESTS=1000000 cargo test --release -p
red_knot_python_semantic -- --ignored types::property_tests::stable`
## Summary
Add an initial set of tests that will eventually document our behavior
around unreachable code. In the last section of this suite, I argue why
we should never type check unreachable sections and never emit any
diagnostics in these sections.
## Summary
Following up the discussion in
https://github.com/astral-sh/ruff/issues/14626#issuecomment-2766548545,
we're to reorganize airflow rules. Before this discussion happens, we
combine required changes and suggested changes in to one single error
code.
This PR first rename the original error code to the new error code as we
discussed. We will gradually extract suggested changes out of AIR301 and
AIR302 to AIR311 and AIR312 in the following PRs
## Test Plan
Except for file, error code rename, the test case should work as it used
to be.
I initially split the lifetime out into three distinct lifetimes on
near-instinct because I moved the struct into the public API. But
because they are all shared borrows, and because there are no other APIs
on `DisplayDiagnostic` to access individual fields (and probably never
will be), it's probably fine to just specify one lifetime. Because of
subtyping, the one lifetime will be the shorter of the three.
There's also the point that `ruff_db` isn't _really_ a public API, since
it isn't a library that others depend on. So my instinct is probably a
bit off there.
## Summary
With this PR, we emit a diagnostic for this case where
previously didn't:
```py
from typing import Literal
def f(m: int, n: Literal[-1, 0, 1]):
# error: [division-by-zero] "Cannot divide object of type `int` by zero"
return m / n
```
## Test Plan
New Markdown test
## Summary
Add autofix infrastructure to `AIR302` name checks and use this logic to
fix`"airflow", "api_connexion", "security", "requires_access_dataset"`, `"airflow", "Dataset"` and `"airflow",
"datasets", "Dataset"`
## Test Plan
The existing test fixture reflects the update
## Summary
Closes#17112. Allows passing in string and list-of-strings literals
into `subprocess.run` (and related) calls without marking them as
untrusted input:
```py
import subprocess
subprocess.run("true")
# "instant" named expressions are also allowed
subprocess.run(c := "ls")
```
## Test Plan
Added test cases covering new behavior, passed with `cargo nextest run`.
## Summary
* ``airflow.auth.managers.base_auth_manager.is_authorized_dataset`` has
been moved to
``airflow.api_fastapi.auth.managers.base_auth_manager.is_authorized_asset``
in Airflow 3.0
* ``airflow.auth.managers.models.resource_details.DatasetDetails`` has
been moved to
``airflow.api_fastapi.auth.managers.models.resource_details.AssetDetails``
in Airflow 3.0
* Dag arguments `default_view` and `orientation` has been removed in
Airflow 3.0
* `airflow.models.baseoperatorlink.BaseOperatorLink` has been moved to
`airflow.sdk.definitions.baseoperatorlink.BaseOperatorLink` in Airflow
3.0
* ``airflow.notifications.basenotifier.BaseNotifier`` has been moved to
``airflow.sdk.BaseNotifier`` in Airflow 3.0
* ``airflow.utils.log.secrets_masker`` has been moved to
``airflow.sdk.execution_time.secrets_masker`` in Airflow 3.0
* ``airflow...DAG.allow_future_exec_dates`` has been removed in Airflow
3.0
* `airflow.utils.db.create_session` has een removed in Airflow 3.0
* `airflow.sensors.base_sensor_operator.BaseSensorOperator` has been
moved to `airflow.sdk.bases.sensor.BaseSensorOperator` removed Airflow
3.0
* `airflow.utils.file.TemporaryDirectory` has been removed in Airflow
3.0 and can be replaced by `tempfile.TemporaryDirectory`
* `airflow.utils.file.mkdirs` has been removed in Airflow 3.0 and can be
replaced by `pathlib.Path({path}).mkdir`
## Test Plan
Test fixture has been added for these changes
## Summary
Unlike other AIR3XX rules, this best practice can be applied to Airflow
1 and Airflow 2 as well. Thus, we think it might make sense for use to
move it to AIR002 so that the first number of the error align to Airflow
version as possible to reduce confusion
## Test Plan
the test fixture has been updated
It was already using this approach internally, so this is "just" a
matter of rejiggering the public API of `Diagnostic`.
We were previously writing directly to a `std::io::Write` since it was
thought that this worked better with the linear typing fakery. Namely,
it increased confidence that the diagnostic rendering was actually
written somewhere useful, instead of just being converted to a string
that could potentially get lost.
For reasons discussed in #17130, the linear type fakery was removed.
And so there is less of a reason to require a `std::io::Write`
implementation for diagnostic rendering. Indeed, this would sometimes
result in `unwrap()` calls when one wants to convert to a `String`.
## Summary
This PR adds Goto type definition to the playground, using the same
infrastructure as the LSP.
The main *challenge* with implementing this feature was that the editor
can now participate in which tab is open.
## Known limitations
The same as for the LSP. Most notably, navigating to types defined in
typeshed isn't supported.
## Test Plan
https://github.com/user-attachments/assets/22dad7c8-7ac7-463f-b066-5d5b2c45d1fe
This just adds an extra blank line. I think these tests were written
against the new renderer before it was used by Red Knot's `main`
function. Once I did that, I saw that it was missing a blank line, and
so I added it to match the status quo. But that means these snapshots
have become stale. So this commit updates them.
I put this in its own commit in case all of the information removed here
was controversial. But it *looks* stale to me. At the very least,
`TypeCheckDiagnostic` no longer exists, so that would need to be fixed.
And it doesn't really make sense to me (at this point) to make
`Diagnostic` a Salsa struct, particularly since we are keen on using it
in Ruff (at some point).
We do keep around `OldSecondaryDiagnosticMessage`, since that's part of
the Red Knot `InferContext` API. But it's a rather simple type, and
we'll be able to delete it entirely once `InferContext` exposes the new
`Diagnostic` type directly.
Since we aren't consuming `OldSecondaryDiagnosticMessage` any more, we
can now accept a slice instead of a vec. (Thanks Clippy.)
This replaces things like `TypeCheckDiagnostic` with the new Diagnostic`
type.
This is a "surgical" replacement where we retain the existing API of
of diagnostic reporting such that _most_ of Red Knot doesn't need to be
changed to support this update. But it will enable us to start using the
new diagnostic renderer and to delete the old renderer. It also paves
the path for exposing the new `Diagnostic` data model to the broader Red
Knot codebase.
Previously, this was only available in the old renderer.
To avoid regressions, we just copy it to the new renderer.
We don't bother with DRY because the old renderer will be
deleted very soon.
Now that we don't need to update the `printed` flag, this can just be an
immutable borrow.
(Arguably this should have been an immutable borrow even initially, but
I didn't want to introduce interior mutability without a more compelling
justification.)
The switch to `Arc` was done because Salsa sometimes requires cloning a
`Diagnostic` (or something that contains a `Diagnostic`). And so it
probably makes sense to make this cheap.
Since `Diagnostic` exposes a mutable API, we adopt "clone on write"
semantics. Although, it's more like, "clone on write when the `Arc` has
more than one reference." In the common case of creating a `Diagnostic`
and then immediately mutating it, no additional copies should be made
over the status quo.
We also drop the linear type fakery. Its interaction with Salsa is
somewhat awkward, and it has been suggested that there will be points
where diagnostics will be dropped unceremoniously without an opportunity
to tag them as having been ignored. Moreover, this machinery was added
out of "good sense" and isn't actually motivated by real world problems
with accidentally ignoring diagnostics. So that makes it easier, I
think, to just kick this out entirely instead of trying to find a way to
make it work.
This is temporary to scaffold the refactor.
The main idea is that we want to take the `InferContext` API,
*as it is*, and migrate that to the new diagnostic data model
*internally*. Then we can rip out the old stuff and iterate
on the API.
I did this mostly because it wasn't buying us much, and I'm
trying to simplify the public API of the types I'd like to
refactor in order to make the refactor simpler.
If we really want something like this, we can re-add it
later.
I removed this to see how much code was depending internally on the
`&[Arc<TypeCheckDiagnostic>]` representation. Thankfully, it was just
one place. So I just removed the `Deref` impl in favor of adding an
explicit `iter` method.
In general, I think using `Deref` for things like this is _somewhat_ of
an abuse. The tip-off is if there are `&self` or `&mut self` methods on
the type, then it's probably not a good candidate for `Deref`.
Summary
--
This PR reimplements
[load-before-global-declaration
(PLE0118)](https://docs.astral.sh/ruff/rules/load-before-global-declaration/)
as a semantic syntax error.
I added a `global` method to the `SemanticSyntaxContext` trait to make
this very easy, at least in ruff. Does red-knot have something similar?
If this approach will also work in red-knot, I think some of the other
PLE rules are also compile-time errors in CPython, PLE0117 in
particular. 0115 and 0116 also mention `SyntaxError`s in their docs, but
I haven't confirmed them in the REPL yet.
Test Plan
--
Existing linter tests for PLE0118. I think this actually can't be tested
very easily in an inline test because the `TestContext` doesn't have a
real way to track globals.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
Summary
--
Fixes https://github.com/astral-sh/ruff/issues/16520 by flagging single,
starred expressions in `return`, `yield`, and
`for` statements.
I thought `yield from` would also be included here, but that error is
emitted by
the CPython parser:
```pycon
>>> ast.parse("def f(): yield from *x")
Traceback (most recent call last):
File "<python-input-214>", line 1, in <module>
ast.parse("def f(): yield from *x")
~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.13/ast.py", line 54, in parse
return compile(source, filename, mode, flags,
_feature_version=feature_version, optimize=optimize)
File "<unknown>", line 1
def f(): yield from *x
^
SyntaxError: invalid syntax
```
And we also already catch it in our parser.
Test Plan
--
New inline tests and updates to existing tests.
## Summary
Implement basic *Goto type definition* support for Red Knot's LSP.
This PR also builds the foundation for other LSP operations. E.g., Goto
definition, hover, etc., should be able to reuse some, if not most,
logic introduced in this PR.
The basic steps of resolving the type definitions are:
1. Find the closest token for the cursor offset. This is a bit more
subtle than I first anticipated because the cursor could be positioned
right between the callee and the `(` in `call(test)`, in which case we
want to resolve the type for `call`.
2. Find the node with the minimal range that fully encloses the token
found in 1. I somewhat suspect that 1 and 2 could be done at the same
time but it complicated things because we also need to compute the spine
(ancestor chain) for the node and there's no guarantee that the found
nodes have the same ancestors
3. Reduce the node found in 2. to a node that is a valid goto target.
This may require traversing upwards to e.g. find the closest expression.
4. Resolve the type for the goto target
5. Resolve the location for the type, return it to the LSP
## Design decisions
The current implementation navigates to the inferred type. I think this
is what we want because it means that it correctly accounts for
narrowing (in which case we want to go to the narrowed type because
that's the value's type at the given position). However, it does have
the downside that Goto type definition doesn't work whenever we infer `T
& Unknown` because intersection types aren't supported. I'm not sure
what to do about this specific case, other than maybe ignoring `Unkown`
in Goto type definition if the type is an intersection?
## Known limitations
* Types defined in the vendored typeshed aren't supported because the
client can't open files from the red knot binary (we can either
implement our own file protocol and handler OR extract the typeshed
files and point there). See
https://github.com/astral-sh/ruff/issues/17041
* Red Knot only exposes an API to get types for expressions and
definitions. However, there are many other nodes with identifiers that
can have a type (e.g. go to type of a globals statement, match patterns,
...). We can add support for those in separate PRs (after we figure out
how to query the types from the semantic model). See
https://github.com/astral-sh/ruff/issues/17113
* We should have a higher-level API for the LSP that doesn't directly
call semantic queries. I intentionally decided not to design that API
just yet.
## Test plan
https://github.com/user-attachments/assets/fa077297-a42d-4ec8-b71f-90c0802b4edb
Goto type definition on a union
<img width="1215" alt="Screenshot 2025-04-01 at 13 02 55"
src="https://github.com/user-attachments/assets/689cabcc-4a86-4a18-b14a-c56f56868085"
/>
Note: I recorded this using a custom typeshed path so that navigating to
builtins works.
<!--
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?
-->
## Summary
from https://github.com/astral-sh/ruff/pull/17034#discussion_r2024222525
This is a simple PR to fix the invalid behavior of `NotImplemented` on
Python >=3.10.
## Test Plan
I think it would be better if we could run mdtest across multiple Python
versions in GitHub Actions.
<!-- How was it tested? -->
---------
Co-authored-by: David Peter <sharkdp@users.noreply.github.com>
## Summary
Add support for decorators on function as well as support
for properties by adding special handling for `@property` and `@<name of
property>.setter`/`.getter` decorators.
closes https://github.com/astral-sh/ruff/issues/16987
## Ecosystem results
- ✔️ A lot of false positives are fixed by our new
understanding of properties
- 🔴 A bunch of new false positives (typically
`possibly-unbound-attribute` or `invalid-argument-type`) occur because
we currently do not perform type narrowing on attributes. And with the
new understanding of properties, this becomes even more relevant. In
many cases, the narrowing occurs through an assertion, so this is also
something that we need to implement to get rid of these false positives.
- 🔴 A few new false positives occur because we do not
understand generics, and therefore some calls to custom setters fail.
- 🔴 Similarly, some false positives occur because we do not
understand protocols yet.
- ✔️ Seems like a true positive to me. [The
setter](e624d8edfa/src/packaging/specifiers.py (L752-L754))
only accepts `bools`, but `None` is assigned in [this
line](e624d8edfa/tests/test_specifiers.py (L688)).
```
+ error[lint:invalid-assignment]
/tmp/mypy_primer/projects/packaging/tests/test_specifiers.py:688:9:
Invalid assignment to data descriptor attribute `prereleases` on type
`SpecifierSet` with custom `__set__` method
```
- ✔️ This is arguable also a true positive. The setter
[here](0c6c75644f/rich/table.py (L359-L363))
returns `Table`, but typeshed wants [setters to return
`None`](bf8d2a9912/stdlib/builtins.pyi (L1298)).
```
+ error[lint:invalid-argument-type]
/tmp/mypy_primer/projects/rich/rich/table.py:359:5: Object of type
`Literal[padding]` cannot be assigned to parameter 2 (`fset`) of bound
method `setter`; expected type `(Any, Any, /) -> None`
```
## Follow ups
- Fix the `@no_type_check` regression
- Implement class decorators
## Test Plan
New Markdown test suites for decorators and properties.
## Summary
Adds import `numpy.typing as npt` to `default in
flake8-import-conventions.aliases`
Resolves#17028
## Test Plan
Manually ran local ruff on the altered fixture and also ran `cargo test`
## Summary
Part of #15382, this PR adds property tests for callable types.
Specifically, this PR updates the property tests to generate an
arbitrary signature for a general callable type which includes:
* Arbitrary combination of parameter kinds in the correct order
* Arbitrary number of parameters
* Arbitrary optional types for annotation and return type
* Arbitrary parameter names (no duplicate names), optional for
positional-only parameters
## Test Plan
```
QUICKCHECK_TESTS=100000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable
```
Also, the commands in CI:
d72b4100a3/.github/workflows/daily_property_tests.yaml (L47-L52)
## Summary
Part of #15382, this PR adds support for disjointness between two
callable types. They are never disjoint because there exists a callable
type that's a subtype of all other callable types:
```py
(*args: object, **kwargs: object) -> Never
```
The `Never` is a subtype of every fully static type thus a callable type
that has the return type of `Never` means that it is a subtype of every
return type.
## Test Plan
Add test cases related to mixed parameter kinds, gradual form (`...`)
and `Never` type.
## Summary
Currently our `Type::Callable` wraps a four-variant `CallableType` enum.
But as time has gone on, I think we've found that the four variants in
`CallableType` are really more different to each other than they are
similar to each other:
- `GeneralCallableType` is a structural type describing all callable
types with a certain signature, but the other three types are "literal
types", more similar to the `FunctionLiteral` variant
- `GeneralCallableType` is not a singleton or a single-valued type, but
the other three are all single-valued types
(`WrapperDescriptorDunderGet` is even a singleton type)
- `GeneralCallableType` has (or should have) ambiguous truthiness, but
all possible inhabitants of the other three types are always truthy.
- As a structural type, `GeneralCallableType` can contain inner unions
and intersections that must be sorted in some contexts in our internal
model, but this is not true for the other three variants.
This PR flattens `Type::Callable` into four distinct `Type::` variants.
In the process, it fixes a number of latent bugs that were concealed by
the current architecture but are laid bare by the refactor. Unit tests
for these bugs are included in the PR.
## Summary
Currently if I run `uv run crates/red_knot_python_semantic/mdtest.py`
from the Ruff repo root, I get this output:
```
~/dev/ruff (main)⚡ % uv run crates/red_knot_python_semantic/mdtest.py
Ready to watch for changes...
```
...And I then have to make some spurious whitespace changes or something
to a test file in order to get the script to actually run mdtest. This
PR changes mdtest.py so that it does an initial run of all mdtests when
you invoke the script, and _then_ starts watching for changes in test
files/Rust code.
## Summary
This PR fixes a bug in callable subtyping to consider both the
positional and keyword form of the standard parameter in the supertype
when matching against variadic, keyword-only and keyword-variadic
parameter in the subtype.
This is done by collecting the unmatched standard parameters and then
checking them against the keyword-only / keyword-variadic parameters
after the positional loop.
## Test Plan
Add test cases.
## Summary
There are quite a few places we infer `Todo` types currently, and some
of them are nested somewhat deeply in type expressions. These can cause
spurious issues for the new `redundant-cast` diagnostics. We fixed all
the false positives we saw in the mypy_primer report before merging
https://github.com/astral-sh/ruff/pull/17100, but I think there are
still lots of places where we'd emit false positives due to this check
-- we currently don't run on that many projects at all in our
mypy_primer check:
d0c8eaa092/.github/workflows/mypy_primer.yaml (L71)
This PR fixes some more false positives from this diagnostic by making
the `Type::contains_todo()` method more expansive.
## Test Plan
I added a regression test which causes us to emit a spurious diagnostic
on `main`, but does not with this PR.
## Summary
In https://github.com/python/typeshed/pull/13520 the typeshed definition
of `typing.Any` was changed from `Any = object()` to `class Any: ...`.
Our automated typeshed updater pulled down this change in
https://github.com/astral-sh/ruff/pull/17106, with the consequence that
we no longer understand `Any`, which is... not good.
This PR gives us the ability to understand `Any` defined as a class
instead of `object()`. It doesn't remove our ability to understand the
old form. Perhaps at some point we'll want to remove it, but for now we
may as well support both old and new typeshed?
This also directly patches typeshed to use the new form of `Any`; this
is purely to work around our tests that no known class is inferred as
`Unknown`, which otherwise fail with the old typeshed and the changes in
this PR. (All other tests pass.) This patch to typeshed will shortly be
subsumed by https://github.com/astral-sh/ruff/pull/17106 anyway.
## Test Plan
Without the typeshed change in this PR, all tests pass except for the
two `known_class_doesnt_fallback_to_unknown_unexpectedly_*` tests (so we
still support the old form of defining `Any`). With the typeshed change
in this PR, all tests pass, so we now support the new form in a way that
is indistinguishable to our test suite from the old form. And
indistinguishable to the ecosystem check: after rebasing
https://github.com/astral-sh/ruff/pull/17106 on this PR, there's zero
ecosystem impact.
## Summary
I don't remember exactly when we made `Identifier` a node but it is now
considered a node (it implements `AnyNodeRef`, it has a range). However,
we never updated
the `SourceOrderVisitor` to visit identifiers because we never had a use
case for it and visiting new nodes can change how the formatter
associates comments (breaking change!).
This PR updates the `SourceOrderVisitor` to visit identifiers and
changes the formatter comment visitor to skip identifiers (updating the
visitor might be desired because it could help simplifying some comment
placement logic but this is out of scope for this PR).
## Test Plan
Tests, updated snapshot tests
## Summary
I noticed we were inferring `Todo` as the declared type for annotations
such as `x: tuple[list[int], list[int]]`. This PR reworks our annotation
parsing so that we instead infer `tuple[Todo, Todo]` for this
annotation, which is quite a bit more precise.
## Test Plan
Existing mdtest updated.
<!--
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?
-->
Closes#16903
## Summary
Check if the current working directory exist. If not, provide an error
instead of panicking.
Fixed a stale comment in `resolve_default_files`.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
I added a test to the `resolve_files.rs`.
Manual testing follows steps of #16903 :
- Terminal 1
```bash
mkdir tmp
cd tmp
```
- Terminal 2
```bash
rm -rf tmp
```
- Terminal 1
```bash
ruff check
```
## Open Issues / Questions to Reviewer
All tests pass when executed with `cargo nextest run`.
However, with `cargo test` the parallelization makes the other tests
fail as we change the `pwd`.
Serial execution with `cargo test` seems to require [another dependency
or some
workarounds](https://stackoverflow.com/questions/51694017/how-can-i-avoid-running-some-tests-in-parallel).
Do you think an additional dependency or test complexity is worth
testing this small edge case, do you have another implementation idea,
or should i rather remove the test?
---
P.S.: I'm currently participating in a batch at the [Recurse
Center](https://www.recurse.com/) and would love to contribute more for
the next six weeks to improve my Rust. Let me know if you're open to
mentoring/reviewing and/or if you have specific areas where help would
be most valued.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
This PR contains the scaffolding for a new control flow graph
implementation, along with its application to the `unreachable` rule. At
the moment, the implementation is a maximal over-approximation: no
control flow is modeled and all statements are counted as reachable.
With each additional statement type we support, this approximation will
improve.
So this PR just contains:
- A `ControlFlowGraph` struct and builder
- Support for printing the flow graph as a Mermaid graph
- Snapshot tests for the actual graphs
- (a very bad!) reimplementation of `unreachable` using the new structs
- Snapshot tests for `unreachable`
# Instructions for Viewing Mermaid snapshots
Unfortunately I don't know how to convince GitHub to render the Mermaid
graphs in the snapshots. However, you can view these locally in VSCode
if you install an extension that supports Mermaid graphs in Markdown,
and then add this to your `settings.json`:
```json
"files.associations": {
"*.md.snap": "markdown",
}
```
## Summary
A few smaller editor improvements that felt worth pulling out of my
other feature PRs:
* Load the `Editor` lazily: This allows splitting the entire monaco
javascript into a separate async bundle, drastically reducing the size
of the `index.js`
* Fix the name of `to_range` and `text_range` to the more idiomatic js
names `toRange` and `textRange`
* Use one indexed values for `Position::line` and `Position::column`,
which is the same as monaco (reduces the need for `+1` and `-1`
operations spread all over the place)
* Preserve the editor state when navigating between tabs. This ensures
that selections are preserved even when switching between tabs.
* Stop the default handling of the `Enter` key press event when renaming
a file because it resulted in adding a newline in the editor
## Summary
This PR adds a new but so far empty and unused `red_knot_ide` crate.
This new crate's purpose is to implement IDE-specific functionality,
such as go to definition, hover, completion, etc., which are used by
both the LSP and the playground.
The crate itself doesn't depend on `lsptypes`. The idea is that the
facade crates (e.g., `red_knot_server`) convert external to internal
types.
Not only allows this to share the logic between server and playground,
it also ensures that the core functionality is easier to test because it
can be tested without needing a full LSP.
## Test Plan
`cargo build`
## Summary
Following up from earlier discussion on Discord, this PR adds logic to
flag casts as redundant when the inferred type of the expression is the
same as the target type. It should follow the semantics from
[mypy](https://github.com/python/mypy/pull/1705).
Example:
```python
def f() -> int:
return 10
# error: [redundant-cast] "Value is already of type `int`"
cast(int, f())
```
## Summary
Part of #13694
Seems there a bit more to cover regarding `in` and other types, but i
can cover them in different PRs
## Test Plan
Add `in.md` file in narrowing conditionals folder
---------
Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary
Rewrites the virtual env discovery to:
* Only use of `System` APIs, this ensures that the discovery will also
work when using a memory file system (testing or WASM)
* Don't traverse ancestor directories. We're not convinced that this is
necessary. Let's wait until someone shows us a use case where it is
needed
* Start from the project root and not from the current working
directory. This ensures that Red Knot picks up the right venv even when
using `knot --project ../other-dir`
## Test Plan
Existing tests, @ntBre tested that the `file_watching` tests no longer
pick up his virtual env in a parent directory
## Summary
In preparation for #17017, where we will need them to suppress new false
positives (once we understand the `ParamSpec.args`/`ParamSpec.kwargs`
properties).
## Test Plan
Tested on branch #17017
<!--
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?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
* Combine AIR302 matches
* Found a few errors. Will be fixed in another PR
## Test Plan
<!-- How was it tested? -->
This PR does not change anything. The existing testing fixture should
work as it used to be
## Summary
Disallow empty `todo_type!()`s without a custom message. They can lead
to spurious diffs in `mypy_primer` where the only thing that's changed
is the file/line information.
<!--
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?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
* The following paths are wrong
* `airflow.providers.amazon.auth_manager.avp.entities` should be
`airflow.providers.amazon.aws.auth_manager.avp.entities`
* `["airflow", "datasets", "manager", "dataset_manager"]` should be
fixed as `airflow.assets.manager` but not
`airflow.assets.manager.asset_manager`
* `["airflow", "datasets.manager", "DatasetManager"]` should be `
["airflow", "datasets", "manager", "DatasetManager"]` instead
## Test Plan
<!-- How was it tested? -->
the test fixture is updated accordingly
<!--
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?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Improve AIR302 test cases
## Test Plan
<!-- How was it tested? -->
test fixtures have been updated accordingly
## Summary
A quick fix for how union/intersection member search ins performed in
Knot.
## Test Plan
* Added a dunder method call test for Union, which exhibits the error
* Also added an intersection error, but it is not triggering currently
due to `call` logic not being fully implemented for intersections.
---------
Co-authored-by: David Peter <mail@david-peter.de>
## Summary
Closes#16661
This PR includes two changes:
- `NotImplementedType` is now a member of `KnownClass`
- We skip `is_assignable_to` checks for `NotImplemented` when checking
return types
### Limitation
```py
def f(cond: bool) -> int:
return 1 if cond else NotImplemented
```
The implementation covers cases where `NotImplemented` appears inside a
`Union`.
However, for more complex types (ex. `Intersection`) it will not worked.
In my opinion, supporting such complexity is unnecessary at this point.
## Test Plan
Two `mdtest` files were updated:
- `mdtest/function/return_type.md`
- `mdtest/type_properties/is_singleton.md`
To test `KnownClass`, run:
```bash
cargo test -p red_knot_python_semantic -- types::class::
```
Summary
--
Detects starred assignment targets outside of tuples and lists like `*a
= (1,)`.
This PR only considers assignment statements. I also checked annotated
assigment statements, but these give a separate error that we already
catch, so I think they're okay not to consider:
```pycon
>>> *a: list[int] = []
File "<python-input-72>", line 1
*a: list[int] = []
^
SyntaxError: invalid syntax
```
Fixes#13759
Test Plan
--
New inline tests, plus a new `SemanticSyntaxError` for an existing
parser test. I also removed a now-invalid case from an otherwise-valid
test fixture.
The new semantic error leads to two errors for the case below:
```python
*foo() = 42
```
but this matches [pyright] too.
[pyright]: https://pyright-play.net/?code=FQMw9mAUCUAEC8sAsAmAUEA
Summary
--
Detect setting or deleting `__debug__`. Assigning to `__debug__` was a
`SyntaxError` on the earliest version I tested (3.8). Deleting
`__debug__` was made a `SyntaxError` in [BPO 45000], which said it was
resolved in Python 3.10. However, `del __debug__` was also a runtime
error (`NameError`) when I tested in Python 3.9.6, so I thought it was
worth including 3.9 in this check.
I don't think it was ever a *good* idea to try `del __debug__`, so I
think there's also an argument for not making this version-dependent at
all. That would only simplify the implementation very slightly, though.
[BPO 45000]: https://github.com/python/cpython/issues/89163
Test Plan
--
New inline tests. This also required adding a `PythonVersion` field to
the `TestContext` that could be taken from the inline `ParseOptions` and
making the version field on the options accessible.
## Summary
This PR adds `as_<group>` methods to `AnyNodeRef` to e.g. convert an
`AnyNodeRef` to an `ExprRef`.
I need this for go to definition where the fallback is to test if
`AnyNodeRef` is an expression and then call `inferred_type` (listing
this mapping at every call site where we need to convert `AnyNodeRef` to
an `ExprRef` is a bit painful ;))
Split out from https://github.com/astral-sh/ruff/pull/16901
## Test Plan
`cargo test`
## Summary
We renamed the `PreorderVisitor` to `SourceOrderVisitor` a long time ago
but it seems that we missed to rename the `visit_preorder` functions to
`visit_source_order`.
This PR renames `visit_preorder` to `visit_source_order`
## Test Plan
`cargo test`
## Summary
This PR refactors the common logic for unpacking in assignment, for loops, and with items.
## Test Plan
Make sure existing tests pass.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
<!--
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?
-->
## Summary
Fixes#16744
Code from
bbf4f830b5/crates/uv-python/src/virtualenv.rs (L124-L144)
## Test Plan
Manual testing
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
From #16861, and the continuation of #16915.
This PR fixes the incorrect behavior of
`TypeInferenceBuilder::infer_name_load` in eager nested scopes.
And this PR closes#16341.
## Test Plan
New test cases are added in `annotations/deferred.md`.
## Summary
Part of #13694
Narrow in or-patterns by taking the type union of the type constraints
in each disjunct pattern.
## Test Plan
Add new tests to narrow/match.md
---------
Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary
Part of #13694
The implementation here was suspiciously straightforward so please lmk
if I missed something
Also some drive-by changes to DRY things up a bit
## Test Plan
Add new tests to narrow/match.md
---------
Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary
This PR adds some branches so that we infer `Todo` types for attribute
access on instances of `super()` and subtypes of `type[Enum]`. It reduces
false positives in the short term until we implement full support for
these features.
## Test Plan
New mdtests added + mypy_primer report
## Summary
Fixes https://github.com/astral-sh/ruff/issues/17018
## Test Plan
I renamed a python file to `knot.toml` and verified that there are no
diagnostics. Renaming back the file to `*.py` brings back the
diagnostics
## Summary
Resolves#16950 and [a 1.5-year-old TODO
comment](8d16a5c8c9/crates/ruff/src/diagnostics.rs (L380)).
After this change, a `pyproject.toml` will be linted the same as any
Python files would when passed via stdin.
## Test Plan
Integration tests.
## Summary
Mainly for partially fixing #16953
## Test Plan
Update is_subtype tests. And should maybe do these checks for many other
types (is subtype of object but object is not subtype)
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Summary
--
This PR detects multiple assignments to the same name in `case` patterns
by recursively visiting each pattern.
Test Plan
--
New inline tests.
Summary
--
Detects irrefutable `match` cases before the final case using a modified
version
of the existing `Pattern::is_irrefutable` method from the AST crate. The
modified method helps to retrieve a more precise diagnostic range to
match what
Python 3.13 shows in the REPL.
Test Plan
--
New inline tests, as well as some updates to existing tests that had
irrefutable
patterns before the last block.
## Summary
`std::time::now` isn't available on `wasm32-unknown-unknown` but it is
used by `FileTime::now`.
This PR replaces the usages of `FileTime::now` with a target specific
helper function that we already had in the memory file system.
Fixes https://github.com/astral-sh/ruff/issues/16966
## Test Plan
Tested that the playground no longer crash when adding an extra-path
## Summary
Further work towards https://github.com/astral-sh/ruff/issues/14169.
We currently panic on encountering cyclic `*` imports. This is easily
fixed using fixpoint iteration.
## Test Plan
Added a test that panics on `main`, but passes with this PR
## Summary
As mentioned in
https://github.com/astral-sh/ruff/pull/16698#discussion_r2004920075,
part of #15382, this PR updates the `is_gradual_equivalent_to`
implementation between callable types to be similar to
`is_equivalent_to` and checks other attributes of parameters like name,
optionality, and parameter kind.
## Test Plan
Expand the existing test cases to consider other properties but not all
similar to how the tests are structured for subtyping and assignability.
## Summary
This PR adds initial support for `*` imports to red-knot. The approach
is to implement a standalone query, called from semantic indexing, that
visits the module referenced by the `*` import and collects all
global-scope public names that will be imported by the `*` import. The
`SemanticIndexBuilder` then adds separate definitions for each of these
names, all keyed to the same `ast::Alias` node that represents the `*`
import.
There are many pieces of `*`-import semantics that are still yet to be
done, even with this PR:
- This PR does not attempt to implement any of the semantics to do with
`__all__`. (If a module defines `__all__`, then only the symbols
included in `__all__` are imported, _not_ all public global-scope
symbols.
- With the logic implemented in this PR as it currently stands, we
sometimes incorrectly consider a symbol bound even though it is defined
in a branch that is statically known to be dead code, e.g. (assuming the
target Python version is set to 3.11):
```py
# a.py
import sys
if sys.version_info < (3, 10):
class Foo: ...
```
```py
# b.py
from a import *
print(Foo) # this is unbound at runtime on 3.11,
# but we currently consider it bound with the logic in this PR
```
Implementing these features is important, but is for now deferred to
followup PRs.
Many thanks to @ntBre, who contributed to this PR in a pairing session
on Friday!
## Test Plan
Assertions in existing mdtests are adjusted, and several new ones are
added.
<!--
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?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
combine similar case condition in AIR302
## Test Plan
<!-- How was it tested? -->
nothing should be changed. existing test case should already cover it
Summary
--
Fixes#16943 by checking if the tuple is not parenthesized before
emitting an error.
Test Plan
--
New inline test based on the initial report
## Summary
Here I fix the last English spelling errors I could find in the repo.
Again, I am trying not to touch variable/function names, or anything
that might be misspelled in the API. The goal is to make this PR safe
and easy to merge.
## Test Plan
I have run all the unit tests. Though, again, all of the changes I make
here are to docs and docstrings. I make no code changes, which I believe
should greatly mitigate the testing concerns.
## Summary
Resolves#16895.
`abstractmethod` is now a `KnownFunction`. When a function is decorated
by `abstractmethod` or when the parent class inherits directly from
`Protocol`, `invalid-return-type` won't be emitted for that function.
## Test Plan
Markdown tests.
---------
Co-authored-by: Carl Meyer <carl@oddbird.net>
## Summary
Fixes#16912
Create a new type `DisplayMaybeParenthesizedType` that is now used in
Union and Intersection display
## Test Plan
Update callable annotations
## Summary
From #16861
This PR fixes the incorrect `ClassDef` handling of
`SemanticIndexBuilder::visit_stmt`, which fixes some of the incorrect
behavior of referencing the class itself in the class scope (a complete
fix requires a different fix, which will be done in the another PR).
---------
Co-authored-by: Carl Meyer <carl@astral.sh>
<!--
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?
-->
## Summary
This is a cleanup PR. I am fixing various English language spelling
errors. This is mostly in docs and docstrings.
## Test Plan
The usual CI tests were run. I tried to build the docs (though I had
some troubles there). The testing needs here are, I trust, very low
impact. (Though I would happily test more.)
## Summary
Log the origin of the sys path prefix. This should help with debugging
if someone doesn't understand
why Red Knot picks up a certain venv.
## Test Plan
Ran the CLI and tested that it logs the origin
## Summary
Part of #15382, this PR adds support for calling a variable that's
annotated with `typing.Callable`.
## Test Plan
Add test cases in a new `call/annotation.md` file.
## Summary
Part of #15382
This PR adds support for checking the assignability of two general
callable types.
This is built on top of #16804 by including the gradual parameters check
and accepting a function that performs the check between the two types.
## Test Plan
Update `is_assignable_to.md` with callable types section.
<!--
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?
-->
## Summary
When callables are displayed in unions, like:
```py
from typing import Callable
def foo(x: Callable[[], int] | None):
# red-knot: Revealed type is `() -> int | None` [revealed-type]
reveal_type(x)
```
This leaves the type rather ambiguous, to fix this we can add
parenthesis to callable type in union
Fixes#16893
## Test Plan
Update callable annotations tests
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
<!--
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?
-->
## Summary
Fixes#16898
## Test Plan
Update test for lang mismatch panic
Summary
--
Detects duplicate type parameter names in function definitions, class
definitions, and type alias statements.
I also boxed the `type_params` field on `StmtTypeAlias` to make it
easier to
`match` with functions and classes. (That's the reason for the red-knot
code
owner review requests, sorry!)
Test Plan
--
New `ruff_python_syntax_errors` unit tests.
Fixes#11119.
## 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>
I am one of the core developers of Airflow and working on the
"airflow.sdk"
package, and this updates the recommended replacments to the correct
user-facing imports.[^1]
cc @Lee-W @uranusjr
[^1]:
33f0f1d639/task-sdk/src/airflow/sdk/__init__.py (L68-L93)
<!--
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?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
Hope and pray? 😉
I'm sure there are some snapshot files I'm supposed to fix first.
<!-- How was it tested? -->
## Summary
This PR removes false-positive diagnostics for `*` imports. Currently we
always emit a diagnostic for these statements unless the module we're
importing from has a symbol named `"*"` in its symbol table for the
global scope. (And if we were doing everything correctly, no module ever
would have a symbol named `"*"` in its global scope!)
The fix here is sort-of hacky and won't be what we'll want to do
long-term. However, I think it's useful to do this as a first step
since:
- It significantly reduces false positives when running on code that
uses `*` imports
- It "resets" the tests to a cleaner state with many fewer TODOs, making
it easier to see what the hard work is that's still to be done.
## Test Plan
`cargo test -p red_knot_python_semantic`
## Summary
This PR adds a suite of tests for wildcard (`*`) imports. The tests
nearly all fail for now, and those that don't, ahem, pass for the wrong
reasons...
I've tried to add TODO comments in all instances for places where we are
currently inferring the incorrect thing, incorrectly emitting a
diagnostic, or emitting a diagnostic with a bad error message.
## Test Plan
`cargo test -p red_knot_python_semantic`
This breaks up call binding into two phases:
- **_Matching parameters_** just looks at the names and kinds
(positional/keyword) of each formal and actual parameters, and matches
them up. Most of the current call binding errors happen during this
phase.
- Once we have matched up formal and actual parameters, we can **_infer
types_** of each actual parameter, and **_check_** that each one is
assignable to the corresponding formal parameter type.
As part of this, we add information to each formal parameter about
whether it is a type form or not. Once [PEP
747](https://peps.python.org/pep-0747/) is finalized, we can hook that
up to this internal type form representation. This replaces the
`ParameterExpectations` type, which did the same thing in a more ad hoc
way.
While we're here, we add a new fluent API for building `Parameter`s,
which makes our signature constructors a bit nicer to read. We also
eliminate a TODO where we were consuming types from the argument list
instead of the bound parameter list when evaluating our special-case
known functions.
Closes#15460
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
This change continues to resolve#16071 (and continues the work started
in #16162). Specifically, this PR changes the code in the parser so that
it uses the `OperatorPrecedence` struct from `ruff_python_ast` instead
of its own version. This is part of an effort to get rid of the
redundant definitions of `OperatorPrecedence` throughout the codebase.
Note that this PR only makes this change for `ruff_python_parser` -- we
still want to make a similar change for the formatter (namely the
`OperatorPrecedence` defined in the expression part of the formatter,
the pattern one is different). I separated the work to keep the PRs
small and easily reviewable.
## Test Plan
Because this is an internal change, I didn't add any additional tests.
Existing tests do pass.
## Summary
Part of #15382
This PR adds support for checking the subtype relationship between the
two callable types.
The main source of reference used for implementation is
https://typing.python.org/en/latest/spec/callables.html#assignability-rules-for-callables.
The implementation is split into two phases:
1. Check all the positional parameters which includes positional-only,
standard (positional or keyword) and variadic kind
2. Collect all the keywords in a `HashMap` to do the keyword parameters
check via name lookup
For (1), there's a helper struct which is similar to `.zip_longest`
(from `itertools`) except that it allows control over one of the
iterator as that's required when processing a variadic parameter. This
is required because positional parameters needs to be checked as per
their position between the two callable types. The struct also keeps
track of the current iteration element because when the loop is exited
(to move on to the phase 2) the current iteration element would be
carried over to the phase 2 check.
This struct is internal to the `is_subtype_of` method as I don't think
it makes sense to expose it outside. It also allows me to use "self" and
"other" suffixed field names as that's only relevant in that context.
## Test Plan
Add extensive tests in markdown.
Converted all of the code snippets from
https://typing.python.org/en/latest/spec/callables.html#assignability-rules-for-callables
to use `knot_extensions.is_subtype_of` and verified the result.
## Summary
This PR checks whether two callable types are equivalent or not.
This is required because for an equivalence relationship, the default
value does not necessarily need to be the same but if the parameter in
one of the callable has a default value then the corresponding parameter
in the other callable should also have a default value. This is the main
reason a manual implementation is required.
And, as per https://typing.python.org/en/latest/spec/callables.html#id4,
the default _type_ doesn't participate in a subtype relationship, only
the optionality (required or not) participates. This means that the
following two callable types are equivalent:
```py
def f1(a: int = 1) -> None: ...
def f2(a: int = 2) -> None: ...
```
Additionally, the name of positional-only, variadic and keyword-variadic
are not required to be the same for an equivalence relation.
A potential solution to avoid the manual implementation would be to only
store whether a parameter has a default value or not but the type is
currently required to check for assignability.
## Test plan
Add tests for callable types in `is_equivalent_to.md`
## Summary
Catch some Instances, but raise type error for the rest of them
Fixes#16851
## Test Plan
Extend invalid.md in annotations
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Currently for something like `X = typing.Tuple[str, str]`, we infer the
value of `X` as `object`. That's because `Tuple` (like many of the
symbols in the typing module) is annotated as a `_SpecialForm` instance
in typeshed's stubs:
23382f5f8c/crates/red_knot_vendored/vendor/typeshed/stdlib/typing.pyi (L215)
and we don't understand implicit type aliases yet, and the stub for
`_SpecialForm.__getitem__` says it always returns `object`:
23382f5f8c/crates/red_knot_vendored/vendor/typeshed/stdlib/typing.pyi (L198-L200)
We have existing false positives in our test suite due to this:
23382f5f8c/crates/red_knot_python_semantic/resources/mdtest/annotations/annotated.md?plain=1#L76-L78
and it's causing _many_ new false positives in #16872, which tries to
make our annotation-expression parsing stricter in some ways.
This PR therefore adds some small special casing for `KnownInstanceType`
variants that fallback to `_SpecialForm`, so that these false positives
can be avoided.
## Test Plan
Existing mdtest altered.
Cc. @MatthewMckee4
Summary
--
Fixes#16874. I previously emitted a syntax error when starred
annotations were _allowed_ rather than when they were actually used.
This caused false positives for any starred parameter name because these
are allowed to have starred annotations but not required to. The fix is
to check if the annotation is actually starred after parsing it.
Test Plan
--
New inline parser tests derived from the initial report and more
examples from the comments, although I think the first case should cover
them all.
## Summary
From #16641
The previous PR attempted to fix the errors presented in this PR, but as
discussed in the conversation, it was concluded that the approach was
undesirable and that further work would be needed to fix the errors with
a correct general solution.
In this PR, I instead add the test cases from the previous PR as TODOs,
as a starting point for future work.
## Test Plan
---------
Co-authored-by: Carl Meyer <carl@oddbird.net>
## Summary
Fixes#16744
Allows the cli to find a virtual environment from the VIRTUAL_ENV
environment variable if no `--python` is set
## Test Plan
Manual testing, of:
- Virtual environments explicitly activated using `source .venv/bin/activate`
- Virtual environments implicilty activated via `uv run`
- Broken virtual environments with no `pyvenv.cfg` file
## Summary
This change follows up on the bug-fix requested in #16747 --
`ruff_python_ast::OperatorPrecedence` had an enum variant, `BitXorOr`,
which which gave the same precedence to the `|` and `^` operators. This
goes against [Python's documentation for operator
precedence](https://docs.python.org/3/reference/expressions.html#operator-precedence),
so this PR changes the code so that it's correct.
This is part of the overall effort to unify redundant definitions of
`OperatorPrecedence` throughout the codebase (#16071)
## Test Plan
Because this is an internal change, I only ran existing tests to ensure
nothing was broken.
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
Fixes#8191 by introducing `--exit-non-zero-on-format` to `ruff format`
which pretty much does what it says on the tin.
## Test Plan
Added a new test!
---------
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## Summary
This PR reworks `TypeInferenceBuilder::infer_type_expression()` so that
we emit diagnostics when encountering a list literal in a type
expression. The only place where a list literal is allowed in a type
expression is if it appears as the first argument to `Callable[]`, and
`Callable` is already heavily special-cased in our type-expression
parsing.
In order to ensure that list literals are _always_ allowed as the
_first_ argument to `Callabler` (but never allowed as the second, third,
etc. argument), I had to do some refactoring of our type-expression
parsing for `Callable` annotations.
## Test Plan
New mdtests added, and existing ones updated
<!--
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?
-->
## Summary
fixes#15048
We want to handle more types from Type::KnownInstance
## Test Plan
Add tests for each type added explicitly in the match
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## Summary
These are just cosmetic changes, but I'm separating them out into a
standalone PR to make a branch I have stacked on top of this easier to
review
## Test Plan
Existing tests all pass
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
Part of #15382
This PR infers the return type `lambda` expression as `Unknown`. In the
future, it would be more useful to infer the expression type considering
the surrounding context (#16696).
## Test Plan
Update existing test cases from `@todo` to the (verified) return type.
## Summary
Previously, the `name` field was on `Parameter` which required it to be
always optional regardless of the parameter kind because a
`typing.Callable` signature does not have name for the parameters. This
is the case for positional-only parameters. This wasn't enforced at the
type level which meant that downstream usages would have to unwrap on
`name` even though it's guaranteed to be present.
This commit moves the `name` field from `Parameter` to the
`ParameterKind` variants and makes it optional only for
`ParameterKind::PositionalOnly` variant while required for all other
variants.
One change that's now required is that a `Callable` form using a gradual
form for parameter types (`...`) would have a default `args` and
`kwargs` name used for variadic and keyword-variadic parameter kind
respectively. This is also the case for invalid `Callable` type forms. I
think this is fine as names are not relevant in this context but happy
to make it optional even in variadic variants.
## Test Plan
No new tests; make sure existing tests are passing.
## Summary
Add error messages for invalid nodes in type expressions
Fixes#16816
## Test Plan
Extend annotations/invalid.md to handle these invalid AST nodes error
messages
## Summary
This PR adds a playground for Red Knot
[Screencast from 2024-08-14
10-33-54.webm](https://github.com/user-attachments/assets/ae81d85f-74a3-4ba6-bb61-4a871b622f05)
Sharing does work 😆 I just forgot to start wrangler.
It supports:
* Multiple files
* Showing the AST
* Showing the tokens
* Sharing
* Persistence to local storage
Future extensions:
* Configuration support: The `pyproject.toml` would *just* be another
file.
* Showing type information on hover
## Blockers
~~Salsa uses `catch_unwind` to break cycles, which Red Knot uses
extensively when inferring types in the standard library.
However, WASM (at least `wasm32-unknown-unknown`) doesn't support
`catch_unwind` today, so the playground always crashes when the type
inference encounters a cycle.~~
~~I created a discussion in the [salsa
zulip](https://salsa.zulipchat.com/#narrow/stream/333573-salsa-3.2E0/topic/WASM.20support)
to see if it would be possible to **not** use catch unwind to break
cycles.~~
~~[Rust tracking issue for WASM catch unwind
support](https://github.com/rust-lang/rust/issues/118168)~~
~~I tried to build the WASM with the nightly compiler option but ran
into problems because wasm-bindgen doesn't support WASM-exceptions. We
could try to write the binding code by hand.~~
~~Another alternative is to use `wasm32-unknown-emscripten` but it's
rather painful to build~~
Brent Westbrook
Micha Reiser
Douglas Creager
David Peter
Alex Waygood
Dylan
Eric Mark Martin
Dhruv Manilawala
Wei Lee
Andrew Gallant
trag1c
cake-monotone
Mohammad Amin Ghasemi
Matthew Mckee
github-actions[bot]
Carl Meyer
Max Mynter
Trevor Manz
Aarni Koskela
renovate[bot]
Mike Perlov
Shunsuke Shibayama
InSync
Aleksei Latyshev
John Stilley
Daniel Wilton
Vasco Schiavo
Ash Berlin-Taylor
Junhson Jean-Baptiste
Josh Cannon