From 4513feeabf8117121ce8ea79b51bb4557d78991b Mon Sep 17 00:00:00 2001 From: Ching-Wei Kang Date: Fri, 11 Sep 2026 14:52:47 -0500 Subject: [PATCH] [pyrefly] Diagnose directory-relative imports Imports that resolve only through the directory-relative fallback can work when a file is run directly but fail when it is loaded as part of a package. Attach a typed, non-fatal diagnostic at that narrow resolver boundary while preserving normal search precedence and keeping explicit fallback roots silent. Map the BasedPyright compatibility rule to the new error kind, document it, and cover resolver, configuration, and end-to-end behavior. Signed-off-by: Ching-Wei Kang --- crates/pyrefly_config/src/error_kind.rs | 3 + .../pyrefly_config/src/migration/pyright.rs | 16 ++- pyrefly/lib/module/finder.rs | 109 +++++++++++++++++- pyrefly/lib/state/loader.rs | 13 +++ pyrefly/lib/test/state.rs | 45 ++++++++ scripts/error_presets.json | 1 + website/docs/error-kinds.mdx | 19 +++ .../migrate/pyright/diagnostics-reference.mdx | 2 +- 8 files changed, 205 insertions(+), 3 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 719edff13a..545b5b9d3c 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -213,6 +213,8 @@ pub enum ErrorKind { /// only re-exported when redundantly aliased (`from x import y as y`), /// listed in `__all__`, or brought in via a wildcard import. ImplicitReexport, + /// An unqualified import that resolves only relative to the importing file. + ImplicitRelativeImport, /// An attribute was implicitly defined by assignment to `self` in a method that we /// do not recognize as always executing (we recognize constructors and some test setup /// methods). @@ -568,6 +570,7 @@ impl ErrorKind { ErrorKind::ImplicitBool => Severity::Ignore, ErrorKind::ImplicitImport => Severity::Warn, ErrorKind::ImplicitReexport => Severity::Ignore, + ErrorKind::ImplicitRelativeImport => Severity::Warn, ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore, ErrorKind::IncompatibleComparison => Severity::Ignore, ErrorKind::InvalidAbstractMethod => Severity::Ignore, diff --git a/crates/pyrefly_config/src/migration/pyright.rs b/crates/pyrefly_config/src/migration/pyright.rs index fbed302f1b..6cc800adbe 100644 --- a/crates/pyrefly_config/src/migration/pyright.rs +++ b/crates/pyrefly_config/src/migration/pyright.rs @@ -560,7 +560,7 @@ impl RuleOverrides { // Import rules add( self.report_implicit_relative_import, - ErrorKind::MissingImport, + ErrorKind::ImplicitRelativeImport, ); // Type argument rules @@ -801,6 +801,20 @@ include = ["basedpyright.py"] ); } + #[test] + fn test_implicit_relative_import_maps_to_dedicated_error_kind() -> anyhow::Result<()> { + let raw_file = r#"{"reportImplicitRelativeImport": "none"}"#; + let config = serde_json::from_str::(raw_file)?.convert(); + let errors = config.root.errors.as_ref().unwrap(); + + assert_eq!( + errors.severity(ErrorKind::ImplicitRelativeImport), + Severity::Ignore + ); + assert_eq!(errors.severity(ErrorKind::MissingImport), Severity::Error); + Ok(()) + } + #[test] fn test_report_trailing_commas() -> anyhow::Result<()> { let raw_file = r#" diff --git a/pyrefly/lib/module/finder.rs b/pyrefly/lib/module/finder.rs index 3b513a39f1..85a5c341e3 100644 --- a/pyrefly/lib/module/finder.rs +++ b/pyrefly/lib/module/finder.rs @@ -31,6 +31,7 @@ static STDLIB_SUGGESTION_CACHE: LazyLock Option { #[cfg(test)] mod tests { use std::path::Path; + use std::sync::Arc; use pyrefly_config::config::ConfigSource; + use pyrefly_config::config::DirectoryRelativeFallbackSearchPathCache; use pyrefly_config::environment::environment::PythonEnvironment; use pyrefly_config::environment::interpreters::Interpreters; use pyrefly_python::module_path::ModulePathDetails; @@ -2777,6 +2787,103 @@ mod tests { config } + #[test] + fn test_directory_relative_fallback_reports_implicit_import() { + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path(); + TestPath::setup_test_directory( + root, + vec![TestPath::dir( + "pkg", + vec![TestPath::file("main.py"), TestPath::file("helper.py")], + )], + ); + let mut config = get_config(ConfigSource::File(root.join("pyrefly.toml"))); + config.fallback_search_path = FallbackSearchPath::DirectoryRelative( + DirectoryRelativeFallbackSearchPathCache::new(Some(root.to_path_buf())), + ); + let module = ModuleName::from_str("helper"); + let origin = ModulePath::filesystem(root.join("pkg/main.py")); + + let result = find_import_filtered( + &config, + module, + Some(&origin), + None, + &DirEntryCache::new(), + None, + ); + + assert_eq!( + result, + FindingOrError::Finding(Finding { + finding: ModulePath::filesystem(root.join("pkg/helper.py")), + error: Some(FindError::ImplicitRelativeImport(module)), + }) + ); + } + + #[test] + fn test_explicit_fallback_does_not_report_implicit_import() { + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path(); + TestPath::setup_test_directory(root, vec![TestPath::file("helper.py")]); + let mut config = get_config(ConfigSource::File(root.join("pyrefly.toml"))); + config.fallback_search_path = + FallbackSearchPath::Explicit(Arc::new(vec![root.to_path_buf()])); + let origin = ModulePath::filesystem(root.join("pkg/main.py")); + + let result = find_import_filtered( + &config, + ModuleName::from_str("helper"), + Some(&origin), + None, + &DirEntryCache::new(), + None, + ); + + assert_eq!( + result, + FindingOrError::new_finding(ModulePath::filesystem(root.join("helper.py"))) + ); + } + + #[test] + fn test_absolute_search_path_precedes_directory_relative_fallback() { + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path(); + TestPath::setup_test_directory( + root, + vec![ + TestPath::dir("src", vec![TestPath::file("helper.py")]), + TestPath::dir( + "pkg", + vec![TestPath::file("main.py"), TestPath::file("helper.py")], + ), + ], + ); + let mut config = get_config(ConfigSource::File(root.join("pyrefly.toml"))); + config.search_path_from_file = vec![root.join("src")]; + config.fallback_search_path = FallbackSearchPath::DirectoryRelative( + DirectoryRelativeFallbackSearchPathCache::new(Some(root.to_path_buf())), + ); + let origin = ModulePath::filesystem(root.join("pkg/main.py")); + + let result = find_import_filtered( + &config, + ModuleName::from_str("helper"), + Some(&origin), + None, + &DirEntryCache::new(), + None, + ); + + assert_eq!( + result, + FindingOrError::new_finding(ModulePath::filesystem(root.join("src/helper.py"))) + ); + } + /// A first-party root plus a site package directory holding representative /// package layouts for testing untyped import handling. fn untyped_imports_config(root: &Path, replace_untyped: &[&str]) -> ConfigFile { diff --git a/pyrefly/lib/state/loader.rs b/pyrefly/lib/state/loader.rs index 740a7950cd..dd59365f39 100644 --- a/pyrefly/lib/state/loader.rs +++ b/pyrefly/lib/state/loader.rs @@ -32,6 +32,8 @@ use crate::state::state::TransactionTimingCounters; #[derive(Debug, Clone, Dupe, PartialEq, Eq)] pub enum FindError { + /// This module resolved only through a directory-relative fallback search. + ImplicitRelativeImport(ModuleName), /// This module could not be found, and we should emit an error MissingImport(ModuleName, Arc>), /// This import could not be found, but the user configured it to be ignored @@ -94,6 +96,16 @@ impl FindError { pub fn display(&self) -> (Option ErrorContext + '_>>, Vec1) { match self { + Self::ImplicitRelativeImport(module) => ( + None, + vec1![ + format!( + "Import `{module}` is implicitly relative and may fail when this file is imported as part of a package" + ), + "Use an explicit relative import or the full absolute package path instead" + .to_owned(), + ], + ), Self::MissingImport(module, err) => { let mut lines = (**err).clone(); // Compute suggestion lazily at display time, using global cache @@ -128,6 +140,7 @@ impl FindError { pub fn kind(&self) -> Option { match self { + Self::ImplicitRelativeImport(..) => Some(ErrorKind::ImplicitRelativeImport), Self::MissingImport(..) => Some(ErrorKind::MissingImport), Self::MissingSource(..) => Some(ErrorKind::MissingSource), Self::MissingSourceForStubs(..) => Some(ErrorKind::MissingSourceForStubs), diff --git a/pyrefly/lib/test/state.rs b/pyrefly/lib/test/state.rs index 71e2321fb3..86cfc11a35 100644 --- a/pyrefly/lib/test/state.rs +++ b/pyrefly/lib/test/state.rs @@ -42,6 +42,8 @@ use tempfile::TempDir; use crate::commands::config_finder::default_config_finder; use crate::config::config::ConfigFile; use crate::config::config::ConfigSource; +use crate::config::error_kind::ErrorKind; +use crate::config::error_kind::Severity; use crate::config::finder::ConfigFinder; use crate::error::error::print_errors; use crate::lsp::non_wasm::server::resolve_export_location; @@ -53,6 +55,42 @@ use crate::state::require::RequireLevels; use crate::state::state::State; use crate::test::util::TestEnv; +#[test] +fn test_directory_relative_import_emits_dedicated_diagnostic() { + let tdir = TempDir::new().unwrap(); + let root = tdir.path(); + let package = root.join("pkg"); + fs::create_dir(&package).unwrap(); + fs::write(root.join(ConfigFile::PYREFLY_FILE_NAME), "").unwrap(); + fs::write(package.join("helper.py"), "answer: int = 42").unwrap(); + let main_path = package.join("main.py"); + fs::write(&main_path, "import helper\nvalue: int = helper.answer\n").unwrap(); + + let mut config = ConfigFile { + source: ConfigSource::File(root.join(ConfigFile::PYREFLY_FILE_NAME)), + enable_fallback_search_path: true, + ..Default::default() + }; + config.python_environment.set_empty_to_default(); + config.interpreters.skip_interpreter_query = true; + config.configure(); + let config = ArcId::new(config); + let handle = Handle::new( + ModuleName::from_str("pkg.main"), + ModulePath::filesystem(main_path), + config.get_sys_info(), + ); + let state = State::new(ConfigFinder::new_constant(config), TEST_THREAD_COUNT); + let mut transaction = state.new_transaction(Require::Errors, None); + + transaction.run(&[handle.dupe()], Require::Errors, None); + let errors = transaction.get_errors([&handle]).collect_display_errors(); + + assert_eq!(errors.len(), 1, "expected one diagnostic, got {errors:?}"); + assert_eq!(errors[0].error_kind(), ErrorKind::ImplicitRelativeImport); + assert!(errors[0].msg().contains("implicitly relative")); +} + #[derive(Debug)] struct MutableShapeExtensionsSourceDb { sys_info: SysInfo, @@ -447,6 +485,13 @@ def f(x: Float[Tensor, "batch channels"]) -> None: enable_fallback_search_path: true, ..Default::default() }; + // This test intentionally models third-party imports with the directory-relative + // fallback. Keep its assertions focused on origin-sensitive tensor shape state. + config + .root + .errors + .get_or_insert_default() + .set_error_severity(ErrorKind::ImplicitRelativeImport, Severity::Ignore); config.root.jaxtyping = Some(true); config.python_environment.set_empty_to_default(); config.interpreters.skip_interpreter_query = true; diff --git a/scripts/error_presets.json b/scripts/error_presets.json index 7a8d0d25d4..b8c0b41e1b 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -50,6 +50,7 @@ "implicit-bool": ["all"], "implicit-import": ["legacy", "default", "strict", "all"], "implicit-reexport": ["all"], + "implicit-relative-import": ["legacy", "default", "strict", "all"], "implicitly-defined-attribute": ["all"], "incompatible-comparison": ["all"], "incompatible-overload-residual": ["legacy", "default", "strict", "all"], diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 91cc775ae1..102ee91d81 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -801,6 +801,25 @@ from bar import a # error: `a` is not exported from module `bar` from foo import a as a # or add `a` to `__all__` ``` +## implicit-relative-import + +Default severity: `warn` + +This error is emitted when an unqualified import resolves only by searching +relative to the importing file. Such an import can work when a file is run as a +script but fail when the same file is imported as part of a package. + +Use an explicit relative import or the module's full absolute package path: + +```python +# pkg/main.py +import helper # implicit-relative-import + +# Fix: +from . import helper +# or: import pkg.helper +``` + ## implicitly-defined-attribute Default severity: `ignore` diff --git a/website/docs/migrate/pyright/diagnostics-reference.mdx b/website/docs/migrate/pyright/diagnostics-reference.mdx index 19e4700a4c..f309bc2be3 100644 --- a/website/docs/migrate/pyright/diagnostics-reference.mdx +++ b/website/docs/migrate/pyright/diagnostics-reference.mdx @@ -162,7 +162,7 @@ of them: | BasedPyright rule | Pyrefly | | --------------------------------------- | -------------------------------- | | `reportExplicitAny` | `explicit-any` | -| `reportImplicitRelativeImport` | `missing-import` | +| `reportImplicitRelativeImport` | `implicit-relative-import` | | `reportIncompatibleUnannotatedOverride` | `bad-override-mutable-attribute` | These are a separate compatibility layer; an upstream Pyright config will not