Skip to content

Commit

Permalink
Eagerly validate typeshed versions
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Aug 14, 2024
1 parent c487149 commit 4e2b2d7
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 200 deletions.
1 change: 0 additions & 1 deletion crates/red_knot_python_semantic/src/module_resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use resolver::SearchPathIterator;
mod module;
mod path;
mod resolver;
mod state;
mod typeshed;

#[cfg(test)]
Expand Down
117 changes: 50 additions & 67 deletions crates/red_knot_python_semantic/src/module_resolver/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ use ruff_db::files::{system_path_to_file, vendored_path_to_file, File, FileError
use ruff_db::system::{System, SystemPath, SystemPathBuf};
use ruff_db::vendored::{VendoredPath, VendoredPathBuf};

use super::typeshed::{typeshed_versions, TypeshedVersionsParseError, TypeshedVersionsQueryResult};
use crate::db::Db;
use crate::module_name::ModuleName;

use super::state::ResolverState;
use super::typeshed::{TypeshedVersionsParseError, TypeshedVersionsQueryResult};
use crate::module_resolver::resolver::ResolverContext;

/// A path that points to a Python module.
///
Expand Down Expand Up @@ -60,7 +59,7 @@ impl ModulePath {
}

#[must_use]
pub(crate) fn is_directory(&self, resolver: &ResolverState) -> bool {
pub(super) fn is_directory(&self, resolver: &ResolverContext) -> bool {
let ModulePath {
search_path,
relative_path,
Expand All @@ -74,7 +73,7 @@ impl ModulePath {
== Err(FileError::IsADirectory)
}
SearchPathInner::StandardLibraryCustom(stdlib_root) => {
match query_stdlib_version(Some(stdlib_root), relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => false,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => {
Expand All @@ -84,7 +83,7 @@ impl ModulePath {
}
}
SearchPathInner::StandardLibraryVendored(stdlib_root) => {
match query_stdlib_version(None, relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => false,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => resolver
Expand All @@ -96,7 +95,7 @@ impl ModulePath {
}

#[must_use]
pub(crate) fn is_regular_package(&self, resolver: &ResolverState) -> bool {
pub(super) fn is_regular_package(&self, resolver: &ResolverContext) -> bool {
let ModulePath {
search_path,
relative_path,
Expand All @@ -113,7 +112,7 @@ impl ModulePath {
.is_ok()
}
SearchPathInner::StandardLibraryCustom(search_path) => {
match query_stdlib_version(Some(search_path), relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => false,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => system_path_to_file(
Expand All @@ -124,7 +123,7 @@ impl ModulePath {
}
}
SearchPathInner::StandardLibraryVendored(search_path) => {
match query_stdlib_version(None, relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => false,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => resolver
Expand All @@ -136,7 +135,7 @@ impl ModulePath {
}

#[must_use]
pub(crate) fn to_file(&self, resolver: &ResolverState) -> Option<File> {
pub(super) fn to_file(&self, resolver: &ResolverContext) -> Option<File> {
let db = resolver.db.upcast();
let ModulePath {
search_path,
Expand All @@ -150,7 +149,7 @@ impl ModulePath {
system_path_to_file(db, search_path.join(relative_path)).ok()
}
SearchPathInner::StandardLibraryCustom(stdlib_root) => {
match query_stdlib_version(Some(stdlib_root), relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => None,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => {
Expand All @@ -159,7 +158,7 @@ impl ModulePath {
}
}
SearchPathInner::StandardLibraryVendored(stdlib_root) => {
match query_stdlib_version(None, relative_path, resolver) {
match query_stdlib_version(relative_path, resolver) {
TypeshedVersionsQueryResult::DoesNotExist => None,
TypeshedVersionsQueryResult::Exists
| TypeshedVersionsQueryResult::MaybeExists => {
Expand Down Expand Up @@ -273,27 +272,23 @@ fn stdlib_path_to_module_name(relative_path: &Utf8Path) -> Option<ModuleName> {

#[must_use]
fn query_stdlib_version(
custom_stdlib_root: Option<&SystemPath>,
relative_path: &Utf8Path,
resolver: &ResolverState,
context: &ResolverContext,
) -> TypeshedVersionsQueryResult {
let Some(module_name) = stdlib_path_to_module_name(relative_path) else {
return TypeshedVersionsQueryResult::DoesNotExist;
};
let ResolverState {
db,
typeshed_versions,
target_version,
} = resolver;
typeshed_versions.query_module(*db, &module_name, custom_stdlib_root, *target_version)
let ResolverContext { db, target_version } = context;

typeshed_versions(*db).query_module(&module_name, *target_version)
}

/// Enumeration describing the various ways in which validation of a search path might fail.
///
/// If validation fails for a search path derived from the user settings,
/// a message must be displayed to the user,
/// as type checking cannot be done reliably in these circumstances.
#[derive(Debug, PartialEq, Eq)]
#[derive(Debug)]
pub(crate) enum SearchPathValidationError {
/// The path provided by the user was not a directory
NotADirectory(SystemPathBuf),
Expand All @@ -304,13 +299,12 @@ pub(crate) enum SearchPathValidationError {
NoStdlibSubdirectory(SystemPathBuf),

/// The typeshed path provided by the user is a directory,
/// but no `stdlib/VERSIONS` file exists.
/// (This is only relevant for stdlib search paths.)
NoVersionsFile(SystemPathBuf),

/// `stdlib/VERSIONS` is a directory.
/// but `stdlib/VERSIONS` could not be read
/// (This is only relevant for stdlib search paths.)
VersionsIsADirectory(SystemPathBuf),
FailedToReadVersionsFile {
path: SystemPathBuf,
error: std::io::Error,
},

/// The path provided by the user is a directory,
/// and a `stdlib/VERSIONS` file exists, but it fails to parse.
Expand All @@ -325,8 +319,9 @@ impl fmt::Display for SearchPathValidationError {
Self::NoStdlibSubdirectory(path) => {
write!(f, "The directory at {path} has no `stdlib/` subdirectory")
}
Self::NoVersionsFile(path) => write!(f, "Expected a file at {path}/stdlib/VERSIONS"),
Self::VersionsIsADirectory(path) => write!(f, "{path}/stdlib/VERSIONS is a directory."),
Self::FailedToReadVersionsFile { path, error } => {
write!(f, "Failed to read the '{path}' file: {error}")
}
Self::VersionsParseError(underlying_error) => underlying_error.fmt(f),
}
}
Expand Down Expand Up @@ -384,11 +379,10 @@ pub(crate) struct SearchPath(Arc<SearchPathInner>);

impl SearchPath {
fn directory_path(system: &dyn System, root: SystemPathBuf) -> SearchPathResult<SystemPathBuf> {
let canonicalized = system.canonicalize_path(&root).unwrap_or(root);
if system.is_directory(&canonicalized) {
Ok(canonicalized)
if system.is_directory(&root) {
Ok(root)
} else {
Err(SearchPathValidationError::NotADirectory(canonicalized))
Err(SearchPathValidationError::NotADirectory(root))
}
}

Expand All @@ -407,32 +401,22 @@ impl SearchPath {
}

/// Create a new standard-library search path pointing to a custom directory on disk
pub(crate) fn custom_stdlib(db: &dyn Db, typeshed: SystemPathBuf) -> SearchPathResult<Self> {
pub(crate) fn custom_stdlib(db: &dyn Db, typeshed: &SystemPath) -> SearchPathResult<Self> {
let system = db.system();
if !system.is_directory(&typeshed) {
if !system.is_directory(typeshed) {
return Err(SearchPathValidationError::NotADirectory(
typeshed.to_path_buf(),
));
}

let stdlib =
Self::directory_path(system, typeshed.join("stdlib")).map_err(|err| match err {
SearchPathValidationError::NotADirectory(path) => {
SearchPathValidationError::NoStdlibSubdirectory(path)
SearchPathValidationError::NotADirectory(_) => {
SearchPathValidationError::NoStdlibSubdirectory(typeshed.to_path_buf())
}
err => err,
})?;
let typeshed_versions =
system_path_to_file(db.upcast(), stdlib.join("VERSIONS")).map_err(|err| match err {
FileError::NotFound => SearchPathValidationError::NoVersionsFile(typeshed),
FileError::IsADirectory => {
SearchPathValidationError::VersionsIsADirectory(typeshed)
}
})?;
super::typeshed::parse_typeshed_versions(db, typeshed_versions)
.as_ref()
.map_err(|validation_error| {
SearchPathValidationError::VersionsParseError(validation_error.clone())
})?;

Ok(Self(Arc::new(SearchPathInner::StandardLibraryCustom(
stdlib,
))))
Expand Down Expand Up @@ -623,11 +607,11 @@ mod tests {
use ruff_db::Db;

use crate::db::tests::TestDb;

use super::*;
use crate::module_resolver::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder};
use crate::python_version::PythonVersion;

use super::*;

impl ModulePath {
#[must_use]
fn join(&self, component: &str) -> ModulePath {
Expand Down Expand Up @@ -661,15 +645,15 @@ mod tests {
.build();

assert_eq!(
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf())
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap())
.unwrap()
.to_module_path()
.with_py_extension(),
None
);

assert_eq!(
&SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf())
&SearchPath::custom_stdlib(&db, stdlib.parent().unwrap())
.unwrap()
.join("foo")
.with_pyi_extension(),
Expand Down Expand Up @@ -780,7 +764,7 @@ mod tests {
let TestCase { db, stdlib, .. } = TestCaseBuilder::new()
.with_custom_typeshed(MockedTypeshed::default())
.build();
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf())
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap())
.unwrap()
.to_module_path()
.push("bar.py");
Expand All @@ -792,7 +776,7 @@ mod tests {
let TestCase { db, stdlib, .. } = TestCaseBuilder::new()
.with_custom_typeshed(MockedTypeshed::default())
.build();
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf())
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap())
.unwrap()
.to_module_path()
.push("bar.rs");
Expand Down Expand Up @@ -824,7 +808,7 @@ mod tests {
.with_custom_typeshed(MockedTypeshed::default())
.build();

let root = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()).unwrap();
let root = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()).unwrap();

// Must have a `.pyi` extension or no extension:
let bad_absolute_path = SystemPath::new("foo/stdlib/x.py");
Expand Down Expand Up @@ -872,8 +856,7 @@ mod tests {
.with_custom_typeshed(typeshed)
.with_target_version(target_version)
.build();
let stdlib =
SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()).unwrap();
let stdlib = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()).unwrap();
(db, stdlib)
}

Expand All @@ -898,7 +881,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let asyncio_regular_package = stdlib_path.join("asyncio");
assert!(asyncio_regular_package.is_directory(&resolver));
Expand Down Expand Up @@ -926,7 +909,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let xml_namespace_package = stdlib_path.join("xml");
assert!(xml_namespace_package.is_directory(&resolver));
Expand All @@ -948,7 +931,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let functools_module = stdlib_path.join("functools.pyi");
assert!(functools_module.to_file(&resolver).is_some());
Expand All @@ -964,7 +947,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let collections_regular_package = stdlib_path.join("collections");
assert_eq!(collections_regular_package.to_file(&resolver), None);
Expand All @@ -980,7 +963,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let importlib_namespace_package = stdlib_path.join("importlib");
assert_eq!(importlib_namespace_package.to_file(&resolver), None);
Expand All @@ -1001,7 +984,7 @@ mod tests {
};

let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY38);
let resolver = ResolverContext::new(&db, PythonVersion::PY38);

let non_existent = stdlib_path.join("doesnt_even_exist");
assert_eq!(non_existent.to_file(&resolver), None);
Expand Down Expand Up @@ -1029,7 +1012,7 @@ mod tests {
};

let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY39);
let resolver = ResolverContext::new(&db, PythonVersion::PY39);

// Since we've set the target version to Py39,
// `collections` should now exist as a directory, according to VERSIONS...
Expand Down Expand Up @@ -1058,7 +1041,7 @@ mod tests {
};

let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY39);
let resolver = ResolverContext::new(&db, PythonVersion::PY39);

// The `importlib` directory now also exists
let importlib_namespace_package = stdlib_path.join("importlib");
Expand All @@ -1082,7 +1065,7 @@ mod tests {
};

let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED);
let resolver = ResolverState::new(&db, PythonVersion::PY39);
let resolver = ResolverContext::new(&db, PythonVersion::PY39);

// The `xml` package no longer exists on py39:
let xml_namespace_package = stdlib_path.join("xml");
Expand Down
Loading

0 comments on commit 4e2b2d7

Please sign in to comment.