Skip to content

Commit

Permalink
Fallback to requires python if no python-version is specified
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Feb 8, 2025
1 parent dd75dc2 commit a2f8e8e
Show file tree
Hide file tree
Showing 12 changed files with 357 additions and 27 deletions.
4 changes: 4 additions & 0 deletions crates/red_knot/tests/file_watching.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,10 @@ fn add_search_path() -> anyhow::Result<()> {

#[test]
fn remove_search_path() -> anyhow::Result<()> {
assert_eq!(
std::env::var("XDG_CONFIG_HOME"),
Err(std::env::VarError::NotPresent)
);
let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| {
Some(Options {
environment: Some(EnvironmentOptions {
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_project/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
225 changes: 204 additions & 21 deletions crates/red_knot_project/src/metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, TooLargeRequiresPythonError};
use crate::metadata::value::ValueSource;
use options::KnotTomlError;
use options::Options;
Expand Down Expand Up @@ -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, TooLargeRequiresPythonError> {
Self::from_options(
pyproject
.tool
Expand All @@ -62,22 +65,36 @@ 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<Self, TooLargeRequiresPythonError> {
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 project doesn't specify a python version but the `project.requires-python` field is set, resolve the version
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.
Expand Down Expand Up @@ -145,19 +162,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);
Expand Down Expand Up @@ -262,6 +294,12 @@ pub enum ProjectDiscoveryError {
source: Box<KnotTomlError>,
path: SystemPathBuf,
},

#[error("the `requires-python` constraint in `{path}` is invalid: {source}")]
InvalidRequiresPythonConstraint {
source: TooLargeRequiresPythonError,
path: SystemPathBuf,
},
}

#[cfg(test)]
Expand Down Expand Up @@ -527,20 +565,20 @@ 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")?;
Expand All @@ -551,6 +589,151 @@ expected `.`, `]`

Ok(())
}
#[test]
fn requires_python_no_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"
"#,
)
.context("Failed to write file")?;

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);

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)?;

snapshot_project!(root);

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)?;

snapshot_project!(root);

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)?;

snapshot_project!(root);

Ok(())
}

#[test]
fn requires_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)?;

snapshot_project!(root);

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)?;

snapshot_project!(root);

Ok(())
}

#[track_caller]
fn assert_error_eq(error: &ProjectDiscoveryError, message: &str) {
Expand Down
Loading

0 comments on commit a2f8e8e

Please sign in to comment.