diff --git a/crates/pyrefly_config/src/config.rs b/crates/pyrefly_config/src/config.rs index dc192f99d4..9d607653ec 100644 --- a/crates/pyrefly_config/src/config.rs +++ b/crates/pyrefly_config/src/config.rs @@ -882,6 +882,18 @@ pub struct SourceDbQueryOutcome { pub error: Option, } +/// The oldest Python version for which the bundled typeshed stubs are accurate. +/// +/// Typeshed publishes this as `[tool.typeshed] oldest-supported-python` in its +/// `pyproject.toml`. We mirror it here because Pyrefly only bundles typeshed's `stdlib/` +/// and `stubs/` trees, not its `pyproject.toml`. Update this whenever the vendored +/// typeshed is updated and drops support for an older version. +const TYPESHED_OLDEST_SUPPORTED_PYTHON: PythonVersion = PythonVersion { + major: 3, + minor: 10, + micro: 0, +}; + impl ConfigFile { /// Gets a ConfigFile for a project directory. `fallback` indicates whether this is a guessed /// project root that we're falling back to after failing to otherwise find an import. @@ -2021,6 +2033,21 @@ impl ConfigFile { } configure_errors.extend(validate(&self.search_path_from_file, "search-path")); + // Typeshed removes the version-specific branches and guards for Python versions it no + // longer supports, so the bundled stubs only describe the standard library accurately for + // versions >= the oldest supported one. A custom `typeshed-path` replaces the bundled + // stdlib and may describe older versions, so only warn for the bundled typeshed. + // Warn instead of failing, because the user's own code may still legitimately target an + // older version. + if self.typeshed_path.is_none() { + let python_version = self.python_version(); + if python_version < TYPESHED_OLDEST_SUPPORTED_PYTHON { + configure_errors.push(anyhow!( + "Python version {python_version} is older than the oldest version described by the bundled typeshed stubs ({TYPESHED_OLDEST_SUPPORTED_PYTHON}). The bundled stubs no longer include the version-specific branches and guards for older versions, so the standard library may be modeled inaccurately for this target. If the project must keep targeting {python_version}, provide compatible custom stubs with `typeshed-path`; otherwise raise `python-version` to {TYPESHED_OLDEST_SUPPORTED_PYTHON} or newer." + )); + } + } + if let ConfigSource::File(path) = &self.source { configure_errors .into_map(|e| ConfigError::warn(e.context(format!("{}", path.display())))) @@ -2744,6 +2771,74 @@ mod tests { assert!(!settings.contains_key("use-ignore-files")); } + #[test] + fn test_configure_warns_when_python_version_is_older_than_typeshed() { + let mut config = ConfigFile::default(); + config.interpreters.skip_interpreter_query = true; + config.python_environment.python_version = Some(PythonVersion::new(3, 9, 0)); + + let messages: Vec = config + .configure() + .iter() + .filter(|error| error.severity() == Severity::Warn) + .map(|error| error.get_message()) + .collect(); + + assert!( + messages.iter().any(|message| message + .contains("older than the oldest version described by the bundled typeshed stubs")), + "expected a warning about an outdated Python version, got: {messages:?}" + ); + } + + #[test] + fn test_configure_does_not_warn_when_custom_typeshed_is_configured() { + let mut config = ConfigFile::default(); + config.interpreters.skip_interpreter_query = true; + config.python_environment.python_version = Some(PythonVersion::new(3, 9, 0)); + // A custom typeshed replaces the bundled stdlib and may describe older versions. + config.typeshed_path = Some(PathBuf::from("/custom/typeshed")); + + let messages: Vec = config + .configure() + .iter() + .map(|error| error.get_message()) + .collect(); + + assert!( + !messages + .iter() + .any(|message| message.contains("bundled typeshed stubs")), + "unexpected bundled-typeshed warning when a custom typeshed is configured: {messages:?}" + ); + } + + #[test] + fn test_configure_does_not_warn_when_python_version_is_typeshed_supported() { + for version in [ + PythonVersion::new(3, 10, 0), + PythonVersion::new(3, 11, 0), + PythonVersion::new(3, 13, 0), + ] { + let mut config = ConfigFile::default(); + config.interpreters.skip_interpreter_query = true; + config.python_environment.python_version = Some(version); + + let messages: Vec = config + .configure() + .iter() + .map(|error| error.get_message()) + .collect(); + + assert!( + !messages.iter().any(|message| message.contains( + "older than the oldest version described by the bundled typeshed stubs" + )), + "unexpected outdated-Python warning for {version}: {messages:?}" + ); + } + } + #[test] fn deserialize_pyrefly_config() { let config_str = r#" diff --git a/crates/pyrefly_config/src/finder.rs b/crates/pyrefly_config/src/finder.rs index b272c5f6ba..6b8df67603 100644 --- a/crates/pyrefly_config/src/finder.rs +++ b/crates/pyrefly_config/src/finder.rs @@ -118,8 +118,14 @@ type BeforeCallback = type LoadCallback = Box (ArcId, Vec) + Send + Sync>; /// Fallback function when no config file exists or loading fails. -type FallbackCallback = - Box ArcId + Send + Sync>; +/// +/// Returns the config along with any user-facing warnings produced while configuring it (for +/// example, a warning that the resolved `python-version` predates the bundled typeshed). Those +/// warnings are accumulated so callers surface them like config-file errors; a synthesized config +/// must not silently swallow them. +type FallbackCallback = Box< + dyn Fn(ModuleNameWithKind, &ModulePath) -> (ArcId, Vec) + Send + Sync, +>; /// Function that replaces the config found for a Python file with one for that file alone, such /// as a PEP 723 script that runs in its own environment. @@ -174,7 +180,7 @@ impl ConfigFinder { Self::new_custom( Box::new(move |_, _| Ok(Some(c1.dupe()))), Box::new(move |_| (c2.dupe(), Vec::new())), - Box::new(move |_, _| c3.dupe()), + Box::new(move |_, _| (c3.dupe(), Vec::new())), file, Box::new(|| {}), ) @@ -298,15 +304,17 @@ impl ConfigFinder { } } - let f = |dir: Option<&Path>| match dir { - Some(parent) => self - .search - .directory_absolute(parent) - .unwrap_or_else(|| (self.fallback)(name, path)), - None => (self.fallback)(name, path), + let f = |dir: Option<&Path>| -> (ArcId, Vec) { + match dir { + Some(parent) => match self.search.directory_absolute(parent) { + Some(config) => (config, Vec::new()), + None => (self.fallback)(name, path), + }, + None => (self.fallback)(name, path), + } }; - let config = match path.details() { + let (config, errors) = match path.details() { ModulePathDetails::FileSystem(x) | ModulePathDetails::Memory(x) => { let absolute = x.absolutize(); f(absolute.parent()) @@ -316,6 +324,19 @@ impl ConfigFinder { ModulePathDetails::BundledTypeshedThirdParty(_) => f(None), ModulePathDetails::BundledThirdParty(_) => f(None), }; + // One run can synthesize configs for several module paths, and the same user-facing + // warning may be produced for more than one of them; report it once. + { + let mut accumulated = self.errors.lock(); + for error in errors { + if !accumulated + .iter() + .any(|existing| existing.get_message() == error.get_message()) + { + accumulated.push(error); + } + } + } (self.file)(config, path) } diff --git a/pyrefly/lib/commands/config_finder.rs b/pyrefly/lib/commands/config_finder.rs index 98dff5938e..3cd1cbb08a 100644 --- a/pyrefly/lib/commands/config_finder.rs +++ b/pyrefly/lib/commands/config_finder.rs @@ -30,10 +30,10 @@ use starlark_map::small_map::SmallMap; use tracing::warn; use crate::config::config::ConfigFile; +use crate::config::finder::debug_log; use crate::config::config::ProjectLayout; use crate::config::finder::ConfigError; use crate::config::finder::ConfigFinder; -use crate::config::finder::debug_log; use crate::module::bundled::BundledStub; use crate::module::third_party::BundledThirdParty; use crate::module::typeshed::BundledTypeshedStdlib; @@ -401,16 +401,14 @@ pub fn standard_config_finder( }) }; - let empty = move || { - cache_empty - .lock() - .get_or_insert_with(|| { - let (config, errors) = configure3.configure(None, ConfigFile::default(), vec![]); - // Since this is a config we generated, these are likely internal errors. - debug_log(errors); - config - }) - .dupe() + let empty = move || -> (ArcId, Vec) { + let mut cache = cache_empty.lock(); + if let Some(config) = cache.as_ref() { + return (config.dupe(), Vec::new()); + } + let (config, errors) = configure3.configure(None, ConfigFile::default(), vec![]); + *cache = Some(config.dupe()); + (config, errors) }; ConfigFinder::new_custom( @@ -428,72 +426,69 @@ pub fn standard_config_finder( }), // Fall back to using a default config, but let's see if we can make the `search_path` somewhat useful // based on a few heuristics. - Box::new(move |module_kind, path| { - let name = module_kind.name(); - let is_fallback = module_kind.is_fallback(); - match path.root_of(name) { - // We were able to walk up `path` and match each component of `name` to a directory until we ran out. - // That means the resulting path is likely the root of the 'project', and should therefore be its `search_path`. - Some(path) if !is_fallback => cache_one - .lock() - .entry(path.clone()) - .or_insert_with(|| { + Box::new( + move |module_kind, path| -> (ArcId, Vec) { + let name = module_kind.name(); + let is_fallback = module_kind.is_fallback(); + match path.root_of(name) { + // We were able to walk up `path` and match each component of `name` to a directory until we ran out. + // That means the resulting path is likely the root of the 'project', and should therefore be its `search_path`. + Some(path) if !is_fallback => { + let mut cache = cache_one.lock(); + if let Some(config) = cache.get(&path) { + return (config.dupe(), Vec::new()); + } let (config, errors) = configure2.configure( Some(&path), ConfigFile::init_at_root(&path, &ProjectLayout::Flat, true), vec![], ); - // Since this is a config we generated, these are likely internal errors. - debug_log(errors); - config - }) - .dupe(), - - // We couldn't walk up and find a possible root of the project, so let's try to create a search - // path that is still useful for this import by including all of its parents. - _ => { - let parent = match path.details() { - ModulePathDetails::FileSystem(x) | ModulePathDetails::Memory(x) => { - if let Some(path) = x.parent() { - path - } else { - return empty(); + cache.insert(path, config.dupe()); + (config, errors) + } + + // We couldn't walk up and find a possible root of the project, so let's try to create a search + // path that is still useful for this import by including all of its parents. + _ => { + let parent = match path.details() { + ModulePathDetails::FileSystem(x) | ModulePathDetails::Memory(x) => { + if let Some(path) = x.parent() { + path + } else { + return empty(); + } } + ModulePathDetails::Namespace(x) => x.as_path(), + ModulePathDetails::BundledTypeshed(_) => { + return (BundledTypeshedStdlib::config(), Vec::new()); + } + ModulePathDetails::BundledTypeshedThirdParty(_) => { + return (BundledTypeshedThirdParty::config(), Vec::new()); + } + ModulePathDetails::BundledThirdParty(_) => { + return (BundledThirdParty::config(), Vec::new()); + } + }; + let mut cache = cache_parents.lock(); + if let Some(config) = cache.get(parent) { + return (config.dupe(), Vec::new()); } - ModulePathDetails::Namespace(x) => x.as_path(), - ModulePathDetails::BundledTypeshed(_) => { - return BundledTypeshedStdlib::config(); - } - ModulePathDetails::BundledTypeshedThirdParty(_) => { - return BundledTypeshedThirdParty::config(); - } - ModulePathDetails::BundledThirdParty(_) => { - return BundledThirdParty::config(); - } - }; - cache_parents - .lock() - .entry(parent.to_owned()) - .or_insert_with(|| { - let fallback_search_path = - FallbackSearchPath::Explicit(cache_ancestors.get_ancestors(parent)); - let mut config = ConfigFile { - source: ConfigSource::Synthetic(Some(parent.to_owned())), - project_includes: ConfigFile::default_project_includes(), - // We use `fallback_search_path` because otherwise a user with `/sys` on their - // computer (all of them) will override `sys.version` in preference to typeshed. - fallback_search_path, - root: ConfigBase::default_for_ide_without_config(), - ..Default::default() - }; - config.rewrite_with_path_to_config(parent); - let (config, errors) = - configure2.configure(Some(parent), config, vec![]); - // Since this is a config we generated, these are likely internal errors. - debug_log(errors); - config - }) - .dupe() + let fallback_search_path = + FallbackSearchPath::Explicit(cache_ancestors.get_ancestors(parent)); + let mut config = ConfigFile { + source: ConfigSource::Synthetic(Some(parent.to_owned())), + project_includes: ConfigFile::default_project_includes(), + // We use `fallback_search_path` because otherwise a user with `/sys` on their + // computer (all of them) will override `sys.version` in preference to typeshed. + fallback_search_path, + root: ConfigBase::default_for_ide_without_config(), + ..Default::default() + }; + config.rewrite_with_path_to_config(parent); + let (config, errors) = configure2.configure(Some(parent), config, vec![]); + cache.insert(parent.to_owned(), config.dupe()); + (config, errors) + } } } }), @@ -515,6 +510,7 @@ mod tests { use pyrefly_python::module_name::ModuleNameWithKind; use pyrefly_python::module_path::ModulePath; use pyrefly_python::module_path::ModulePathDetails; + use pyrefly_python::sys_info::PythonVersion; use pyrefly_util::includes::Includes; use pyrefly_util::test_path::TestPath; @@ -1112,6 +1108,46 @@ mod tests { ); } + /// A synthesized config still has to deliver config warnings to the user: the config + /// finder's fallback used to debug-log them, so `--python-version 3.9` on an unconfigured + /// project warned about nothing. + #[test] + fn test_unconfigured_project_reports_outdated_python_version_warning() { + use clap::Parser; + + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path(); + TestPath::setup_test_directory( + root, + vec![TestPath::dir("pkg", vec![TestPath::file("mod.py")])], + ); + + let args = ConfigOverrideArgs::try_parse_from([ + "pyrefly", + "--python-version", + "3.9", + "--skip-interpreter-query", + ]) + .unwrap(); + let finder = default_config_finder_with_overrides(args, false, None); + let config = finder.python_file( + ModuleNameWithKind::guaranteed(ModuleName::from_str("pkg.mod")), + &ModulePath::filesystem(root.join("pkg/mod.py")), + ); + + assert_eq!(config.python_version(), PythonVersion::new(3, 9, 0)); + let messages: Vec = finder + .errors() + .iter() + .map(|error| error.get_message()) + .collect(); + assert!( + messages.iter().any(|message| message + .contains("older than the oldest version described by the bundled typeshed stubs")), + "expected the outdated Python version warning from the synthesized config, got: {messages:?}" + ); + } + #[test] fn test_unconfigured_project_under_hidden_dir_is_not_excluded() { let tempdir = tempfile::tempdir().unwrap(); diff --git a/pyrefly/lib/commands/files.rs b/pyrefly/lib/commands/files.rs index 8133d22ed8..3be6c3c3af 100644 --- a/pyrefly/lib/commands/files.rs +++ b/pyrefly/lib/commands/files.rs @@ -40,7 +40,6 @@ use crate::config::config::SynthesizedPresetReason; use crate::config::error_kind::Severity; use crate::config::finder::ConfigError; use crate::config::finder::ConfigFinder; -use crate::config::finder::debug_log; /// Whether (and how) to emit the "no `pyrefly.toml` found" upsell after /// a CLI check. Computed once during file resolution so the hot path @@ -140,28 +139,32 @@ pub fn get_project_config_for_current_dir( ) -> anyhow::Result<(ArcId, Vec)> { let current_dir = std::env::current_dir().context("cannot identify current dir")?; let config_finder = default_config_finder_with_overrides(args.clone(), false, wrapper); - let config = config_finder.directory(¤t_dir).unwrap_or_else(|| { - // No marker found upward. Run the synthesized config through the - // same unconfigured resolver wiring that the file-mode path uses - // (via `DefaultConfigConfigurerWithOverrides`); without this, - // `pyrefly check` (project mode) and `pyrefly check ` (file - // mode) produce divergent configs in unconfigured repos — the - // file-mode upsell would never fire for project-mode checks, and - // a nearby mypy.ini / pyrightconfig.json would be migrated for - // file mode but ignored here. - let mut synthesized = - ConfigFile::init_at_root(¤t_dir, &ProjectLayout::new(¤t_dir), false); - apply_unconfigured_resolver_if_applicable( - &mut synthesized, - Some(¤t_dir), - UnconfiguredOverride::Auto, - ); - let (config, errors) = args.override_config_at(synthesized, Some(¤t_dir)); - // Since this is a config we generated, these are likely internal errors. - debug_log(errors); - config - }); - Ok((config, config_finder.errors())) + let (config, mut errors) = match config_finder.directory(¤t_dir) { + Some(config) => (config, Vec::new()), + None => { + // No marker found upward. Run the synthesized config through the + // same unconfigured resolver wiring that the file-mode path uses + // (via `DefaultConfigConfigurerWithOverrides`); without this, + // `pyrefly check` (project mode) and `pyrefly check ` (file + // mode) produce divergent configs in unconfigured repos — the + // file-mode upsell would never fire for project-mode checks, and + // a nearby mypy.ini / pyrightconfig.json would be migrated for + // file mode but ignored here. + let mut synthesized = + ConfigFile::init_at_root(¤t_dir, &ProjectLayout::new(¤t_dir), false); + apply_unconfigured_resolver_if_applicable( + &mut synthesized, + Some(¤t_dir), + UnconfiguredOverride::Auto, + ); + // Keep any warnings produced while configuring the synthesized config: they are + // user-facing (for example, an unsupported `python-version`), unlike the parse + // errors that only a real config file can produce. + args.override_config_at(synthesized, Some(¤t_dir)) + } + }; + errors.append(&mut config_finder.errors()); + Ok((config, errors)) } pub fn get_config_finder_for_snippet( diff --git a/pyrefly/lib/test/incremental.rs b/pyrefly/lib/test/incremental.rs index 5eed83bdb3..ecf442b0e9 100644 --- a/pyrefly/lib/test/incremental.rs +++ b/pyrefly/lib/test/incremental.rs @@ -3969,7 +3969,7 @@ g("x") # E: `str` is not assignable to upper bound `int` of type variable `T` })) }), Box::new(move |_| (main_load.dupe(), Vec::new())), - Box::new(move |_, _| main_fallback.dupe()), + Box::new(move |_, _| (main_fallback.dupe(), Vec::new())), Box::new(|config, _| config), Box::new(|| {}), );