From 5f96f69151568da8300fe6d3bf513ae4da3ee6ba Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 19 Jul 2024 13:53:09 +0100 Subject: [PATCH] [red-knot] Fix bug where module resolution would not be invalidated if an entire package was deleted (#12378) --- crates/red_knot_module_resolver/src/path.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_module_resolver/src/path.rs b/crates/red_knot_module_resolver/src/path.rs index 073dcfe04c5da..87516173065a2 100644 --- a/crates/red_knot_module_resolver/src/path.rs +++ b/crates/red_knot_module_resolver/src/path.rs @@ -369,10 +369,9 @@ impl<'a> ModuleResolutionPathRefInner<'a> { #[must_use] fn is_regular_package(&self, search_path: Self, resolver: &ResolverState) -> bool { - fn is_non_stdlib_pkg(state: &ResolverState, path: &SystemPath) -> bool { - let file_system = state.system(); - file_system.path_exists(&path.join("__init__.py")) - || file_system.path_exists(&path.join("__init__.pyi")) + fn is_non_stdlib_pkg(resolver: &ResolverState, path: &SystemPath) -> bool { + system_path_to_file(resolver.db.upcast(), path.join("__init__.py")).is_some() + || system_path_to_file(resolver.db.upcast(), path.join("__init__.py")).is_some() } match (self, search_path) { @@ -387,8 +386,13 @@ impl<'a> ModuleResolutionPathRefInner<'a> { match Self::query_stdlib_version( path, search_path, &stdlib_root, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => match path { - FilePathRef::System(path) => resolver.db.system().path_exists(&path.join("__init__.pyi")), - FilePathRef::Vendored(path) => resolver.db.vendored().exists(path.join("__init__.pyi")), + FilePathRef::System(path) => system_path_to_file(resolver.db.upcast(),path.join("__init__.pyi")).is_some(), + // No need to use `vendored_path_to_file` here: + // (1) The vendored filesystem is immutable, so we don't need to worry about Salsa invalidation + // (2) The caching Salsa provides probably won't speed us up that much + // (TODO: check that assumption when we're able to run red-knot on larger code bases) + // (3) We don't need the `File` object that `vendored_path_to_file` would return; we just need to know if the file exists + FilePathRef::Vendored(path) => resolver.db.vendored().exists(path.join("__init__.pyi")) }, } }