This PR adds a syntax error if the parser encounters a `TryStmt` that
has except clauses both with and without a star.
The displayed error points to each except clause that contradicts the
original except clause kind. So, for example,
```python
try:
....
except: #<-- we assume this is the desired except kind
....
except*: #<--- error will point here
....
except*: #<--- and here
....
```
Closes#14860
This adds support for `type[Any]`, which represents an unknown type (not
an instance of an unknown type), and `type`, which we are choosing to
interpret as `type[object]`.
Closes#14546
## Summary
This is already several hundred lines of code, and it will get more
complex with call-signature checking.
## Test Plan
This is a pure code move; the moved code wasn't changed, just imports.
Existing tests pass.
## Summary
Add a `is_fully_static` premise to the equivalence on subtyping property tests.
## Test Plan
```
cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable
```
Without this, `cargo insta test` re-compiles every time it is run, even
if there are no changes. With this, I can re-run `cargo insta test` (or
other `cargo build` commands) without it resulting in re-compiles.
I made an identical change to uv a while back:
https://github.com/astral-sh/uv/pull/6825
## Summary
This is the second PR out of three that adds support for
enabling/disabling lint rules in Red Knot. You may want to take a look
at the [first PR](https://github.com/astral-sh/ruff/pull/14869) in this
stack to familiarize yourself with the used terminology.
This PR adds a new syntax to define a lint:
```rust
declare_lint! {
/// ## What it does
/// Checks for references to names that are not defined.
///
/// ## Why is this bad?
/// Using an undefined variable will raise a `NameError` at runtime.
///
/// ## Example
///
/// ```python
/// print(x) # NameError: name 'x' is not defined
/// ```
pub(crate) static UNRESOLVED_REFERENCE = {
summary: "detects references to names that are not defined",
status: LintStatus::preview("1.0.0"),
default_level: Level::Warn,
}
}
```
A lint has a name and metadata about its status (preview, stable,
removed, deprecated), the default diagnostic level (unless the
configuration changes), and documentation. I use a macro here to derive
the kebab-case name and extract the documentation automatically.
This PR doesn't yet add any mechanism to discover all known lints. This
will be added in the next and last PR in this stack.
## Documentation
I documented some rules but then decided that it's probably not my best
use of time if I document all of them now (it also means that I play
catch-up with all of you forever). That's why I left some rules
undocumented (marked with TODO)
## Where is the best place to define all lints?
I'm not sure. I think what I have in this PR is fine but I also don't
love it because most lints are in a single place but not all of them. If
you have ideas, let me know.
## Why is the message not part of the lint, unlike Ruff's `Violation`
I understand that the main motivation for defining `message` on
`Violation` in Ruff is to remove the need to repeat the same message
over and over again. I'm not sure if this is an actual problem. Most
rules only emit a diagnostic in a single place and they commonly use
different messages if they emit diagnostics in different code paths,
requiring extra fields on the `Violation` struct.
That's why I'm not convinced that there's an actual need for it and
there are alternatives that can reduce the repetition when creating a
diagnostic:
* Create a helper function. We already do this in red knot with the
`add_xy` methods
* Create a custom `Diagnostic` implementation that tailors the entire
diagnostic and pre-codes e.g. the message
Avoiding an extra field on the `Violation` also removes the need to
allocate intermediate strings as it is commonly the place in Ruff.
Instead, Red Knot can use a borrowed string with `format_args`
## Test Plan
`cargo test`
## Summary
This PR introduces a structured `DiagnosticId` instead of using a plain
`&'static str`. It is the first of three in a stack that implements a
basic rules infrastructure for Red Knot.
`DiagnosticId` is an enum over all known diagnostic codes. A closed enum
reduces the risk of accidentally introducing two identical diagnostic
codes. It also opens the possibility of generating reference
documentation from the enum in the future (not part of this PR).
The enum isn't *fully closed* because it uses a `&'static str` for lint
names. This is because we want the flexibility to define lints in
different crates, and all names are only known in `red_knot_linter` or
above. Still, lower-level crates must already reference the lint names
to emit diagnostics. We could define all lint-names in `DiagnosticId`
but I decided against it because:
* We probably want to share the `DiagnosticId` type between Ruff and Red
Knot to avoid extra complexity in the diagnostic crate, and both tools
use different lint names.
* Lints require a lot of extra metadata beyond just the name. That's why
I think defining them close to their implementation is important.
In the long term, we may also want to support plugins, which would make
it impossible to know all lint names at compile time. The next PR in the
stack introduces extra syntax for defining lints.
A closed enum does have a few disadvantages:
* rustc can't help us detect unused diagnostic codes because the enum is
public
* Adding a new diagnostic in the workspace crate now requires changes to
at least two crates: It requires changing the workspace crate to add the
diagnostic and the `ruff_db` crate to define the diagnostic ID. I
consider this an acceptable trade. We may want to move `DiagnosticId` to
its own crate or into a shared `red_knot_diagnostic` crate.
## Preventing duplicate diagnostic identifiers
One goal of this PR is to make it harder to introduce ambiguous
diagnostic IDs, which is achieved by defining a closed enum. However,
the enum isn't fully "closed" because it doesn't explicitly list the IDs
for all lint rules. That leaves the possibility that a lint rule and a
diagnostic ID share the same name.
I made the names unambiguous in this PR by separating them into
different namespaces by using `lint/<rule>` for lint rule codes. I don't
mind the `lint` prefix in a *Ruff next* context, but it is a bit weird
for a standalone type checker. I'd like to not overfocus on this for now
because I see a few different options:
* We remove the `lint` prefix and add a unit test in a top-level crate
that iterates over all known lint rules and diagnostic IDs to ensure the
names are non-overlapping.
* We only render `[lint]` as the error code and add a note to the
diagnostic mentioning the lint rule. This is similar to clippy and has
the advantage that the header line remains short
(`lint/some-long-rule-name` is very long ;))
* Any other form of adjusting the diagnostic rendering to make the
distinction clear
I think we can defer this decision for now because the `DiagnosticId`
contains all the relevant information to change the rendering
accordingly.
## Why `Lint` and not `LintRule`
I see three kinds of diagnostics in Red Knot:
* Non-suppressable: Reveal type, IO errors, configuration errors, etc.
(any `DiagnosticId`)
* Lints: code-related diagnostics that are suppressable.
* Lint rules: The same as lints, but they can be enabled or disabled in
the configuration. The majority of lints in Red Knot and the Ruff
linter.
Our current implementation doesn't distinguish between lints and Lint
rules because we aren't aware of a suppressible code-related lint that
can't be configured in the configuration. The only lint that comes to my
mind is maybe `division-by-zero` if we're 99.99% sure that it is always
right. However, I want to keep the door open to making this distinction
in the future if it proves useful.
Another reason why I chose lint over lint rule (or just rule) is that I
want to leave room for a future lint rule and lint phase concept:
* lint is the *what*: a specific code smell, pattern, or violation
* the lint rule is the *how*: I could see a future `LintRule` trait in
`red_knot_python_linter` that provides the necessary hooks to run as
part of the linter. A lint rule produces diagnostics for exactly one
lint. A lint rule differs from all lints in `red_knot_python_semantic`
because they don't run as "rules" in the Ruff sense. Instead, they're a
side-product of type inference.
* the lint phase is a different form of *how*: A lint phase can produce
many different lints in a single pass. This is a somewhat common pattern
in Ruff where running one analysis collects the necessary information
for finding many different lints
* diagnostic is the *presentation*: Unlike a lint, the diagnostic isn't
the what, but how a specific lint gets presented. I expect that many
lints can use one generic `LintDiagnostic`, but a few lints might need
more flexibility and implement their custom diagnostic rendering (at
least custom `Diagnostic` implementation).
## Test Plan
`cargo test`
## Summary
Add replacement fixes to deprecated arguments of a DAG.
Ref #14582#14626
## Test Plan
Diff was verified and snapshots were updated.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## Summary
Per suggestion in
https://github.com/astral-sh/ruff/pull/14802#discussion_r1875455417
This is a bit less error-prone and allows us to handle both expressions
in the current scope or a different scope. Also, there's currently no
need for this method outside of `TypeInferenceBuilder`, so no reason to
expose it in `types.rs`.
## Test Plan
Pure refactor, no functional change; existing tests pass.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Part 1 of the big change introduced in #14828. This temporarily causes
all fixes for `round(...)` to be considered unsafe, but they will
eventually be enhanced.
## Test Plan
`cargo nextest run` and `cargo insta test`.
## Summary
Close#11243. Fix `pytest-parametrize-names-wrong-type (PT006)` to edit
both `argnames` and `argvalues` if both of them are single-element
tuples/lists.
```python
# Before fix
@pytest.mark.parametrize(("x",), [(1,), (2,)])
def test_foo(x):
...
# After fix:
@pytest.mark.parametrize("x", [1, 2])
def test_foo(x):
...
```
## Test Plan
New test cases
This PR introduces three changes to the diagnostic and fix behavior
(still under preview) for [boolean-chained-comparison
(PLR1716)](https://docs.astral.sh/ruff/rules/boolean-chained-comparison/#boolean-chained-comparison-plr1716).
1. We now offer a _fix_ in the case of parenthesized expressions like
`(a < b) and b < c`. The fix will merge the chains of comparisons and
then balance parentheses by _adding_ parentheses to one side of the
expression.
2. We now trigger a diagnostic (and fix) in the case where some
comparisons have multiple comparators like `a < b < c and c < d`.
3. When adjacent comparators are parenthesized, we prefer the left
parenthesization and apply the replacement to the whole parenthesized
range. So, for example, `a < (b) and ((b)) < c` becomes `a < (b) < c`.
While these seem like somewhat disconnected changes, they are actually
related. If we only offered (1), then we would see the following fix
behavior:
```diff
- (a < b) and b < c and ((c < d))
+ (a < b < c) and ((c < d))
```
This is because the fix which add parentheses to the first pair of
comparisons overlaps with the fix that removes the `and` between the
second two comparisons. So the latter fix is deferred. However, the
latter fix does not get a second chance because, upon the next lint
iteration, there is no violation of `PLR1716`.
Upon adopting (2), however, both fixes occur by the time ruff completes
several iterations and we get:
```diff
- (a < b) and b < c and ((c < d))
+ ((a < b < c < d))
```
Finally, (3) fixes a previously unobserved bug wherein the autofix for
`a < (b) and b < c` used to result in `a<(b<c` which gives a syntax
error. It could in theory have been fixed in a separate PR, but seems to
be on theme here.
----------
- Closes#13524
- (1), (2), and (3) are implemented in separate commits for ease of
review and modification.
- Technically a user can trigger an error in ruff (by reaching max
iterations) if they have a humongous boolean chained comparison with
differing parentheses levels.
## Summary
Minor change for the documentation of COM818 rule. This was a block
called “In the event that a tuple is intended”, but the suggested change
did not produce a tuple.
## Test Plan
```python
>>> import json
>>> (json.dumps({"bar": 1}),) # this is a tuple
('{"bar": 1}',)
>>> (json.dumps({"bar": 1})) # not a tuple
'{"bar": 1}'
```
Improves error message for [except*](https://peps.python.org/pep-0654/)
(Rules: B025, B029, B030, B904)
Example python snippet:
```python
try:
a = 1
except* ValueError:
a = 2
except* ValueError:
a = 2
try:
pass
except* ():
pass
try:
pass
except* 1: # error
pass
try:
raise ValueError
except* ValueError:
raise UserWarning
```
Error messages
Before:
```
$ ruff check --select=B foo.py
foo.py:6:9: B025 try-except block with duplicate exception `ValueError`
foo.py:11:1: B029 Using `except ():` with an empty tuple does not catch anything; add exceptions to handle
foo.py:16:9: B030 `except` handlers should only be exception classes or tuples of exception classes
foo.py:22:5: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
Found 4 errors.
```
After:
```
$ ruff check --select=B foo.py
foo.py:6:9: B025 try-except* block with duplicate exception `ValueError`
foo.py:11:1: B029 Using `except* ():` with an empty tuple does not catch anything; add exceptions to handle
foo.py:16:9: B030 `except*` handlers should only be exception classes or tuples of exception classes
foo.py:22:5: B904 Within an `except*` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
Found 4 errors.
```
Closes https://github.com/astral-sh/ruff/issues/14791
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
This adds support for `type[a.X]`, where the `type` special form is
applied to a qualified name that resolves to a class literal. This works
for both nested classes and classes imported from another module.
Closes#14545
## Summary
Inferred and declared types for function parameters, in the function
body scope.
Fixes#13693.
## Test Plan
Added mdtests.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Airflow 3.0 removes various deprecated functions, members, modules, and
other values. They have been deprecated in 2.x, but the removal causes
incompatibilities that we want to detect. This PR deprecates the
following names.
* in `DAG`
* `sla_miss_callback` was removed
* in `airflow.operators.trigger_dagrun.TriggerDagRunOperator`
* `execution_date` was removed
* in `airflow.operators.weekday.DayOfWeekSensor`,
`airflow.operators.datetime.BranchDateTimeOperator` and
`airflow.operators.weekday.BranchDayOfWeekOperator`
* `use_task_execution_day` was removed in favor of
`use_task_logical_date`
The full list of rules we will extend
https://github.com/apache/airflow/issues/44556
## Test Plan
<!-- How was it tested? -->
A test fixture is included in the PR.
## Summary
`typing.Never` and `typing.LiteralString` are only conditionally
exported from `typing` for Python versions 3.11 and later. We run the
Markdown tests with the default Python version of 3.9, so here we change
the import to `typing_extensions` instead, and add a new test to make
sure we'll continue to understand the `typing`-version of these symbols
for newer versions.
This didn't cause problems so far, as we don't understand
`sys.version_info` branches yet.
## Test Plan
New Markdown tests to make sure this will continue to work in the
future.
## Summary
Fixes https://github.com/astral-sh/ruff/issues/14778
The formatter incorrectly removed the inner implicitly concatenated
string for following single-line f-string:
```py
f"{'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'a' if True else ""}"
# formatted
f"{ if True else ''}"
```
This happened because I changed the `RemoveSoftlinesBuffer` in
https://github.com/astral-sh/ruff/pull/14489 to remove any content
wrapped in `if_group_breaks`. After all, it emulates an *all flat*
layout. This works fine when `if_group_breaks` is only used to **add**
content if the gorup breaks. It doesn't work if the same content is
rendered differently depending on if the group fits using
`if_group_breaks` and `if_groups_fits` because the enclosing `group`
might still *break* if the entire content exceeds the line-length limit.
This PR fixes this by unwrapping any `if_group_fits` content by removing
the `if_group_fits` start and end tags.
## Test Plan
added test
## Summary
This adds support for specifying the target Python version from a
Markdown test. It is a somewhat limited ad-hoc solution, but designed to
be future-compatible. TOML blocks can be added to arbitrary sections in
the Markdown block. They have the following format:
````markdown
```toml
[tool.knot.environment]
target-version = "3.13"
```
````
So far, there is nothing else that can be configured, but it should be
straightforward to extend this to things like a custom typeshed path.
This is in preparation for the statically-known branches feature where
we are going to have to specify the target version for lots of tests.
## Test Plan
- New Markdown test that fails without the explicitly specified
`target-version`.
- Manually tested various error paths when specifying a wrong
`target-version` field.
- Made sure that running tests is as fast as before.
## Summary
Fixes https://github.com/astral-sh/ruff/issues/14807
I suspect that this broke when we updated notify, although I'm not quiet
sure how this *ever* worked...
The problem was that the file watcher didn't skip over `Access` events,
but Ruff itself accesses the `pyproject.toml` when checking the project.
That means, Ruff triggers `Access` events but it also schedules a
re-check on every `Access` event... and this goes one forever.
This PR skips over `Access` and `Other` event. `Access` events are
uninteresting because they're only reads, they don't change any file
metadata or content.
The `Other` events should be rare and are mainly to inform about file
watcher changes... we don't need those.
I also added an explicit handling for the `Rescan` event. File watchers
emit a `Rescan` event if they failed to capture some file watching
changes
and it signals that the program should assume that all files might have
changed (the program should do a rescan to *get up to date*).
## Test Plan
I tested that Ruff no longer loops when running `check --watch`. I
verified that Ruff rechecks file after making content changes.
