From ea7bb199bc4ce13bde6fa5a27d216744d6e4940e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 28 Jun 2023 10:50:54 -0400 Subject: [PATCH] Fill-in missing implementation for `is_native_module_file_name` (#5410) ## Summary This was just an oversight -- the last remaining `todo!()` that I never filled in. We clearly don't have any test coverage for it yet, but this mimics the Pyright implementation. --- .../src/implicit_imports.rs | 7 +- .../ruff_python_resolver/src/native_module.rs | 65 ++++++++++++++++++- crates/ruff_python_resolver/src/resolver.rs | 37 +++++++---- 3 files changed, 86 insertions(+), 23 deletions(-) diff --git a/crates/ruff_python_resolver/src/implicit_imports.rs b/crates/ruff_python_resolver/src/implicit_imports.rs index 25a7d040c6..926c947901 100644 --- a/crates/ruff_python_resolver/src/implicit_imports.rs +++ b/crates/ruff_python_resolver/src/implicit_imports.rs @@ -64,12 +64,7 @@ pub(crate) fn find(dir_path: &Path, exclusions: &[&Path]) -> BTreeMap bool { file_extension == "so" || file_extension == "pyd" || file_extension == "dylib" } -/// Returns `true` if the given file name is that of a native module. -pub(crate) fn is_native_module_file_name(_module_name: &Path, _file_name: &Path) -> bool { - todo!() +/// Given a file name, returns the name of the native module it represents. +/// +/// For example, given `foo.abi3.so`, return `foo`. +pub(crate) fn native_module_name(file_name: &Path) -> Option<&str> { + file_name + .file_stem() + .and_then(OsStr::to_str) + .map(|file_stem| { + file_stem + .split_once('.') + .map_or(file_stem, |(file_stem, _)| file_stem) + }) +} + +/// Returns `true` if the given file name is that of a native module with the given name. +pub(crate) fn is_native_module_file_name(module_name: &str, file_name: &Path) -> bool { + // The file name must be that of a native module. + if !file_name + .extension() + .map_or(false, is_native_module_file_extension) + { + return false; + }; + + // The file must represent the module name. + native_module_name(file_name) == Some(module_name) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + #[test] + fn module_name() { + assert_eq!( + super::native_module_name(&PathBuf::from("foo.so")), + Some("foo") + ); + + assert_eq!( + super::native_module_name(&PathBuf::from("foo.abi3.so")), + Some("foo") + ); + + assert_eq!( + super::native_module_name(&PathBuf::from("foo.cpython-38-x86_64-linux-gnu.so")), + Some("foo") + ); + + assert_eq!( + super::native_module_name(&PathBuf::from("foo.cp39-win_amd64.pyd")), + Some("foo") + ); + } + + #[test] + fn module_file_extension() { + assert!(super::is_native_module_file_extension("so".as_ref())); + assert!(super::is_native_module_file_extension("pyd".as_ref())); + assert!(super::is_native_module_file_extension("dylib".as_ref())); + assert!(!super::is_native_module_file_extension("py".as_ref())); + } } diff --git a/crates/ruff_python_resolver/src/resolver.rs b/crates/ruff_python_resolver/src/resolver.rs index ee845f6839..3cb3eca260 100644 --- a/crates/ruff_python_resolver/src/resolver.rs +++ b/crates/ruff_python_resolver/src/resolver.rs @@ -1,6 +1,7 @@ //! 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; @@ -138,18 +139,21 @@ 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(native_lib_path) = dir_path - .read_dir() - .unwrap() - .flatten() - .filter(|entry| entry.file_type().map_or(false, |ft| ft.is_file())) - .find(|entry| { - native_module::is_native_module_file_name(&dir_path, &entry.path()) - }) - { - debug!("Resolved import with file: {native_lib_path:?}"); - is_native_lib = true; - resolved_paths.push(native_lib_path.path()); + 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); + } } } @@ -427,8 +431,13 @@ fn is_namespace_package_resolved( implicit_imports: &BTreeMap, ) -> bool { if !module_descriptor.imported_symbols.is_empty() { - // Pyright uses `!Array.from(moduleDescriptor.importedSymbols.keys()).some((symbol) => implicitImports.has(symbol))`. - // But that only checks if any of the symbols are in the implicit imports? + // TODO(charlie): Pyright uses: + // + // ```typescript + // !Array.from(moduleDescriptor.importedSymbols.keys()).some((symbol) => implicitImports.has(symbol))` + // ``` + // + // However, that only checks if _any_ of the symbols are in the implicit imports. for symbol in &module_descriptor.imported_symbols { if !implicit_imports.contains_key(symbol) { return false;