Handle type[Any] correctly #14876

dcreager · 2024-12-09T15:16:32Z

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

github-actions · 2024-12-09T15:35:53Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

carljm

🚀

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md

crates/red_knot_python_semantic/src/types/mro.rs

* main: [`airflow`] Add fix to remove deprecated keyword arguments (`AIR302`) (#14887) Improve mdtests style (#14884) Reference `suppress-dummy-regex-options` in documentation of rules supporting it (#14888) [`flake8-bugbear`] `itertools.batched()` without explicit `strict` (`B911`) (#14408) [`ruff`] Mark autofix for `RUF052` as always unsafe (#14824) [red-knot] Improve type inference for except handlers (#14838) More typos found by codespell (#14880) [red-knot] move standalone expression_ty to TypeInferenceBuilder::file_expression_ty (#14879) [`ruff`] Do not simplify `round()` calls (`RUF046`) (#14832) Stop referring to early ruff versions (#14862) Fix a typo in `class.rs` (#14877) [`flake8-pyi`] Also remove `self` and `cls`'s annotation (`PYI034`) (#14801) [`pyupgrade`] Remove unreachable code in `UP015` implementation (#14871) [`flake8-bugbear`] Skip `B028` if `warnings.warn` is called with `*args` or `**kwargs` (#14870)

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md

crates/red_knot_python_semantic/src/types.rs

dcreager

(Initial responses, still need to make the type ⇒ type[object] change and add the equivalent_to tests)

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md

crates/red_knot_python_semantic/src/types.rs

AlexWaygood

Nice!

crates/red_knot_python_semantic/src/types.rs

crates/red_knot_python_semantic/src/types/mro.rs

* main: [red-knot] Add infrastructure to declare lints (#14873) [red-knot] Typed diagnostic id (#14869)

dcreager · 2024-12-10T18:07:16Z

crates/red_knot_python_semantic/src/types.rs

-            (Type::Unknown | Type::Any | Type::Todo(_), _) => false,
-            (_, Type::Unknown | Type::Any | Type::Todo(_)) => false,


I removed these since they (and the new Subclass(Any) form) are handled by the is_fully_static checks above.

crates/red_knot_python_semantic/src/types.rs

dcreager · 2024-12-10T18:09:05Z

crates/red_knot_python_semantic/src/types.rs

@@ -1027,7 +1084,8 @@ impl<'db> Type<'db> {
            | Type::BytesLiteral(_)
            | Type::SliceLiteral(_)
            | Type::KnownInstance(_) => true,
-            Type::ClassLiteral(_) | Type::SubclassOf(_) | Type::Instance(_) => {
+            Type::SubclassOf(SubclassOfType { base }) => matches!(base, ClassBase::Class(_)),


SubclassOf(Any) is not fully static

carljm

Looking good! In addition to the additional unit tests I mentioned inline, this PR seems a good candidate to run the quickcheck property-based tests for core type algebra invariants. These don't run in CI because they are slow, but it would be good to see whether they catch any issues here. See https://github.com/astral-sh/ruff/blob/main/crates/red_knot_python_semantic/src/types/property_tests.rs

EDIT: just realized that in order to get value from that on this PR, we should also add SubclassOf types to the arbitrary_core_type function there!

crates/red_knot_python_semantic/src/types.rs

AlexWaygood · 2024-12-10T18:18:27Z

crates/red_knot_python_semantic/src/types.rs

+            (
+                Type::Instance(InstanceType { class: self_class }),
+                Type::Instance(InstanceType {
+                    class: target_class,
+                }),
+            ) if {
+                let self_known = self_class.known(db);
+                matches!(
+                    self_known,
+                    Some(KnownClass::NoneType | KnownClass::NoDefaultType)
+                ) && self_known == target_class.known(db)
+            } =>
+            {
+                true
+            }


this gets a little hard to read 😄 how about writing it like this?

Suggested change

(

Type::Instance(InstanceType { class: self_class }),

Type::Instance(InstanceType {

class: target_class,

}),

) if {

let self_known = self_class.known(db);

matches!(

self_known,

Some(KnownClass::NoneType | KnownClass::NoDefaultType)

) && self_known == target_class.known(db)

} =>

{

true

}

(Type::Instance(self_instance), Type::Instance(target_instance)) => {

let Some(self_known_class) = self_instance.class.known(db) else {

return false;

};

if !matches!(

self_known_class,

KnownClass::NoneType | KnownClass::NoDefaultType

) {

return false;

}

Some(self_known_class) == target_instance.class.known(db)

}

We have to be careful here; these return false in the suggested change are not correct. They would have to be return self == other, and I'm not sure we want to duplicate that condition in various places.

argh, great spot. thanks.

I had also considered handling each of the special cases with a separate if statement + early return. Then it would possibly be clearer that self == other is the fall-back / general case. But we had other large match blocks elsewhere that made me think that might be preferred house style. I will take a stab at the early return pattern and see if it's more readable.

I will take a stab at the early return pattern and see if it's more readable.

Now using early return. lmkwyt

AlexWaygood · 2024-12-10T18:20:01Z

crates/red_knot_python_semantic/src/types.rs

+            ) if object_class.is_known(db, KnownClass::Object)
+                && type_class.is_known(db, KnownClass::Type) =>
+            {
+                true
+            }


Suggested change

) if object_class.is_known(db, KnownClass::Object)

&& type_class.is_known(db, KnownClass::Type) =>

{

true

}

) => {

object_class.is_known(db, KnownClass::Object)

&& type_class.is_known(db, KnownClass::Type)

}

This one should be safe because a Type::Instance can't be == to a Type::SubclassOf.

crates/red_knot_python_semantic/src/types.rs

sharkdp · 2024-12-10T19:34:03Z

In addition to the additional unit tests I mentioned inline, this PR seems a good candidate to run the quickcheck property-based tests for core type algebra invariants. These don't run in CI because they are slow, but it would be good to see whether they catch any issues here. See https://github.com/astral-sh/ruff/blob/main/crates/red_knot_python_semantic/src/types/property_tests.rs

I already did this. I didn't mention it here because they are currently broken (surprise!). I'll put up a small PR with a fix.

Edit: #14897

* main: [red-knot] split call-outcome enums to their own submodule (#14898) [red-knot] Property tests: account non-fully-static types (#14897) ruff: add worktree support to build.rs (#14896)

sharkdp

Thank you for all the updates and the new tests!

FWIW, I did successfully run the property tests on your branch after cherry-picking my fix in #14897 and applying the following patch, where the 4=>3 change is a weird way to suppress the creation of intersection types to prevent running into #14899:

diff --git a/crates/red_knot_python_semantic/src/types/property_tests.rs b/crates/red_knot_python_semantic/src/types/property_tests.rs
index 15f4d5648..e6b223034 100644
--- a/crates/red_knot_python_semantic/src/types/property_tests.rs
+++ b/crates/red_knot_python_semantic/src/types/property_tests.rs
@@ -64,6 +64,10 @@ fn arbitrary_core_type(g: &mut Gen) -> Ty {
         Ty::BuiltinClassLiteral("int"),
         Ty::BuiltinClassLiteral("bool"),
         Ty::BuiltinClassLiteral("object"),
+        Ty::SubclassOfAny,
+        Ty::SubclassOfBuiltinClass("object"),
+        Ty::SubclassOfBuiltinClass("int"),
+        Ty::SubclassOfBuiltinClass("bool"),
     ])
     .unwrap()
     .clone()
@@ -78,7 +82,7 @@ fn arbitrary_type(g: &mut Gen, size: u32) -> Ty {
     if size == 0 {
         arbitrary_core_type(g)
     } else {
-        match u32::arbitrary(g) % 4 {
+        match u32::arbitrary(g) % 3 {
             0 => arbitrary_core_type(g),
             1 => Ty::Union(
                 (0..*g.choose(&[2, 3]).unwrap())

dcreager

I did successfully run the property tests on your branch

Thanks @sharkdp, was just looking at the property tests myself and running into the assignable bug

carljm

Looks good to me!!

AlexWaygood added the red-knot Multi-file analysis & type inference label Dec 9, 2024

dcreager force-pushed the dcreager/type-of-any branch from ba56f82 to 839f7a4 Compare December 9, 2024 16:57

dcreager added 2 commits December 9, 2024 14:17

Handle type[Any] correctly

cdfbfa7

Treat type as a synonym for type[Any]

4ac0df3

dcreager force-pushed the dcreager/type-of-any branch from 839f7a4 to 4ac0df3 Compare December 9, 2024 19:17

Add test cases for type[object]

b65a156

dcreager marked this pull request as ready for review December 9, 2024 19:45

dcreager requested review from carljm, MichaReiser, AlexWaygood and sharkdp as code owners December 9, 2024 19:45

carljm approved these changes Dec 9, 2024

View reviewed changes

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md Show resolved Hide resolved

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md Show resolved Hide resolved

sharkdp reviewed Dec 9, 2024

View reviewed changes

crates/red_knot_python_semantic/resources/mdtest/type_of/any.md Outdated Show resolved Hide resolved

sharkdp reviewed Dec 9, 2024

View reviewed changes

crates/red_knot_python_semantic/src/types/mro.rs Outdated Show resolved Hide resolved

dcreager added 5 commits December 10, 2024 09:12

Use variable assignments in tests

683f901

Handle assignability correctly

ebb3031

Remove unused function

3095cfd

Add bound method TODOs

3c1043d

carljm reviewed Dec 10, 2024

View reviewed changes

Add failing mdtests

24d1fdc

dcreager commented Dec 10, 2024

View reviewed changes

AlexWaygood reviewed Dec 10, 2024

View reviewed changes

crates/red_knot_python_semantic/src/types.rs Outdated Show resolved Hide resolved

crates/red_knot_python_semantic/src/types/mro.rs Outdated Show resolved Hide resolved

dcreager added 6 commits December 10, 2024 12:33

Inline and remove ClassBase::is_subtype_of

d47b4cc

Reuse existing clause for C ⊆ type

2ec0bcc

Fix is_fully_static for SubclassOf(Any)

e651e7d

Remove superfluous clause

42a2233

Rename SubclassOfClass

5fd4217

Subclass[object] ⇔ Instance[type]

9213222

dcreager added 3 commits December 10, 2024 12:50

Interpret type as "instance of type", aka type[object]

1f944c2

Verify that equivalence is symmetric

a6a1a3d

Merge branch 'main' into dcreager/type-of-any

113af14

* main: [red-knot] Add infrastructure to declare lints (#14873) [red-knot] Typed diagnostic id (#14869)

dcreager commented Dec 10, 2024

View reviewed changes

carljm reviewed Dec 10, 2024

View reviewed changes

crates/red_knot_python_semantic/src/types.rs Show resolved Hide resolved

AlexWaygood reviewed Dec 10, 2024

View reviewed changes

dcreager added 4 commits December 10, 2024 15:08

Renames per Alex

62da549

Add tests for is_fully_static

20bbaaf

Add property test constructors for new test types

72f09c2

Merge branch 'main' into dcreager/type-of-any

ae96c9f

* main: [red-knot] split call-outcome enums to their own submodule (#14898) [red-knot] Property tests: account non-fully-static types (#14897) ruff: add worktree support to build.rs (#14896)

sharkdp approved these changes Dec 10, 2024

View reviewed changes

dcreager commented Dec 10, 2024

View reviewed changes

dcreager added 2 commits December 10, 2024 15:28

Use early return in is_equivalent_to

4eddff6

Add comment

1148f96

carljm approved these changes Dec 10, 2024

View reviewed changes

dcreager mentioned this pull request Dec 10, 2024

[red-knot] support type[Any] #14546

Closed

dcreager merged commit d4126f6 into main Dec 10, 2024
21 checks passed

dcreager deleted the dcreager/type-of-any branch December 10, 2024 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle type[Any] correctly #14876

Handle type[Any] correctly #14876

dcreager commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

carljm left a comment

dcreager left a comment

AlexWaygood left a comment

dcreager Dec 10, 2024

dcreager Dec 10, 2024

carljm left a comment •

edited

Loading

AlexWaygood Dec 10, 2024

carljm Dec 10, 2024

AlexWaygood Dec 10, 2024

dcreager Dec 10, 2024

dcreager Dec 10, 2024

AlexWaygood Dec 10, 2024

carljm Dec 10, 2024

sharkdp commented Dec 10, 2024 •

edited

Loading

sharkdp left a comment •

edited

Loading

dcreager left a comment

carljm left a comment

		(Type::Unknown \| Type::Any \| Type::Todo(_), _) => false,
		(_, Type::Unknown \| Type::Any \| Type::Todo(_)) => false,

Handle type[Any] correctly #14876

Handle type[Any] correctly #14876

Conversation

dcreager commented Dec 9, 2024 • edited Loading

github-actions bot commented Dec 9, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

carljm left a comment

Choose a reason for hiding this comment

dcreager left a comment

Choose a reason for hiding this comment

AlexWaygood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carljm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp commented Dec 10, 2024 • edited Loading

sharkdp left a comment • edited Loading

Choose a reason for hiding this comment

dcreager left a comment

Choose a reason for hiding this comment

carljm left a comment

Choose a reason for hiding this comment

dcreager commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

`ruff-ecosystem` results

carljm left a comment •

edited

Loading

sharkdp commented Dec 10, 2024 •

edited

Loading

sharkdp left a comment •

edited

Loading