diff --git a/crates/red_knot_project/Cargo.toml b/crates/red_knot_project/Cargo.toml index ea31f99f79a36..fc2752a1e6817 100644 --- a/crates/red_knot_project/Cargo.toml +++ b/crates/red_knot_project/Cargo.toml @@ -24,7 +24,7 @@ anyhow = { workspace = true } crossbeam = { workspace = true } glob = { workspace = true } notify = { workspace = true } -pep440_rs = { workspace = true } +pep440_rs = { workspace = true, features = ["version-ranges"] } rayon = { workspace = true } rustc-hash = { workspace = true } salsa = { workspace = true } diff --git a/crates/red_knot_project/src/metadata.rs b/crates/red_knot_project/src/metadata.rs index 38217e89791f5..9002ed7fcff1f 100644 --- a/crates/red_knot_project/src/metadata.rs +++ b/crates/red_knot_project/src/metadata.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use thiserror::Error; use crate::combine::Combine; -use crate::metadata::pyproject::{Project, PyProject, PyProjectError}; +use crate::metadata::pyproject::{Project, PyProject, PyProjectError, ResolveRequiresPythonError}; use crate::metadata::value::ValueSource; use options::KnotTomlError; use options::Options; @@ -49,7 +49,10 @@ impl ProjectMetadata { } /// Loads a project from a `pyproject.toml` file. - pub(crate) fn from_pyproject(pyproject: PyProject, root: SystemPathBuf) -> Self { + pub(crate) fn from_pyproject( + pyproject: PyProject, + root: SystemPathBuf, + ) -> Result { Self::from_options( pyproject .tool @@ -62,22 +65,37 @@ impl ProjectMetadata { /// Loads a project from a set of options with an optional pyproject-project table. pub(crate) fn from_options( - options: Options, + mut options: Options, root: SystemPathBuf, project: Option<&Project>, - ) -> Self { + ) -> Result { let name = project - .and_then(|project| project.name.as_ref()) - .map(|name| Name::new(&***name)) + .and_then(|project| project.name.as_deref()) + .map(|name| Name::new(&**name)) .unwrap_or_else(|| Name::new(root.file_name().unwrap_or("root"))); - // TODO(https://github.com/astral-sh/ruff/issues/15491): Respect requires-python - Self { + // If the `options` don't specify a python version but the `project.requires-python` field is set, + // use that as a lower bound instead. + if let Some(project) = project { + if !options + .environment + .as_ref() + .is_some_and(|env| env.python_version.is_some()) + { + if let Some(requires_python) = project.resolve_requires_python_lower_bound()? { + let mut environment = options.environment.unwrap_or_default(); + environment.python_version = Some(requires_python); + options.environment = Some(environment); + } + } + } + + Ok(Self { name, root, options, extra_configuration_paths: Vec::new(), - } + }) } /// Discovers the closest project at `path` and returns its metadata. @@ -145,19 +163,34 @@ impl ProjectMetadata { } tracing::debug!("Found project at '{}'", project_root); - return Ok(ProjectMetadata::from_options( + + let metadata = ProjectMetadata::from_options( options, project_root.to_path_buf(), pyproject .as_ref() .and_then(|pyproject| pyproject.project.as_ref()), - )); + ) + .map_err(|err| { + ProjectDiscoveryError::InvalidRequiresPythonConstraint { + source: err, + path: pyproject_path, + } + })?; + + return Ok(metadata); } if let Some(pyproject) = pyproject { let has_knot_section = pyproject.knot().is_some(); let metadata = - ProjectMetadata::from_pyproject(pyproject, project_root.to_path_buf()); + ProjectMetadata::from_pyproject(pyproject, project_root.to_path_buf()) + .map_err( + |err| ProjectDiscoveryError::InvalidRequiresPythonConstraint { + source: err, + path: pyproject_path, + }, + )?; if has_knot_section { tracing::debug!("Found project at '{}'", project_root); @@ -262,15 +295,21 @@ pub enum ProjectDiscoveryError { source: Box, path: SystemPathBuf, }, + + #[error("Invalid `requires-python` version specifier (`{path}`): {source}")] + InvalidRequiresPythonConstraint { + source: ResolveRequiresPythonError, + path: SystemPathBuf, + }, } #[cfg(test)] mod tests { //! Integration tests for project discovery - use crate::snapshot_project; use anyhow::{anyhow, Context}; use insta::assert_ron_snapshot; + use red_knot_python_semantic::PythonVersion; use ruff_db::system::{SystemPathBuf, TestSystem}; use crate::{ProjectDiscoveryError, ProjectMetadata}; @@ -290,7 +329,15 @@ mod tests { assert_eq!(project.root(), &*root); - snapshot_project!(project); + with_escaped_paths(|| { + assert_ron_snapshot!(&project, @r#" + ProjectMetadata( + name: Name("app"), + root: "/app", + options: Options(), + ) + "#); + }); Ok(()) } @@ -319,7 +366,16 @@ mod tests { ProjectMetadata::discover(&root, &system).context("Failed to discover project")?; assert_eq!(project.root(), &*root); - snapshot_project!(project); + + with_escaped_paths(|| { + assert_ron_snapshot!(&project, @r#" + ProjectMetadata( + name: Name("backend"), + root: "/app", + options: Options(), + ) + "#); + }); // Discovering the same package from a subdirectory should give the same result let from_src = ProjectMetadata::discover(&root.join("db"), &system) @@ -402,7 +458,19 @@ expected `.`, `]` let sub_project = ProjectMetadata::discover(&root.join("packages/a"), &system)?; - snapshot_project!(sub_project); + with_escaped_paths(|| { + assert_ron_snapshot!(sub_project, @r#" + ProjectMetadata( + name: Name("nested-project"), + root: "/app/packages/a", + options: Options( + src: Some(SrcOptions( + root: Some("src"), + )), + ), + ) + "#); + }); Ok(()) } @@ -440,7 +508,19 @@ expected `.`, `]` let root = ProjectMetadata::discover(&root, &system)?; - snapshot_project!(root); + with_escaped_paths(|| { + assert_ron_snapshot!(root, @r#" + ProjectMetadata( + name: Name("project-root"), + root: "/app", + options: Options( + src: Some(SrcOptions( + root: Some("src"), + )), + ), + ) + "#); + }); Ok(()) } @@ -472,7 +552,15 @@ expected `.`, `]` let sub_project = ProjectMetadata::discover(&root.join("packages/a"), &system)?; - snapshot_project!(sub_project); + with_escaped_paths(|| { + assert_ron_snapshot!(sub_project, @r#" + ProjectMetadata( + name: Name("nested-project"), + root: "/app/packages/a", + options: Options(), + ) + "#); + }); Ok(()) } @@ -507,7 +595,19 @@ expected `.`, `]` let root = ProjectMetadata::discover(&root.join("packages/a"), &system)?; - snapshot_project!(root); + with_escaped_paths(|| { + assert_ron_snapshot!(root, @r#" + ProjectMetadata( + name: Name("project-root"), + root: "/app", + options: Options( + environment: Some(EnvironmentOptions( + r#python-version: Some("3.10"), + )), + ), + ) + "#); + }); Ok(()) } @@ -527,27 +627,304 @@ expected `.`, `]` ( root.join("pyproject.toml"), r#" - [project] - name = "super-app" - requires-python = ">=3.12" + [project] + name = "super-app" + requires-python = ">=3.12" - [tool.knot.src] - root = "this_option_is_ignored" - "#, + [tool.knot.src] + root = "this_option_is_ignored" + "#, ), ( root.join("knot.toml"), r#" - [src] - root = "src" - "#, + [src] + root = "src" + "#, ), ]) .context("Failed to write files")?; let root = ProjectMetadata::discover(&root, &system)?; - snapshot_project!(root); + with_escaped_paths(|| { + assert_ron_snapshot!(root, @r#" + ProjectMetadata( + name: Name("super-app"), + root: "/app", + options: Options( + environment: Some(EnvironmentOptions( + r#python-version: Some("3.12"), + )), + src: Some(SrcOptions( + root: Some("src"), + )), + ), + ) + "#); + }); + + Ok(()) + } + #[test] + fn requires_python_major_minor() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">=3.12" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::PY312) + ); + + Ok(()) + } + + #[test] + fn requires_python_major_only() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">=3" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::from((3, 0))) + ); + + Ok(()) + } + + /// A `requires-python` constraint with major, minor and patch can be simplified + /// to major and minor (e.g. 3.12.1 -> 3.12). + #[test] + fn requires_python_major_minor_patch() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">=3.12.8" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::PY312) + ); + + Ok(()) + } + + #[test] + fn requires_python_beta_version() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">= 3.13.0b0" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::PY313) + ); + + Ok(()) + } + + #[test] + fn requires_python_greater_than_major_minor() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + # This is somewhat nonsensical because 3.12.1 > 3.12 is true. + # That's why simplifying the constraint to >= 3.12 is correct + requires-python = ">3.12" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::PY312) + ); + + Ok(()) + } + + /// `python-version` takes precedence if both `requires-python` and `python-version` are configured. + #[test] + fn requires_python_and_python_version() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">=3.12" + + [tool.knot.environment] + python-version = "3.10" + "#, + ) + .context("Failed to write file")?; + + let root = ProjectMetadata::discover(&root, &system)?; + + assert_eq!( + root.options + .environment + .unwrap_or_default() + .python_version + .as_deref(), + Some(&PythonVersion::PY310) + ); + + Ok(()) + } + + #[test] + fn requires_python_less_than() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = "<3.12" + "#, + ) + .context("Failed to write file")?; + + let Err(error) = ProjectMetadata::discover(&root, &system) else { + return Err(anyhow!("Expected project discovery to fail because the `requires-python` doesn't specify a lower bound (it only specifies an upper bound).")); + }; + + assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): value `<3.12` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`."); + + Ok(()) + } + + #[test] + fn requires_python_no_specifiers() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = "" + "#, + ) + .context("Failed to write file")?; + + let Err(error) = ProjectMetadata::discover(&root, &system) else { + return Err(anyhow!("Expected project discovery to fail because the `requires-python` specifiers are empty and don't define a lower bound.")); + }; + + assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): value `` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`."); + + Ok(()) + } + + #[test] + fn requires_python_too_large_major_version() -> anyhow::Result<()> { + let system = TestSystem::default(); + let root = SystemPathBuf::from("/app"); + + system + .memory_file_system() + .write_file( + root.join("pyproject.toml"), + r#" + [project] + requires-python = ">=999.0" + "#, + ) + .context("Failed to write file")?; + + let Err(error) = ProjectMetadata::discover(&root, &system) else { + return Err(anyhow!("Expected project discovery to fail because of the requires-python major version that is larger than 255.")); + }; + + assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): The major version `999` is larger than the maximum supported value 255"); Ok(()) } @@ -557,15 +934,12 @@ expected `.`, `]` assert_eq!(error.to_string().replace('\\', "/"), message); } - /// Snapshots a project but with all paths using unix separators. - #[macro_export] - macro_rules! snapshot_project { - ($project:expr) => {{ - assert_ron_snapshot!($project,{ - ".root" => insta::dynamic_redaction(|content, _content_path| { - content.as_str().unwrap().replace("\\", "/") - }), + fn with_escaped_paths(f: impl FnOnce() -> R) -> R { + let mut settings = insta::Settings::clone_current(); + settings.add_dynamic_redaction(".root", |content, _path| { + content.as_str().unwrap().replace('\\', "/") }); - }}; -} + + settings.bind(f) + } } diff --git a/crates/red_knot_project/src/metadata/pyproject.rs b/crates/red_knot_project/src/metadata/pyproject.rs index 4ad5f3c5b8a36..58f650ee9199d 100644 --- a/crates/red_knot_project/src/metadata/pyproject.rs +++ b/crates/red_knot_project/src/metadata/pyproject.rs @@ -1,11 +1,12 @@ -use pep440_rs::{Version, VersionSpecifiers}; +use crate::metadata::options::Options; +use crate::metadata::value::{RangedValue, ValueSource, ValueSourceGuard}; +use pep440_rs::{release_specifiers_to_ranges, Version, VersionSpecifiers}; +use red_knot_python_semantic::PythonVersion; use serde::{Deserialize, Deserializer, Serialize}; +use std::collections::Bound; use std::ops::Deref; use thiserror::Error; -use crate::metadata::options::Options; -use crate::metadata::value::{RangedValue, ValueSource, ValueSourceGuard}; - /// A `pyproject.toml` as specified in PEP 517. #[derive(Deserialize, Serialize, Debug, Default, Clone)] #[serde(rename_all = "kebab-case")] @@ -55,6 +56,73 @@ pub struct Project { pub requires_python: Option>, } +impl Project { + pub(super) fn resolve_requires_python_lower_bound( + &self, + ) -> Result>, ResolveRequiresPythonError> { + let Some(requires_python) = self.requires_python.as_ref() else { + return Ok(None); + }; + + tracing::debug!("Resolving requires-python constraint: `{requires_python}`"); + + let ranges = release_specifiers_to_ranges((**requires_python).clone()); + let Some((lower, _)) = ranges.bounding_range() else { + return Ok(None); + }; + + let version = match lower { + // Ex) `>=3.10.1` -> `>=3.10` + Bound::Included(version) => version, + + // Ex) `>3.10.1` -> `>=3.10` or `>3.10` -> `>=3.10` + // The second example looks obscure at first but it is required because + // `3.10.1 > 3.10` is true but we only have two digits here. So including 3.10 is the + // right move. Overall, using `>` without a patch release is most likely bogus. + Bound::Excluded(version) => version, + + // Ex) `<3.10` or `` + Bound::Unbounded => { + return Err(ResolveRequiresPythonError::NoLowerBound( + requires_python.to_string(), + )) + } + }; + + // Take the major and minor version + let mut versions = version.release().iter().take(2); + + let Some(major) = versions.next().copied() else { + return Ok(None); + }; + + let minor = versions.next().copied().unwrap_or_default(); + + tracing::debug!("Resolved requires-python constraint to: {major}.{minor}"); + + let major = + u8::try_from(major).map_err(|_| ResolveRequiresPythonError::TooLargeMajor(major))?; + let minor = + u8::try_from(minor).map_err(|_| ResolveRequiresPythonError::TooLargeMajor(minor))?; + + Ok(Some( + requires_python + .clone() + .map_value(|_| PythonVersion::from((major, minor))), + )) + } +} + +#[derive(Debug, Error)] +pub enum ResolveRequiresPythonError { + #[error("The major version `{0}` is larger than the maximum supported value 255")] + TooLargeMajor(u64), + #[error("The minor version `{0}` is larger than the maximum supported value 255")] + TooLargeMinor(u64), + #[error("value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`.")] + NoLowerBound(String), +} + #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub struct Tool { diff --git a/crates/red_knot_project/src/metadata/value.rs b/crates/red_knot_project/src/metadata/value.rs index 9e047580f0433..fc2d133430623 100644 --- a/crates/red_knot_project/src/metadata/value.rs +++ b/crates/red_knot_project/src/metadata/value.rs @@ -118,6 +118,15 @@ impl RangedValue { self } + #[must_use] + pub fn map_value(self, f: impl FnOnce(T) -> R) -> RangedValue { + RangedValue { + value: f(self.value), + source: self.source, + range: self.range, + } + } + pub fn into_inner(self) -> T { self.value } diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_root_project.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_root_project.snap deleted file mode 100644 index 5cc6076e7708d..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_root_project.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: root ---- -ProjectMetadata( - name: Name("project-root"), - root: "/app", - options: Options( - src: Some(SrcOptions( - root: Some("src"), - )), - ), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_sub_project.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_sub_project.snap deleted file mode 100644 index 47fc0e19468e6..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_in_sub_project.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: sub_project ---- -ProjectMetadata( - name: Name("nested-project"), - root: "/app/packages/a", - options: Options( - src: Some(SrcOptions( - root: Some("src"), - )), - ), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_with_outer_knot_section.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_with_outer_knot_section.snap deleted file mode 100644 index 9aedec703362e..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_with_outer_knot_section.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: root ---- -ProjectMetadata( - name: Name("project-root"), - root: "/app", - options: Options( - environment: Some(EnvironmentOptions( - r#python-version: Some("3.10"), - )), - ), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_without_knot_sections.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_without_knot_sections.snap deleted file mode 100644 index 48d837c066012..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__nested_projects_without_knot_sections.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: sub_project ---- -ProjectMetadata( - name: Name("nested-project"), - root: "/app/packages/a", - options: Options(), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_knot_and_pyproject_toml.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_knot_and_pyproject_toml.snap deleted file mode 100644 index 1d79e42b7a0fe..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_knot_and_pyproject_toml.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: root ---- -ProjectMetadata( - name: Name("super-app"), - root: "/app", - options: Options( - src: Some(SrcOptions( - root: Some("src"), - )), - ), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_pyproject.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_pyproject.snap deleted file mode 100644 index 4c9a1a545aea3..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_with_pyproject.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: project ---- -ProjectMetadata( - name: Name("backend"), - root: "/app", - options: Options(), -) diff --git a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_without_pyproject.snap b/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_without_pyproject.snap deleted file mode 100644 index 21d5fea654fbf..0000000000000 --- a/crates/red_knot_project/src/snapshots/red_knot_project__metadata__tests__project_without_pyproject.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: crates/red_knot_project/src/metadata.rs -expression: project ---- -ProjectMetadata( - name: Name("app"), - root: "/app", - options: Options(), -)