From 9fc83d1c3190d693cc831da74081bdbabeee0ae9 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 1 Sep 2024 19:09:02 -0700 Subject: [PATCH 1/4] [red-knot] fix scope inference with deferred types --- Cargo.lock | 1 + crates/red_knot_python_semantic/src/types/infer.rs | 13 ++++++------- crates/red_knot_workspace/Cargo.toml | 1 + crates/red_knot_workspace/tests/check.rs | 14 ++++++++++++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b28e2e6f738db..87f84b32ef64d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1997,6 +1997,7 @@ dependencies = [ "ruff_text_size", "rustc-hash 2.0.0", "salsa", + "tempfile", "tracing", ] diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 2f7c7ca82c756e..f5d77c4e72a079 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -468,11 +468,8 @@ impl<'db> TypeInferenceBuilder<'db> { .as_deref() .expect("function type params scope without type params"); - // TODO: this should also be applied to parameter annotations. - if !self.is_stub() { - self.infer_optional_expression(function.returns.as_deref()); - } - + // TODO: defer annotation resolution in stubs, with __future__.annotations, or stringified + self.infer_optional_expression(function.returns.as_deref()); self.infer_type_parameters(type_params); self.infer_parameters(&function.parameters); } @@ -569,6 +566,8 @@ impl<'db> TypeInferenceBuilder<'db> { // TODO: this should also be applied to parameter annotations. if !self.is_stub() { self.infer_optional_annotation_expression(returns.as_deref()); + } else { + self.types.has_deferred = true; } } @@ -688,19 +687,19 @@ impl<'db> TypeInferenceBuilder<'db> { for base in class.bases() { self.infer_expression(base); } + } else { + self.types.has_deferred = true; } } fn infer_function_deferred(&mut self, function: &ast::StmtFunctionDef) { if self.is_stub() { - self.types.has_deferred = true; self.infer_optional_annotation_expression(function.returns.as_deref()); } } fn infer_class_deferred(&mut self, class: &ast::StmtClassDef) { if self.is_stub() { - self.types.has_deferred = true; for base in class.bases() { self.infer_expression(base); } diff --git a/crates/red_knot_workspace/Cargo.toml b/crates/red_knot_workspace/Cargo.toml index 7f6579b7877efc..dd73febde35884 100644 --- a/crates/red_knot_workspace/Cargo.toml +++ b/crates/red_knot_workspace/Cargo.toml @@ -29,6 +29,7 @@ tracing = { workspace = true } [dev-dependencies] ruff_db = { workspace = true, features = ["testing"] } +tempfile = { workspace = true } [lints] workspace = true diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index e2f8c5fd0ba5a6..3d027495c8c9b1 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -24,16 +24,26 @@ fn corpus_no_panic() -> anyhow::Result<()> { let corpus = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/test/corpus"); let system_corpus = SystemPathBuf::from_path_buf(corpus.clone()).expect("corpus path to be UTF8"); + let stub_dir = tempfile::TempDir::new()?; let db = setup_db(&system_corpus)?; for path in fs::read_dir(&corpus).expect("corpus to be a directory") { let path = path.expect("path to not be an error").path(); println!("checking {path:?}"); - let path = SystemPathBuf::from_path_buf(path.clone()).expect("path to be UTF-8"); + let syspath = SystemPathBuf::from_path_buf(path.clone()).expect("path to be UTF-8"); // this test is only asserting that we can pull every expression type without a panic // (and some non-expressions that clearly define a single type) - let file = system_path_to_file(&db, path).expect("file to exist"); + let file = system_path_to_file(&db, syspath).expect("file to exist"); + pull_types(&db, file); + // try the file as a stub also + let stub_path = stub_dir + .path() + .join(format!("{}i", path.file_name().unwrap().to_str().unwrap())); + std::fs::copy(path, stub_path.clone())?; + println!("checking {stub_path:?}"); + let syspath = SystemPathBuf::from_path_buf(stub_path).unwrap(); + let file = system_path_to_file(&db, syspath).unwrap(); pull_types(&db, file); } Ok(()) From 8d085c6aec0cd9cb136a74a9e0ec5f8369025266 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 1 Sep 2024 19:18:47 -0700 Subject: [PATCH 2/4] clippy --- crates/red_knot_python_semantic/src/types/infer.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index f5d77c4e72a079..79a9d8bf4459d5 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -564,10 +564,10 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_parameters(parameters); // TODO: this should also be applied to parameter annotations. - if !self.is_stub() { - self.infer_optional_annotation_expression(returns.as_deref()); - } else { + if self.is_stub() { self.types.has_deferred = true; + } else { + self.infer_optional_annotation_expression(returns.as_deref()); } } @@ -683,12 +683,12 @@ impl<'db> TypeInferenceBuilder<'db> { // inference of bases deferred in stubs // TODO also defer stringified generic type parameters - if !self.is_stub() { + if self.is_stub() { + self.types.has_deferred = true; + } else { for base in class.bases() { self.infer_expression(base); } - } else { - self.types.has_deferred = true; } } From 76d2fc501b1ffb167f7639e44ef9d683b090cfb6 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 3 Sep 2024 10:25:51 -0700 Subject: [PATCH 3/4] review comments --- crates/red_knot_workspace/tests/check.rs | 32 +++++++++++------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index 3d027495c8c9b1..9d23fce9aa580f 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -21,29 +21,27 @@ fn setup_db(workspace_root: &SystemPath) -> anyhow::Result { #[test] #[allow(clippy::print_stdout)] fn corpus_no_panic() -> anyhow::Result<()> { + let root = SystemPathBuf::from_path_buf(tempfile::TempDir::new()?.into_path()).unwrap(); + let db = setup_db(&root)?; + let corpus = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/test/corpus"); - let system_corpus = - SystemPathBuf::from_path_buf(corpus.clone()).expect("corpus path to be UTF8"); - let stub_dir = tempfile::TempDir::new()?; - let db = setup_db(&system_corpus)?; - - for path in fs::read_dir(&corpus).expect("corpus to be a directory") { - let path = path.expect("path to not be an error").path(); - println!("checking {path:?}"); - let syspath = SystemPathBuf::from_path_buf(path.clone()).expect("path to be UTF-8"); + + for path in fs::read_dir(&corpus)? { + let source = path?.path(); + let source_fn = source.file_name().unwrap().to_str().unwrap(); + let py_dest = root.join(source_fn); + fs::copy(&source, py_dest.as_std_path())?; + println!("checking {py_dest:?}"); // this test is only asserting that we can pull every expression type without a panic // (and some non-expressions that clearly define a single type) - let file = system_path_to_file(&db, syspath).expect("file to exist"); + let file = system_path_to_file(&db, py_dest).unwrap(); pull_types(&db, file); // try the file as a stub also - let stub_path = stub_dir - .path() - .join(format!("{}i", path.file_name().unwrap().to_str().unwrap())); - std::fs::copy(path, stub_path.clone())?; - println!("checking {stub_path:?}"); - let syspath = SystemPathBuf::from_path_buf(stub_path).unwrap(); - let file = system_path_to_file(&db, syspath).unwrap(); + let pyi_dest = root.join(format!("{source_fn}i")); + std::fs::copy(source, pyi_dest.as_std_path())?; + println!("checking {pyi_dest:?}"); + let file = system_path_to_file(&db, pyi_dest).unwrap(); pull_types(&db, file); } Ok(()) From 53bc4e139e920d6bcb372c3f8d9eda42966e06ab Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 3 Sep 2024 10:54:42 -0700 Subject: [PATCH 4/4] more useful output --- crates/red_knot_workspace/tests/check.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index 9d23fce9aa580f..cf0404c3d16b8b 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -28,19 +28,19 @@ fn corpus_no_panic() -> anyhow::Result<()> { for path in fs::read_dir(&corpus)? { let source = path?.path(); + println!("checking {source:?}"); let source_fn = source.file_name().unwrap().to_str().unwrap(); let py_dest = root.join(source_fn); fs::copy(&source, py_dest.as_std_path())?; - println!("checking {py_dest:?}"); // this test is only asserting that we can pull every expression type without a panic // (and some non-expressions that clearly define a single type) let file = system_path_to_file(&db, py_dest).unwrap(); pull_types(&db, file); // try the file as a stub also + println!("re-checking as .pyi"); let pyi_dest = root.join(format!("{source_fn}i")); std::fs::copy(source, pyi_dest.as_std_path())?; - println!("checking {pyi_dest:?}"); let file = system_path_to_file(&db, pyi_dest).unwrap(); pull_types(&db, file); }