## Summary
[**Rendered version of the new test
suite**](https://github.com/astral-sh/ruff/blob/david/intersection-type-tests/crates/red_knot_python_semantic/resources/mdtest/intersection_types.md)
Moves most of our existing intersection-types tests to a dedicated
Markdown test suite, extends the test coverage, unifies the notation for
these tests, groups tests into a proper structure, and adds some
explanations for various simplification strategies.
This changeset also:
- Adds a new simplification where `~Never` is removed from
intersections.
- Adds a new simplification where adding `~object` simplifies the whole
intersection to `Never`
- Avoids unnecessary assignment-checks between inferred and declared
type. This was added to this changeset to avoid many false positive
errors in this test suite.
Resolves the task described in this old comment
[here](e01da82a5a..e7e432bca2 (r1819924085)).
## Test Plan
Running the new Markdown tests
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Prompted by
> One nit: I think we need to consider `Any` and `Unknown` and `Todo` as
all (gradually) equivalent to each other, and thus `type & Any` and
`type & Unknown` and `type & Todo` as also equivalent. The distinction
between `Any` vs `Unknown` vs `Todo` is entirely about
provenance/debugging, there is no type level distinction. (And I've been
wondering if the `Any` vs `Unknown` distinction is really worth it.)
The thought here is that _most_ places want to treat `Any`, `Unknown`,
and `Todo` identically. So this PR simplifies things by having a single
`Type::Any` variant, and moves the provenance part into a new `AnyType`
type. If you need to treat e.g. `Todo` differently, you still can by
pattern-matching into the `AnyType`. But if you don't, you can just use
`Type::Any(_)`.
(This would also allow us to (more easily) distinguish "unknown via an
unannotated value" from "unknown because of a typing error" should we
want to do that in the future)
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Carl Meyer <carl@astral.sh>
## Summary
This moves almost all of our existing `UnionBuilder` tests to a
Markdown-based test suite.
I see how this could be a more controversial change, since these tests
where written specifically for `UnionBuilder`, and by creating the union
types using Python type expressions, we add an additional layer on top
(parsing and inference of these expressions) that moves these tests away
from clean unit tests more in the direction of integration tests. Also,
there are probably a few implementation details of `UnionBuilder` hidden
in the test assertions (e.g. order of union elements after
simplifications).
That said, I think we would like to see all those properties that are
being tested here from *any* implementation of union types. And the
Markdown tests come with the usual advantages:
- More consice
- Better readability
- No re-compiliation when working on tests
- Easier to add additional explanations and structure to the test suite
This changeset adds a few additional tests, but keeps the logic of the
existing tests except for a few minor modifications for consistency.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: T-256 <132141463+T-256@users.noreply.github.com>
## Summary
- Add a workflow to run property tests on a daily basis (based on
`daily_fuzz.yaml`)
- Mark `assignable_to_is_reflexive` as flaky (related to #14899)
- Add new (failing) `intersection_assignable_to_both` test (also related
to #14899)
## Test Plan
Ran:
```bash
export QUICKCHECK_TESTS=100000
while cargo test --release -p red_knot_python_semantic -- \
--ignored types::property_tests::stable; do :; done
```
Observed successful property_tests CI run
## Summary
This changeset migrates all existing `is_assignable_to` tests to a
Markdown-based test. It also increases our test coverage in a hopefully
meaningful way (not claiming to be complete in any sense). But at least
I found and fixed one bug while doing so.
## Test Plan
Ran property tests to make sure the new test succeeds after fixing it.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Adds a type-check-time Python API that allows us to create and
manipulate types and to test various of their properties. For example,
this can be used to write a Markdown test to make sure that `A & B` is a
subtype of `A` and `B`, but not of an unrelated class `C` (something
that requires quite a bit more code to do in Rust):
```py
from knot_extensions import Intersection, is_subtype_of, static_assert
class A: ...
class B: ...
type AB = Intersection[A, B]
static_assert(is_subtype_of(AB, A))
static_assert(is_subtype_of(AB, B))
class C: ...
static_assert(not is_subtype_of(AB, C))
```
I think this functionality is also helpful for interactive debugging
sessions, in order to query various properties of Red Knot's type
system. Which is something that otherwise requires a custom Rust unit
test, some boilerplate code and constant re-compilation.
## Test Plan
- New Markdown tests
- Tested the modified typeshed_sync workflow locally
## Summary
`Type[Any]` should be assignable to `object`. All types should be
assignable to `object`.
We specifically didn't understand the former; this PR adds a test for
it, and a case to ensure that `Type[Any]` is assignable to anything that
`type` is assignable to (which includes `object`).
This PR also adds a property test that all types are assignable to
object. In order to make it pass, I added a special case to check early
if we are assigning to `object` and just return `true`. In principle,
once we get all the more general cases correct, this special case might
be removable. But having the special case for now allows the property
test to pass.
And we add a property test that all types are subtypes of object. This
failed for the case of an intersection with no positive elements (that
is, a negation type). This really does need to be a special case for
`object`, because there is no other type we can know that a negation
type is a subtype of.
## Test Plan
Added unit test and property test.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
We now support class patterns in a match statement, adding a narrowing
constraint that within the body of that match arm, we can assume that
the subject is an instance of that class.
---------
Co-authored-by: Carl Meyer <carl@astral.sh>
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
This implements checking of calls.
I ended up following Micha's original suggestion from back when the
signature representation was first introduced, and flattening it to a
single array of parameters. This turned out to be easier to manage,
because we can represent parameters using indices into that array, and
represent the bound argument types as an array of the same length.
Starred and double-starred arguments are still TODO; these won't be very
useful until we have generics.
The handling of diagnostics is just hacked into `return_ty_result`,
which was already inconsistent about whether it emitted diagnostics or
not; now it's even more inconsistent. This needs to be addressed, but
could be a follow-up.
The new benchmark errors here surface the need for intersection support
in `is_assignable_to`.
Fixes#14161.
## Test Plan
Added mdtests.
## Summary
When debugging, I frequently want to know which symbols are being looked
up. `symbol_by_id` adds tracing information, but it only shows the
`ScopedSymbolId`. Since `symbol_by_id` is only called from `symbol`, it
seems reasonable to move the tracing call one level up from
`symbol_by_id` to `symbol`, where we can also show the name of the
symbol.
**Before**:
```
6 └─┐red_knot_python_semantic::types::infer::infer_expression_types{expression=Id(60de), file=/home/shark/tomllib_modified/_parser.py}
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(33)}
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(123)}
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(54)}
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(122)}
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(165)}
6 ┌─┘
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(32)}
6 ┌─┘
6 └─┐red_knot_python_semantic::types::symbol_by_id{symbol=ScopedSymbolId(232)}
6 ┌─┘
6 ┌─┘
6 ┌─┘
6┌─┘
```
**After**:
```
5 └─┐red_knot_python_semantic::types::infer::infer_expression_types{expression=Id(60de), file=/home/shark/tomllib_modified/_parser.py}
5 └─┐red_knot_python_semantic::types::symbol{name="dict"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="dict"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="list"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="list"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="isinstance"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="isinstance"}
5 ┌─┘
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="ValueError"}
5 ┌─┘
5 └─┐red_knot_python_semantic::types::symbol{name="ValueError"}
5 ┌─┘
5 ┌─┘
5 ┌─┘
5┌─┘
```
## Test Plan
```
cargo run --bin red_knot -- --current-directory path/to/tomllib -vvv
```
## Summary
While looking at #14899, I looked at seeing if I could get shrinking on
the examples. It turned out to be straightforward, with a couple of
caveats.
I'm calling `clone` a lot during shrinking. Since by the shrink step
we're already looking at a test failure this feels fine? Unless I
misunderstood `quickcheck`'s core loop
When shrinking `Intersection`s, in order to just rely on `quickcheck`'s
`Vec` shrinking without thinking about it too much, the shrinking
strategy is:
- try to shrink the negative side (keeping the positive side the same)
- try to shrink the positive side (keeping the negative side the same)
This means that you can't shrink from `(A & B & ~C & ~D)` directly to
`(A & ~C)`! You would first need an intermediate failure at `(A & B &
~C)` or `(A & ~C & ~D)`. This feels good enough. Shrinking the negative
side first also has the benefit of trying to strip down negative
elements in these intersections.
## Test Plan
`cargo test -p red_knot_python_semantic -- --ignored
types::property_tests::stable` still fails as it current does on `main`,
but now the errors seem more minimal.
Just like in #15045 for unary expressions: In binary expressions, we
were only looking for dunder expressions for `Type::Instance` types. We
had some special cases for coercing the various `Literal` types into
their corresponding `Instance` types before doing the lookup. But we can
side-step all of that by using the existing `Type::to_meta_type` and
`Type::to_instance` methods.
Resolves#14840
## Summary
Usage of ellipsis literal as default argument is allowed in stub files.
## Test Plan
Added mdtest for both python files and stub files.
---------
Co-authored-by: Carl Meyer <carl@oddbird.net>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
The test expression in an `elif` clause is evaluated whether or not we
take the branch. Our control flow model for if/elif chains failed to
reflect this, causing wrong inference in cases where an assignment
expression occurs inside an `elif` test expression. Our "no branch taken
yet" snapshot (which is the starting state for every new elif branch)
can't simply be the pre-if state, it must be updated after visiting each
test expression.
Once we do this, it also means we no longer need to track a vector of
narrowing constraints to reapply for each new branch, since our "branch
not taken" state (which is the initial state for each branch) is
continuously updated to include the negative narrowing constraints of
all previous branches.
Fixes#15033.
## Test Plan
Added mdtests.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
We understand `sys.version_info` branches now! As such, I _believe_ this
branch is no longer required; all tests pass without it. I also ran
`QUICKCHECK_TESTS=100000 cargo test -p red_knot_python_semantic --
--ignored types::property_tests::stable`, and no tests failed except for
the known issue with `Type::is_assignable_to()`
(https://github.com/astral-sh/ruff/issues/14899)
## Test Plan
See above
## Summary
Remove `Type::tuple` in favor of `TupleType::from_elements`, avoid a few
intermediate `Vec`tors. Resolves an old [review
comment](https://github.com/astral-sh/ruff/pull/14744#discussion_r1867493706).
## Test Plan
New regression test for something I ran into while implementing this.
## Summary
Part of #13773
This PR adds diagnostics when there is a length mismatch during
unpacking between the number of target expressions and the number of
types for the unpack value expression.
There are 3 cases of diagnostics here where the first two occurs when
there isn't a starred expression and the last one occurs when there's a
starred expression:
1. Number of target expressions is **less** than the number of types
that needs to be unpacked
2. Number of target expressions is **greater** then the number of types
that needs to be unpacked
3. When there's a starred expression as one of the target expression and
the number of target expressions is greater than the number of types
Examples for all each of the above cases:
```py
# red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]
a, b = (1, 2, 3)
# red-knot: Not enough values to unpack (expected 2, got 1) [lint:invalid-assignment]
a, b = (1,)
# red-knot: Not enough values to unpack (expected 3 or more, got 2) [lint:invalid-assignment]
a, *b, c, d = (1, 2)
```
The (3) case is a bit special because it uses a distinct wording
"expected n or more" instead of "expected n" because of the starred
expression.
### Location
The diagnostic location is the target expression that's being unpacked.
For nested targets, the location will be the nested expression. For
example:
```py
(a, (b, c), d) = (1, (2, 3, 4), 5)
# ^^^^^^
# red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]
```
For future improvements, it would be useful to show the context for why
this unpacking failed. For example, for why the expected number of
targets is `n`, we can highlight the relevant elements for the value
expression.
In the **ecosystem**, **Pyright** uses the target expressions for
location while **mypy** uses the value expression for the location. For
example:
```py
if 1:
# mypy: Too many values to unpack (2 expected, 3 provided) [misc]
# vvvvvvvvv
a, b = (1, 2, 3)
# ^^^^
# Pyright: Expression with type "tuple[Literal[1], Literal[2], Literal[3]]" cannot be assigned to target tuple
# Type "tuple[Literal[1], Literal[2], Literal[3]]" is incompatible with target tuple
# Tuple size mismatch; expected 2 but received 3 [reportAssignmentType]
# red-knot: Too many values to unpack (expected 2, got 3) [lint:invalid-assignment]
```
## Test Plan
Update existing test cases TODO with the error directives.
## Summary
Ref:
3533d7f5b4 (r150651102)
This PR removes the `Ranged` implementation on `DefinitionKind` and
instead uses a method called `target_range` to avoid any confusion about
what range this is for i.e., it's not the range of the node that
represents the definition.
## Summary
Related to #13773
This PR adds support for unpacking `for` statement targets.
This involves updating the `value` field in the `Unpack` target to use
an enum which specifies the "where did the value expression came from?".
This is because for an iterable expression, we need to unpack the
iterator type while for assignment statement we need to unpack the value
type itself. And, this needs to be done in the unpack query.
### Question
One of the ways unpacking works in `for` statement is by looking at the
union of the types because if the iterable expression is a tuple then
the iterator type will be union of all the types in the tuple. This
means that the test cases that will test the unpacking in `for`
statement will also implicitly test the unpacking union logic. I was
wondering if it makes sense to merge these cases and only add the ones
that are specific to the union unpacking or for statement unpacking
logic.
## Test Plan
Add test cases involving iterating over a tuple type. I've intentionally
left out certain cases for now and I'm curious to know any thoughts on
the above query.
## Summary
This changeset adds support for precise type-inference and
boundness-handling of definitions inside control-flow branches with
statically-known conditions, i.e. test-expressions whose truthiness we
can unambiguously infer as *always false* or *always true*.
This branch also includes:
- `sys.platform` support
- statically-known branches handling for Boolean expressions and while
loops
- new `target-version` requirements in some Markdown tests which were
now required due to the understanding of `sys.version_info` branches.
closes#12700closes#15034
## Performance
### `tomllib`, -7%, needs to resolve one additional module (sys)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `./red_knot_main --project /home/shark/tomllib` | 22.2 ± 1.3 | 19.1 |
25.6 | 1.00 |
| `./red_knot_feature --project /home/shark/tomllib` | 23.8 ± 1.6 | 20.8
| 28.6 | 1.07 ± 0.09 |
### `black`, -6%
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `./red_knot_main --project /home/shark/black` | 129.3 ± 5.1 | 119.0 |
137.8 | 1.00 |
| `./red_knot_feature --project /home/shark/black` | 136.5 ± 6.8 | 123.8
| 147.5 | 1.06 ± 0.07 |
## Test Plan
- New Markdown tests for the main feature in
`statically-known-branches.md`
- New Markdown tests for `sys.platform`
- Adapted tests for `EllipsisType`, `Never`, etc
