From 864f50a3a43b4e332fb62fbfe56016ef36822af9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 28 Jun 2023 14:06:17 -0400 Subject: [PATCH] Remove all `unwrap` calls from the resolver (#5426) --- .../src/implicit_imports.rs | 9 +--- .../ruff_python_resolver/src/native_module.rs | 14 ++++- crates/ruff_python_resolver/src/resolver.rs | 20 ++----- crates/ruff_python_resolver/src/search.rs | 54 ++++++++++--------- 4 files changed, 47 insertions(+), 50 deletions(-) diff --git a/crates/ruff_python_resolver/src/implicit_imports.rs b/crates/ruff_python_resolver/src/implicit_imports.rs index 926c947901..76191b87b6 100644 --- a/crates/ruff_python_resolver/src/implicit_imports.rs +++ b/crates/ruff_python_resolver/src/implicit_imports.rs @@ -55,14 +55,7 @@ pub(crate) fn find(dir_path: &Path, exclusions: &[&Path]) -> BTreeMap bool { @@ -36,6 +36,18 @@ pub(crate) fn is_native_module_file_name(module_name: &str, file_name: &Path) -> native_module_name(file_name) == Some(module_name) } +/// Find the native module within the namespace package at the given path. +pub(crate) fn find_native_module(dir_path: &Path) -> Option { + let module_name = dir_path.file_name()?.to_str()?; + dir_path + .read_dir() + .ok()? + .flatten() + .filter(|entry| entry.file_type().map_or(false, |ft| ft.is_file())) + .map(|entry| entry.path()) + .find(|path| is_native_module_file_name(module_name, path)) +} + #[cfg(test)] mod tests { use std::path::PathBuf; diff --git a/crates/ruff_python_resolver/src/resolver.rs b/crates/ruff_python_resolver/src/resolver.rs index e175edff44..d10b7fbe83 100644 --- a/crates/ruff_python_resolver/src/resolver.rs +++ b/crates/ruff_python_resolver/src/resolver.rs @@ -1,7 +1,6 @@ //! Resolves Python imports to their corresponding files on disk. use std::collections::BTreeMap; -use std::ffi::OsStr; use std::path::{Path, PathBuf}; use log::debug; @@ -139,21 +138,10 @@ fn resolve_module_descriptor( } else { if allow_native_lib && dir_path.is_dir() { // We couldn't find a `.py[i]` file; search for a native library. - if let Some(module_name) = dir_path.file_name().and_then(OsStr::to_str) { - if let Some(native_lib_path) = dir_path - .read_dir() - .unwrap() - .flatten() - .filter(|entry| entry.file_type().map_or(false, |ft| ft.is_file())) - .map(|entry| entry.path()) - .find(|path| { - native_module::is_native_module_file_name(module_name, path) - }) - { - debug!("Resolved import with file: {native_lib_path:?}"); - is_native_lib = true; - resolved_paths.push(native_lib_path); - } + if let Some(native_lib_path) = native_module::find_native_module(&dir_path) { + debug!("Resolved import with file: {native_lib_path:?}"); + is_native_lib = true; + resolved_paths.push(native_lib_path); } } diff --git a/crates/ruff_python_resolver/src/search.rs b/crates/ruff_python_resolver/src/search.rs index 01c6114ae4..0539c8296a 100644 --- a/crates/ruff_python_resolver/src/search.rs +++ b/crates/ruff_python_resolver/src/search.rs @@ -2,8 +2,8 @@ use std::collections::HashMap; use std::ffi::OsStr; -use std::fs; use std::path::{Path, PathBuf}; +use std::{fs, io}; use log::debug; @@ -94,9 +94,9 @@ fn find_site_packages_path( Some(default_dir.join(SITE_PACKAGES)) } -fn find_paths_from_pth_files(parent_dir: &Path) -> Vec { - fs::read_dir(parent_dir) - .unwrap() +fn find_paths_from_pth_files(parent_dir: &Path) -> io::Result + '_> { + Ok(parent_dir + .read_dir()? .flatten() .filter(|entry| { // Collect all *.pth files. @@ -130,8 +130,7 @@ fn find_paths_from_pth_files(parent_dir: &Path) -> Vec { } } None - }) - .collect() + })) } /// Find the Python search paths for the given virtual environment. @@ -144,7 +143,9 @@ fn find_python_search_paths(config: &Config, host: &Host) -> V let lib_path = venv_path.join(venv).join(lib_name); if let Some(site_packages_path) = find_site_packages_path(&lib_path, None) { // Add paths from any `.pth` files in each of the `site-packages` directories. - found_paths.extend(find_paths_from_pth_files(&site_packages_path)); + if let Ok(pth_paths) = find_paths_from_pth_files(&site_packages_path) { + found_paths.extend(pth_paths); + } // Add the `site-packages` directory to the search path. found_paths.push(site_packages_path); @@ -212,45 +213,48 @@ fn typeshed_subdirectory( /// Generate a map from PyPI-registered package name to a list of paths /// containing the package's stubs. -fn build_typeshed_third_party_package_map(third_party_dir: &Path) -> HashMap> { +fn build_typeshed_third_party_package_map( + third_party_dir: &Path, +) -> io::Result>> { let mut package_map = HashMap::new(); // Iterate over every directory. - for outer_entry in fs::read_dir(third_party_dir).unwrap() { - let outer_entry = outer_entry.unwrap(); - if outer_entry.file_type().unwrap().is_dir() { + for outer_entry in fs::read_dir(third_party_dir)? { + let outer_entry = outer_entry?; + if outer_entry.file_type()?.is_dir() { // Iterate over any subdirectory children. - for inner_entry in fs::read_dir(outer_entry.path()).unwrap() { - let inner_entry = inner_entry.unwrap(); + for inner_entry in fs::read_dir(outer_entry.path())? { + let inner_entry = inner_entry?; - if inner_entry.file_type().unwrap().is_dir() { + if inner_entry.file_type()?.is_dir() { package_map .entry(inner_entry.file_name().to_string_lossy().to_string()) .or_insert_with(Vec::new) .push(outer_entry.path()); - } else if inner_entry.file_type().unwrap().is_file() { + } else if inner_entry.file_type()?.is_file() { if inner_entry .path() .extension() .map_or(false, |extension| extension == "pyi") { - let stripped_file_name = inner_entry + if let Some(stripped_file_name) = inner_entry .path() .file_stem() - .unwrap() - .to_string_lossy() - .to_string(); - package_map - .entry(stripped_file_name) - .or_insert_with(Vec::new) - .push(outer_entry.path()); + .and_then(std::ffi::OsStr::to_str) + .map(std::string::ToString::to_string) + { + package_map + .entry(stripped_file_name) + .or_insert_with(Vec::new) + .push(outer_entry.path()); + } } } } } } - package_map + Ok(package_map) } /// Determine the current `typeshed` subdirectory for a third-party package. @@ -260,7 +264,7 @@ pub(crate) fn third_party_typeshed_package_paths( host: &Host, ) -> Option> { let typeshed_path = typeshed_subdirectory(false, config, host)?; - let package_paths = build_typeshed_third_party_package_map(&typeshed_path); + let package_paths = build_typeshed_third_party_package_map(&typeshed_path).ok()?; let first_name_part = module_descriptor.name_parts.first().map(String::as_str)?; package_paths.get(first_name_part).cloned() }