[red-knot] Anchor relative paths in configurations (#15634)

crates/red_knot/src/main.rs

@@ -7,6 +7,7 @@ use colored::Colorize;
 use crossbeam::channel as crossbeam_channel;
 use python_version::PythonVersion;
 use red_knot_project::metadata::options::{EnvironmentOptions, Options};
+use red_knot_project::metadata::value::RelativePathBuf;
 use red_knot_project::watch;
 use red_knot_project::watch::ProjectWatcher;
 use red_knot_project::{ProjectDatabase, ProjectMetadata};
@@ -69,22 +70,16 @@ struct Args {
 }
 
 impl Args {
-    fn to_options(&self, cli_cwd: &SystemPath) -> Options {
+    fn to_options(&self) -> Options {
         Options {
             environment: Some(EnvironmentOptions {
                 python_version: self.python_version.map(Into::into),
-                venv_path: self
+                venv_path: self.venv_path.as_ref().map(RelativePathBuf::cli),
-                    .venv_path
+                typeshed: self.typeshed.as_ref().map(RelativePathBuf::cli),
-                    .as_ref()
-                    .map(|venv_path| SystemPath::absolute(venv_path, cli_cwd)),
-                typeshed: self
-                    .typeshed
-                    .as_ref()
-                    .map(|typeshed| SystemPath::absolute(typeshed, cli_cwd)),
                 extra_paths: self.extra_search_path.as_ref().map(|extra_search_paths| {
                     extra_search_paths
                         .iter()
-                        .map(|path| SystemPath::absolute(path, cli_cwd))
+                        .map(RelativePathBuf::cli)
                         .collect()
                 }),
                 ..EnvironmentOptions::default()
@@ -158,8 +153,8 @@ fn run() -> anyhow::Result<ExitStatus> {
         .transpose()?
         .unwrap_or_else(|| cli_base_path.clone());
 
-    let system = OsSystem::new(cwd.clone());
+    let system = OsSystem::new(cwd);
-    let cli_options = args.to_options(&cwd);
+    let cli_options = args.to_options();
     let mut workspace_metadata = ProjectMetadata::discover(system.current_directory(), &system)?;
     workspace_metadata.apply_cli_options(cli_options.clone());
 

crates/red_knot/tests/cli.rs

@@ -1,12 +1,13 @@
 use anyhow::Context;
 use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
+use std::path::Path;
 use std::process::Command;
 use tempfile::TempDir;
 
 /// Specifying an option on the CLI should take precedence over the same setting in the
 /// project's configuration.
 #[test]
-fn test_config_override() -> anyhow::Result<()> {
+fn config_override() -> anyhow::Result<()> {
     let tempdir = TempDir::new()?;
 
     std::fs::write(
@@ -29,7 +30,7 @@ print(sys.last_exc)
     )
     .context("Failed to write test.py")?;
 
-    insta::with_settings!({filters => vec![(&*tempdir_filter(&tempdir), "<temp_dir>/")]}, {
+    insta::with_settings!({filters => vec![(&*tempdir_filter(tempdir.path()), "<temp_dir>/")]}, {
         assert_cmd_snapshot!(knot().arg("--project").arg(tempdir.path()), @r"
         success: false
         exit_code: 1
@@ -51,10 +52,163 @@ print(sys.last_exc)
     Ok(())
 }
 
+/// Paths specified on the CLI are relative to the current working directory and not the project root.
+///
+/// We test this by adding an extra search path from the CLI to the libs directory when
+/// running the CLI from the child directory (using relative paths).
+///
+/// Project layout:
+/// ```
+///  - libs
+///    |- utils.py
+///  - child
+///    | - test.py
+/// - pyproject.toml
+/// ```
+///
+/// And the command is run in the `child` directory.
+#[test]
+fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> {
+    let tempdir = TempDir::new()?;
+
+    let project_dir = tempdir.path().canonicalize()?;
+
+    let libs = project_dir.join("libs");
+    std::fs::create_dir_all(&libs).context("Failed to create `libs` directory")?;
+
+    let child = project_dir.join("child");
+    std::fs::create_dir(&child).context("Failed to create `child` directory")?;
+
+    std::fs::write(
+        tempdir.path().join("pyproject.toml"),
+        r#"
+[tool.knot.environment]
+python-version = "3.11"
+"#,
+    )
+    .context("Failed to write `pyproject.toml`")?;
+
+    std::fs::write(
+        libs.join("utils.py"),
+        r#"
+def add(a: int, b: int) -> int:
+    a + b
+"#,
+    )
+    .context("Failed to write `utils.py`")?;
+
+    std::fs::write(
+        child.join("test.py"),
+        r#"
+from utils import add
+
+stat = add(10, 15)
+"#,
+    )
+    .context("Failed to write `child/test.py`")?;
+
+    let project_filter = tempdir_filter(&project_dir);
+    let filters = vec![
+        (&*project_filter, "<temp_dir>/"),
+        (r#"\\(\w\w|\s|\.|")"#, "/$1"),
+    ];
+
+    // Make sure that the CLI fails when the `libs` directory is not in the search path.
+    insta::with_settings!({filters => filters}, {
+        assert_cmd_snapshot!(knot().current_dir(&child), @r#"
+        success: false
+        exit_code: 1
+        ----- stdout -----
+        error[lint:unresolved-import] <temp_dir>/child/test.py:2:1 Cannot resolve import `utils`
+
+        ----- stderr -----
+        "#);
+    });
+
+    insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "<temp_dir>/")]}, {
+        assert_cmd_snapshot!(knot().current_dir(child).arg("--extra-search-path").arg("../libs"), @r"
+        success: true
+        exit_code: 0
+        ----- stdout -----
+
+        ----- stderr -----
+        ");
+    });
+
+    Ok(())
+}
+
+/// Paths specified in a configuration file are relative to the project root.
+///
+/// We test this by adding `libs` (as a relative path) to the extra search path in the configuration and run
+/// the CLI from a subdirectory.
+///
+/// Project layout:
+/// ```
+///  - libs
+///    |- utils.py
+///  - child
+///    | - test.py
+/// - pyproject.toml
+/// ```
+#[test]
+fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Result<()> {
+    let tempdir = TempDir::new()?;
+
+    let project_dir = tempdir.path();
+
+    let libs = project_dir.join("libs");
+    std::fs::create_dir_all(&libs).context("Failed to create `libs` directory")?;
+
+    let child = project_dir.join("child");
+    std::fs::create_dir(&child).context("Failed to create `child` directory")?;
+
+    std::fs::write(
+        tempdir.path().join("pyproject.toml"),
+        r#"
+[tool.knot.environment]
+python-version = "3.11"
+extra-paths = ["libs"]
+"#,
+    )
+    .context("Failed to write `pyproject.toml`")?;
+
+    std::fs::write(
+        libs.join("utils.py"),
+        r#"
+def add(a: int, b: int) -> int:
+    a + b
+"#,
+    )
+    .context("Failed to write `utils.py`")?;
+
+    std::fs::write(
+        child.join("test.py"),
+        r#"
+from utils import add
+
+stat = add(10, 15)
+"#,
+    )
+    .context("Failed to write `child/test.py`")?;
+
+    insta::with_settings!({filters => vec![(&*tempdir_filter(tempdir.path()), "<temp_dir>/")]}, {
+        assert_cmd_snapshot!(knot().current_dir(child), @r"
+        success: true
+        exit_code: 0
+        ----- stdout -----
+
+        ----- stderr -----
+        ");
+    });
+
+    Ok(())
+}
+
 fn knot() -> Command {
     Command::new(get_cargo_bin("red_knot"))
 }
 
-fn tempdir_filter(tempdir: &TempDir) -> String {
+fn tempdir_filter(path: &Path) -> String {
-    format!(r"{}\\?/?", regex::escape(tempdir.path().to_str().unwrap()))
+    format!(r"{}\\?/?", regex::escape(path.to_str().unwrap()))
 }

crates/red_knot/tests/file_watching.rs

@@ -6,6 +6,7 @@ use std::time::{Duration, Instant};
 use anyhow::{anyhow, Context};
 use red_knot_project::metadata::options::{EnvironmentOptions, Options};
 use red_knot_project::metadata::pyproject::{PyProject, Tool};
+use red_knot_project::metadata::value::RelativePathBuf;
 use red_knot_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
 use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
 use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform, PythonVersion};
@@ -791,7 +792,7 @@ fn search_path() -> anyhow::Result<()> {
     let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| {
         Some(Options {
             environment: Some(EnvironmentOptions {
-                extra_paths: Some(vec![root_path.join("site_packages")]),
+                extra_paths: Some(vec![RelativePathBuf::cli(root_path.join("site_packages"))]),
                 ..EnvironmentOptions::default()
             }),
             ..Options::default()
@@ -832,7 +833,7 @@ fn add_search_path() -> anyhow::Result<()> {
     // Register site-packages as a search path.
     case.update_options(Options {
         environment: Some(EnvironmentOptions {
-            extra_paths: Some(vec![site_packages.clone()]),
+            extra_paths: Some(vec![RelativePathBuf::cli("site_packages")]),
             ..EnvironmentOptions::default()
         }),
         ..Options::default()
@@ -855,7 +856,7 @@ fn remove_search_path() -> anyhow::Result<()> {
     let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| {
         Some(Options {
             environment: Some(EnvironmentOptions {
-                extra_paths: Some(vec![root_path.join("site_packages")]),
+                extra_paths: Some(vec![RelativePathBuf::cli(root_path.join("site_packages"))]),
                 ..EnvironmentOptions::default()
             }),
             ..Options::default()
@@ -951,7 +952,7 @@ fn changed_versions_file() -> anyhow::Result<()> {
         |root_path, _project_path| {
             Some(Options {
                 environment: Some(EnvironmentOptions {
-                    typeshed: Some(root_path.join("typeshed")),
+                    typeshed: Some(RelativePathBuf::cli(root_path.join("typeshed"))),
                     ..EnvironmentOptions::default()
                 }),
                 ..Options::default()
@@ -1375,10 +1376,12 @@ mod unix {
 
                 Ok(())
             },
-            |_root, project| {
+            |_root, _project| {
                 Some(Options {
                     environment: Some(EnvironmentOptions {
-                        extra_paths: Some(vec![project.join(".venv/lib/python3.12/site-packages")]),
+                        extra_paths: Some(vec![RelativePathBuf::cli(
+                            ".venv/lib/python3.12/site-packages",
+                        )]),
                         python_version: Some(PythonVersion::PY312),
                         ..EnvironmentOptions::default()
                     }),

crates/red_knot_project/src/metadata.rs

@@ -1,15 +1,18 @@
 use red_knot_python_semantic::ProgramSettings;
 use ruff_db::system::{System, SystemPath, SystemPathBuf};
 use ruff_python_ast::name::Name;
+use std::sync::Arc;
 use thiserror::Error;
 
 use crate::combine::Combine;
 use crate::metadata::pyproject::{Project, PyProject, PyProjectError};
+use crate::metadata::value::ValueSource;
 use options::KnotTomlError;
 use options::Options;
 
 pub mod options;
 pub mod pyproject;
+pub mod value;
 
 #[derive(Debug, PartialEq, Eq)]
 #[cfg_attr(test, derive(serde::Serialize))]
@@ -87,7 +90,10 @@ impl ProjectMetadata {
             let pyproject_path = project_root.join("pyproject.toml");
 
             let pyproject = if let Ok(pyproject_str) = system.read_to_string(&pyproject_path) {
-                match PyProject::from_toml_str(&pyproject_str) {
+                match PyProject::from_toml_str(
+                    &pyproject_str,
+                    ValueSource::File(Arc::new(pyproject_path.clone())),
+                ) {
                     Ok(pyproject) => Some(pyproject),
                     Err(error) => {
                         return Err(ProjectDiscoveryError::InvalidPyProject {
@@ -103,7 +109,10 @@ impl ProjectMetadata {
             // A `knot.toml` takes precedence over a `pyproject.toml`.
             let knot_toml_path = project_root.join("knot.toml");
             if let Ok(knot_str) = system.read_to_string(&knot_toml_path) {
-                let options = match Options::from_toml_str(&knot_str) {
+                let options = match Options::from_toml_str(
+                    &knot_str,
+                    ValueSource::File(Arc::new(knot_toml_path.clone())),
+                ) {
                     Ok(options) => options,
                     Err(error) => {
                         return Err(ProjectDiscoveryError::InvalidKnotToml {

crates/red_knot_project/src/metadata/options.rs

@@ -1,7 +1,8 @@
+use crate::metadata::value::{RelativePathBuf, ValueSource, ValueSourceGuard};
 use red_knot_python_semantic::{
     ProgramSettings, PythonPlatform, PythonVersion, SearchPathSettings, SitePackages,
 };
-use ruff_db::system::{System, SystemPath, SystemPathBuf};
+use ruff_db::system::{System, SystemPath};
 use ruff_macros::Combine;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
@@ -16,7 +17,8 @@ pub struct Options {
 }
 
 impl Options {
-    pub(crate) fn from_toml_str(content: &str) -> Result<Self, KnotTomlError> {
+    pub(crate) fn from_toml_str(content: &str, source: ValueSource) -> Result<Self, KnotTomlError> {
+        let _guard = ValueSourceGuard::new(source);
         let options = toml::from_str(content)?;
         Ok(options)
     }
@@ -44,19 +46,19 @@ impl Options {
         project_root: &SystemPath,
         system: &dyn System,
     ) -> SearchPathSettings {
-        let src_roots =
+        let src_roots = if let Some(src_root) = self.src.as_ref().and_then(|src| src.root.as_ref())
-            if let Some(src_root) = self.src.as_ref().and_then(|src| src.root.as_deref()) {
+        {
-                vec![src_root.to_path_buf()]
+            vec![src_root.absolute(project_root, system)]
-            } else {
+        } else {
-                let src = project_root.join("src");
+            let src = project_root.join("src");
 
-                // Default to `src` and the project root if `src` exists and the root hasn't been specified.
+            // Default to `src` and the project root if `src` exists and the root hasn't been specified.
-                if system.is_directory(&src) {
+            if system.is_directory(&src) {
-                    vec![project_root.to_path_buf(), src]
+                vec![project_root.to_path_buf(), src]
-                } else {
+            } else {
-                    vec![project_root.to_path_buf()]
+                vec![project_root.to_path_buf()]
-                }
+            }
-            };
+        };
 
         let (extra_paths, python, typeshed) = self
             .environment
@@ -71,11 +73,17 @@ impl Options {
             .unwrap_or_default();
 
         SearchPathSettings {
-            extra_paths: extra_paths.unwrap_or_default(),
+            extra_paths: extra_paths
+                .unwrap_or_default()
+                .into_iter()
+                .map(|path| path.absolute(project_root, system))
+                .collect(),
             src_roots,
-            typeshed,
+            typeshed: typeshed.map(|path| path.absolute(project_root, system)),
             site_packages: python
-                .map(|venv_path| SitePackages::Derived { venv_path })
+                .map(|venv_path| SitePackages::Derived {
+                    venv_path: venv_path.absolute(project_root, system),
+                })
                 .unwrap_or(SitePackages::Known(vec![])),
         }
     }
@@ -91,23 +99,23 @@ pub struct EnvironmentOptions {
     /// List of user-provided paths that should take first priority in the module resolution.
     /// Examples in other type checkers are mypy's MYPYPATH environment variable,
     /// or pyright's stubPath configuration setting.
-    pub extra_paths: Option<Vec<SystemPathBuf>>,
+    pub extra_paths: Option<Vec<RelativePathBuf>>,
 
     /// Optional path to a "typeshed" directory on disk for us to use for standard-library types.
     /// If this is not provided, we will fallback to our vendored typeshed stubs for the stdlib,
     /// bundled as a zip file in the binary
-    pub typeshed: Option<SystemPathBuf>,
+    pub typeshed: Option<RelativePathBuf>,
 
     // TODO: Rename to python, see https://github.com/astral-sh/ruff/issues/15530
     /// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.
-    pub venv_path: Option<SystemPathBuf>,
+    pub venv_path: Option<RelativePathBuf>,
 }
 
 #[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case", deny_unknown_fields)]
 pub struct SrcOptions {
     /// The root of the project, used for finding first-party modules.
-    pub root: Option<SystemPathBuf>,
+    pub root: Option<RelativePathBuf>,
 }
 
 #[derive(Error, Debug)]

crates/red_knot_project/src/metadata/pyproject.rs

@@ -4,6 +4,7 @@ use std::ops::Deref;
 use thiserror::Error;
 
 use crate::metadata::options::Options;
+use crate::metadata::value::{ValueSource, ValueSourceGuard};
 
 /// A `pyproject.toml` as specified in PEP 517.
 #[derive(Deserialize, Serialize, Debug, Default, Clone)]
@@ -28,7 +29,11 @@ pub enum PyProjectError {
 }
 
 impl PyProject {
-    pub(crate) fn from_toml_str(content: &str) -> Result<Self, PyProjectError> {
+    pub(crate) fn from_toml_str(
+        content: &str,
+        source: ValueSource,
+    ) -> Result<Self, PyProjectError> {
+        let _guard = ValueSourceGuard::new(source);
         toml::from_str(content).map_err(PyProjectError::TomlSyntax)
     }
 }

crates/red_knot_project/src/metadata/value.rs

@@ -0,0 +1,163 @@
+use ruff_db::system::{System, SystemPath, SystemPathBuf};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use std::cell::RefCell;
+use std::hash::{Hash, Hasher};
+use std::sync::Arc;
+
+use crate::combine::Combine;
+use crate::Db;
+
+#[derive(Clone, Debug)]
+pub enum ValueSource {
+    /// Value loaded from a project's configuration file.
+    ///
+    /// Ideally, we'd use [`ruff_db::files::File`] but we can't because the database hasn't been
+    /// created when loading the configuration.
+    File(Arc<SystemPathBuf>),
+    /// The value comes from a CLI argument, while it's left open if specified using a short argument,
+    /// long argument (`--extra-paths`) or `--config key=value`.
+    Cli,
+}
+
+thread_local! {
+    /// Serde doesn't provide any easy means to pass a value to a [`Deserialize`] implementation,
+    /// but we want to associate each deserialized [`RelativePath`] with the source from
+    /// which it originated. We use a thread local variable to work around this limitation.
+    ///
+    /// Use the [`ValueSourceGuard`] to initialize the thread local before calling into any
+    /// deserialization code. It ensures that the thread local variable gets cleaned up
+    /// once deserialization is done (once the guard gets dropped).
+    static VALUE_SOURCE: RefCell<Option<ValueSource>> = const { RefCell::new(None) };
+}
+
+/// Guard to safely change the [`VALUE_SOURCE`] for the current thread.
+#[must_use]
+pub(super) struct ValueSourceGuard {
+    prev_value: Option<ValueSource>,
+}
+
+impl ValueSourceGuard {
+    pub(super) fn new(source: ValueSource) -> Self {
+        let prev = VALUE_SOURCE.replace(Some(source));
+        Self { prev_value: prev }
+    }
+}
+
+impl Drop for ValueSourceGuard {
+    fn drop(&mut self) {
+        VALUE_SOURCE.set(self.prev_value.take());
+    }
+}
+
+/// A possibly relative path in a configuration file.
+///
+/// Relative paths in configuration files or from CLI options
+/// require different anchoring:
+///
+/// * CLI: The path is relative to the current working directory
+/// * Configuration file: The path is relative to the project's root.
+#[derive(Debug, Clone)]
+pub struct RelativePathBuf {
+    path: SystemPathBuf,
+    source: ValueSource,
+}
+
+impl RelativePathBuf {
+    pub fn new(path: impl AsRef<SystemPath>, source: ValueSource) -> Self {
+        Self {
+            path: path.as_ref().to_path_buf(),
+            source,
+        }
+    }
+
+    pub fn cli(path: impl AsRef<SystemPath>) -> Self {
+        Self::new(path, ValueSource::Cli)
+    }
+
+    /// Returns the relative path as specified by the user.
+    pub fn path(&self) -> &SystemPath {
+        &self.path
+    }
+
+    /// Returns the owned relative path.
+    pub fn into_path_buf(self) -> SystemPathBuf {
+        self.path
+    }
+
+    /// Resolves the absolute path for `self` based on its origin.
+    pub fn absolute_with_db(&self, db: &dyn Db) -> SystemPathBuf {
+        self.absolute(db.project().root(db), db.system())
+    }
+
+    /// Resolves the absolute path for `self` based on its origin.
+    pub fn absolute(&self, project_root: &SystemPath, system: &dyn System) -> SystemPathBuf {
+        let relative_to = match &self.source {
+            ValueSource::File(_) => project_root,
+            ValueSource::Cli => system.current_directory(),
+        };
+
+        SystemPath::absolute(&self.path, relative_to)
+    }
+}
+
+// TODO(micha): Derive most of those implementations once `RelativePath` uses `Value`.
+//   and use `serde(transparent, deny_unknown_fields)`
+impl Combine for RelativePathBuf {
+    fn combine(self, _other: Self) -> Self {
+        self
+    }
+
+    #[inline(always)]
+    fn combine_with(&mut self, _other: Self) {}
+}
+
+impl Hash for RelativePathBuf {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.path.hash(state);
+    }
+}
+
+impl PartialEq for RelativePathBuf {
+    fn eq(&self, other: &Self) -> bool {
+        self.path.eq(&other.path)
+    }
+}
+
+impl Eq for RelativePathBuf {}
+
+impl PartialOrd for RelativePathBuf {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for RelativePathBuf {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.path.cmp(&other.path)
+    }
+}
+
+impl Serialize for RelativePathBuf {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        self.path.serialize(serializer)
+    }
+}
+
+impl<'de> Deserialize<'de> for RelativePathBuf {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let path = SystemPathBuf::deserialize(deserializer)?;
+        Ok(VALUE_SOURCE.with_borrow(|source| {
+            let source = source
+                .clone()
+                .expect("Thread local `VALUE_SOURCE` to be set. Use `ValueSourceGuard` to set the value source before calling serde/toml `from_str`.");
+
+            Self { path, source }
+        }))
+    }
+}