diff --git a/crates/red_knot/src/main.rs b/crates/red_knot/src/main.rs index 3c1751e540..a8bb2850f8 100644 --- a/crates/red_knot/src/main.rs +++ b/crates/red_knot/src/main.rs @@ -99,10 +99,11 @@ fn run_check(args: CheckCommand) -> anyhow::Result { let exit_zero = args.exit_zero; let cli_options = args.into_options(); - let mut workspace_metadata = ProjectMetadata::discover(system.current_directory(), &system)?; - workspace_metadata.apply_cli_options(cli_options.clone()); + let mut project_metadata = ProjectMetadata::discover(system.current_directory(), &system)?; + project_metadata.apply_cli_options(cli_options.clone()); + project_metadata.apply_configuration_files(&system)?; - let mut db = ProjectDatabase::new(workspace_metadata, system)?; + let mut db = ProjectDatabase::new(project_metadata, system)?; let (main_loop, main_loop_cancellation_token) = MainLoop::new(cli_options); diff --git a/crates/red_knot/tests/cli.rs b/crates/red_knot/tests/cli.rs index 28360dfc87..3ac2d260bf 100644 --- a/crates/red_knot/tests/cli.rs +++ b/crates/red_knot/tests/cli.rs @@ -98,7 +98,7 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> { ])?; // Make sure that the CLI fails when the `libs` directory is not in the search path. - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r###" + assert_cmd_snapshot!(case.command().current_dir(case.root().join("child")), @r###" success: false exit_code: 1 ----- stdout ----- @@ -115,7 +115,7 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> { ----- stderr ----- "###); - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")).arg("--extra-search-path").arg("../libs"), @r" + assert_cmd_snapshot!(case.command().current_dir(case.root().join("child")).arg("--extra-search-path").arg("../libs"), @r" success: true exit_code: 0 ----- stdout ----- @@ -167,7 +167,7 @@ fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Re ), ])?; - assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r" + assert_cmd_snapshot!(case.command().current_dir(case.root().join("child")), @r" success: true exit_code: 0 ----- stdout ----- @@ -717,6 +717,109 @@ fn exit_code_exit_zero_is_true() -> anyhow::Result<()> { Ok(()) } +#[test] +fn user_configuration() -> anyhow::Result<()> { + let case = TestCase::with_files([ + ( + "project/knot.toml", + r#" + [rules] + division-by-zero = "warn" + "#, + ), + ( + "project/main.py", + r#" + y = 4 / 0 + + for a in range(0, y): + x = a + + print(x) + "#, + ), + ])?; + + let config_directory = case.root().join("home/.config"); + let config_env_var = if cfg!(windows) { + "APPDATA" + } else { + "XDG_CONFIG_HOME" + }; + + assert_cmd_snapshot!( + case.command().current_dir(case.root().join("project")).env(config_env_var, config_directory.as_os_str()), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + warning: lint:division-by-zero + --> /project/main.py:2:5 + | + 2 | y = 4 / 0 + | ----- Cannot divide object of type `Literal[4]` by zero + 3 | + 4 | for a in range(0, y): + | + + warning: lint:possibly-unresolved-reference + --> /project/main.py:7:7 + | + 5 | x = a + 6 | + 7 | print(x) + | - Name `x` used when possibly not defined + | + + + ----- stderr ----- + "### + ); + + // The user-level configuration promotes `possibly-unresolved-reference` to an error. + // Changing the level for `division-by-zero` has no effect, because the project-level configuration + // has higher precedence. + case.write_file( + config_directory.join("knot/knot.toml"), + r#" + [rules] + division-by-zero = "error" + possibly-unresolved-reference = "error" + "#, + )?; + + assert_cmd_snapshot!( + case.command().current_dir(case.root().join("project")).env(config_env_var, config_directory.as_os_str()), + @r###" + success: false + exit_code: 1 + ----- stdout ----- + warning: lint:division-by-zero + --> /project/main.py:2:5 + | + 2 | y = 4 / 0 + | ----- Cannot divide object of type `Literal[4]` by zero + 3 | + 4 | for a in range(0, y): + | + + error: lint:possibly-unresolved-reference + --> /project/main.py:7:7 + | + 5 | x = a + 6 | + 7 | print(x) + | ^ Name `x` used when possibly not defined + | + + + ----- stderr ----- + "### + ); + + Ok(()) +} + struct TestCase { _temp_dir: TempDir, _settings_scope: SettingsBindDropGuard, @@ -784,7 +887,7 @@ impl TestCase { Ok(()) } - fn project_dir(&self) -> &Path { + fn root(&self) -> &Path { &self.project_dir } diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs index eaad5ed0dd..ff1f21b982 100644 --- a/crates/red_knot/tests/file_watching.rs +++ b/crates/red_knot/tests/file_watching.rs @@ -12,7 +12,9 @@ use red_knot_project::{Db, ProjectDatabase, ProjectMetadata}; use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform, PythonVersion}; use ruff_db::files::{system_path_to_file, File, FileError}; use ruff_db::source::source_text; -use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; +use ruff_db::system::{ + OsSystem, System, SystemPath, SystemPathBuf, UserConfigDirectoryOverrideGuard, +}; use ruff_db::Upcast; struct TestCase { @@ -220,17 +222,44 @@ where } trait SetupFiles { - fn setup(self, root_path: &SystemPath, project_path: &SystemPath) -> anyhow::Result<()>; + fn setup(self, context: &SetupContext) -> anyhow::Result<()>; +} + +struct SetupContext<'a> { + system: &'a OsSystem, + root_path: &'a SystemPath, +} + +impl<'a> SetupContext<'a> { + fn system(&self) -> &'a OsSystem { + self.system + } + + fn join_project_path(&self, relative: impl AsRef) -> SystemPathBuf { + self.project_path().join(relative) + } + + fn project_path(&self) -> &SystemPath { + self.system.current_directory() + } + + fn root_path(&self) -> &'a SystemPath { + self.root_path + } + + fn join_root_path(&self, relative: impl AsRef) -> SystemPathBuf { + self.root_path().join(relative) + } } impl SetupFiles for [(P, &'static str); N] where P: AsRef, { - fn setup(self, _root_path: &SystemPath, project_path: &SystemPath) -> anyhow::Result<()> { + fn setup(self, context: &SetupContext) -> anyhow::Result<()> { for (relative_path, content) in self { let relative_path = relative_path.as_ref(); - let absolute_path = project_path.join(relative_path); + let absolute_path = context.join_project_path(relative_path); if let Some(parent) = absolute_path.parent() { std::fs::create_dir_all(parent).with_context(|| { format!("Failed to create parent directory for file `{relative_path}`") @@ -250,10 +279,10 @@ where impl SetupFiles for F where - F: FnOnce(&SystemPath, &SystemPath) -> anyhow::Result<()>, + F: FnOnce(&SetupContext) -> anyhow::Result<()>, { - fn setup(self, root_path: &SystemPath, project_path: &SystemPath) -> anyhow::Result<()> { - self(root_path, project_path) + fn setup(self, context: &SetupContext) -> anyhow::Result<()> { + self(context) } } @@ -261,13 +290,12 @@ fn setup(setup_files: F) -> anyhow::Result where F: SetupFiles, { - setup_with_options(setup_files, |_root, _project_path| None) + setup_with_options(setup_files, |_context| None) } -// TODO: Replace with configuration? fn setup_with_options( setup_files: F, - create_options: impl FnOnce(&SystemPath, &SystemPath) -> Option, + create_options: impl FnOnce(&SetupContext) -> Option, ) -> anyhow::Result where F: SetupFiles, @@ -295,13 +323,17 @@ where std::fs::create_dir_all(project_path.as_std_path()) .with_context(|| format!("Failed to create project directory `{project_path}`"))?; + let system = OsSystem::new(&project_path); + let setup_context = SetupContext { + system: &system, + root_path: &root_path, + }; + setup_files - .setup(&root_path, &project_path) + .setup(&setup_context) .context("Failed to setup test files")?; - let system = OsSystem::new(&project_path); - - if let Some(options) = create_options(&root_path, &project_path) { + if let Some(options) = create_options(&setup_context) { std::fs::write( project_path.join("pyproject.toml").as_std_path(), toml::to_string(&PyProject { @@ -315,7 +347,9 @@ where .context("Failed to write configuration")?; } - let project = ProjectMetadata::discover(&project_path, &system)?; + let mut project = ProjectMetadata::discover(&project_path, &system)?; + project.apply_configuration_files(&system)?; + let program_settings = project.to_program_settings(&system); for path in program_settings @@ -789,10 +823,12 @@ fn directory_deleted() -> anyhow::Result<()> { #[test] fn search_path() -> anyhow::Result<()> { - let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| { + let mut case = setup_with_options([("bar.py", "import sub.a")], |context| { Some(Options { environment: Some(EnvironmentOptions { - extra_paths: Some(vec![RelativePathBuf::cli(root_path.join("site_packages"))]), + extra_paths: Some(vec![RelativePathBuf::cli( + context.join_root_path("site_packages"), + )]), ..EnvironmentOptions::default() }), ..Options::default() @@ -853,10 +889,12 @@ fn add_search_path() -> anyhow::Result<()> { #[test] fn remove_search_path() -> anyhow::Result<()> { - let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| { + let mut case = setup_with_options([("bar.py", "import sub.a")], |context| { Some(Options { environment: Some(EnvironmentOptions { - extra_paths: Some(vec![RelativePathBuf::cli(root_path.join("site_packages"))]), + extra_paths: Some(vec![RelativePathBuf::cli( + context.join_root_path("site_packages"), + )]), ..EnvironmentOptions::default() }), ..Options::default() @@ -894,7 +932,7 @@ import os print(sys.last_exc, os.getegid()) "#, )], - |_root_path, _project_path| { + |_context| { Some(Options { environment: Some(EnvironmentOptions { python_version: Some(RangedValue::cli(PythonVersion::PY311)), @@ -942,21 +980,31 @@ print(sys.last_exc, os.getegid()) #[test] fn changed_versions_file() -> anyhow::Result<()> { let mut case = setup_with_options( - |root_path: &SystemPath, project_path: &SystemPath| { - std::fs::write(project_path.join("bar.py").as_std_path(), "import sub.a")?; - std::fs::create_dir_all(root_path.join("typeshed/stdlib").as_std_path())?; - std::fs::write(root_path.join("typeshed/stdlib/VERSIONS").as_std_path(), "")?; + |context: &SetupContext| { std::fs::write( - root_path.join("typeshed/stdlib/os.pyi").as_std_path(), + context.join_project_path("bar.py").as_std_path(), + "import sub.a", + )?; + std::fs::create_dir_all(context.join_root_path("typeshed/stdlib").as_std_path())?; + std::fs::write( + context + .join_root_path("typeshed/stdlib/VERSIONS") + .as_std_path(), + "", + )?; + std::fs::write( + context + .join_root_path("typeshed/stdlib/os.pyi") + .as_std_path(), "# not important", )?; Ok(()) }, - |root_path, _project_path| { + |context| { Some(Options { environment: Some(EnvironmentOptions { - typeshed: Some(RelativePathBuf::cli(root_path.join("typeshed"))), + typeshed: Some(RelativePathBuf::cli(context.join_root_path("typeshed"))), ..EnvironmentOptions::default() }), ..Options::default() @@ -1007,12 +1055,12 @@ fn changed_versions_file() -> anyhow::Result<()> { /// we're seeing is that Windows only emits a single event, similar to Linux. #[test] fn hard_links_in_project() -> anyhow::Result<()> { - let mut case = setup(|_root: &SystemPath, project: &SystemPath| { - let foo_path = project.join("foo.py"); + let mut case = setup(|context: &SetupContext| { + let foo_path = context.join_project_path("foo.py"); std::fs::write(foo_path.as_std_path(), "print('Version 1')")?; // Create a hardlink to `foo` - let bar_path = project.join("bar.py"); + let bar_path = context.join_project_path("bar.py"); std::fs::hard_link(foo_path.as_std_path(), bar_path.as_std_path()) .context("Failed to create hard link from foo.py -> bar.py")?; @@ -1078,12 +1126,12 @@ fn hard_links_in_project() -> anyhow::Result<()> { ignore = "windows doesn't support observing changes to hard linked files." )] fn hard_links_to_target_outside_project() -> anyhow::Result<()> { - let mut case = setup(|root: &SystemPath, project: &SystemPath| { - let foo_path = root.join("foo.py"); + let mut case = setup(|context: &SetupContext| { + let foo_path = context.join_root_path("foo.py"); std::fs::write(foo_path.as_std_path(), "print('Version 1')")?; // Create a hardlink to `foo` - let bar_path = project.join("bar.py"); + let bar_path = context.join_project_path("bar.py"); std::fs::hard_link(foo_path.as_std_path(), bar_path.as_std_path()) .context("Failed to create hard link from foo.py -> bar.py")?; @@ -1186,9 +1234,9 @@ mod unix { ignore = "FSEvents doesn't emit change events for symlinked directories outside of the watched paths." )] fn symlink_target_outside_watched_paths() -> anyhow::Result<()> { - let mut case = setup(|root: &SystemPath, project: &SystemPath| { + let mut case = setup(|context: &SetupContext| { // Set up the symlink target. - let link_target = root.join("bar"); + let link_target = context.join_root_path("bar"); std::fs::create_dir_all(link_target.as_std_path()) .context("Failed to create link target directory")?; let baz_original = link_target.join("baz.py"); @@ -1196,7 +1244,7 @@ mod unix { .context("Failed to write link target file")?; // Create a symlink inside the project - let bar = project.join("bar"); + let bar = context.join_project_path("bar"); std::os::unix::fs::symlink(link_target.as_std_path(), bar.as_std_path()) .context("Failed to create symlink to bar package")?; @@ -1267,9 +1315,9 @@ mod unix { /// ``` #[test] fn symlink_inside_project() -> anyhow::Result<()> { - let mut case = setup(|_root: &SystemPath, project: &SystemPath| { + let mut case = setup(|context: &SetupContext| { // Set up the symlink target. - let link_target = project.join("patched/bar"); + let link_target = context.join_project_path("patched/bar"); std::fs::create_dir_all(link_target.as_std_path()) .context("Failed to create link target directory")?; let baz_original = link_target.join("baz.py"); @@ -1277,7 +1325,7 @@ mod unix { .context("Failed to write link target file")?; // Create a symlink inside site-packages - let bar_in_project = project.join("bar"); + let bar_in_project = context.join_project_path("bar"); std::os::unix::fs::symlink(link_target.as_std_path(), bar_in_project.as_std_path()) .context("Failed to create symlink to bar package")?; @@ -1358,9 +1406,9 @@ mod unix { #[test] fn symlinked_module_search_path() -> anyhow::Result<()> { let mut case = setup_with_options( - |root: &SystemPath, project: &SystemPath| { + |context: &SetupContext| { // Set up the symlink target. - let site_packages = root.join("site-packages"); + let site_packages = context.join_root_path("site-packages"); let bar = site_packages.join("bar"); std::fs::create_dir_all(bar.as_std_path()) .context("Failed to create bar directory")?; @@ -1369,7 +1417,8 @@ mod unix { .context("Failed to write baz.py")?; // Symlink the site packages in the venv to the global site packages - let venv_site_packages = project.join(".venv/lib/python3.12/site-packages"); + let venv_site_packages = + context.join_project_path(".venv/lib/python3.12/site-packages"); std::fs::create_dir_all(venv_site_packages.parent().unwrap()) .context("Failed to create .venv directory")?; std::os::unix::fs::symlink( @@ -1380,7 +1429,7 @@ mod unix { Ok(()) }, - |_root, _project| { + |_context| { Some(Options { environment: Some(EnvironmentOptions { extra_paths: Some(vec![RelativePathBuf::cli( @@ -1450,9 +1499,9 @@ mod unix { #[test] fn nested_projects_delete_root() -> anyhow::Result<()> { - let mut case = setup(|root: &SystemPath, project_root: &SystemPath| { + let mut case = setup(|context: &SetupContext| { std::fs::write( - project_root.join("pyproject.toml").as_std_path(), + context.join_project_path("pyproject.toml").as_std_path(), r#" [project] name = "inner" @@ -1462,7 +1511,7 @@ fn nested_projects_delete_root() -> anyhow::Result<()> { )?; std::fs::write( - root.join("pyproject.toml").as_std_path(), + context.join_root_path("pyproject.toml").as_std_path(), r#" [project] name = "outer" @@ -1487,3 +1536,79 @@ fn nested_projects_delete_root() -> anyhow::Result<()> { Ok(()) } + +#[test] +fn changes_to_user_configuration() -> anyhow::Result<()> { + let mut _config_dir_override: Option = None; + + let mut case = setup(|context: &SetupContext| { + std::fs::write( + context.join_project_path("pyproject.toml").as_std_path(), + r#" + [project] + name = "test" + "#, + )?; + + std::fs::write( + context.join_project_path("foo.py").as_std_path(), + "a = 10 / 0", + )?; + + let config_directory = context.join_root_path("home/.config"); + std::fs::create_dir_all(config_directory.join("knot").as_std_path())?; + std::fs::write( + config_directory.join("knot/knot.toml").as_std_path(), + r#" + [rules] + division-by-zero = "ignore" + "#, + )?; + + _config_dir_override = Some( + context + .system() + .with_user_config_directory(Some(config_directory)), + ); + + Ok(()) + })?; + + let foo = case + .system_file(case.project_path("foo.py")) + .expect("foo.py to exist"); + let diagnostics = case + .db() + .check_file(foo) + .context("Failed to check project.")?; + + assert!( + diagnostics.is_empty(), + "Expected no diagnostics but got: {diagnostics:#?}" + ); + + // Enable division-by-zero in the user configuration with warning severity + update_file( + case.root_path().join("home/.config/knot/knot.toml"), + r#" + [rules] + division-by-zero = "warn" + "#, + )?; + + let changes = case.stop_watch(event_for_file("knot.toml")); + + case.apply_changes(changes); + + let diagnostics = case + .db() + .check_file(foo) + .context("Failed to check project.")?; + + assert!( + diagnostics.len() == 1, + "Expected exactly one diagnostic but got: {diagnostics:#?}" + ); + + Ok(()) +} diff --git a/crates/red_knot_project/src/db/changes.rs b/crates/red_knot_project/src/db/changes.rs index a1caf26376..8c7111ed43 100644 --- a/crates/red_knot_project/src/db/changes.rs +++ b/crates/red_knot_project/src/db/changes.rs @@ -8,6 +8,7 @@ use ruff_db::files::{system_path_to_file, File, Files}; use ruff_db::system::walk_directory::WalkState; use ruff_db::system::SystemPath; use ruff_db::Db as _; +use ruff_python_ast::PySourceType; use rustc_hash::FxHashSet; impl ProjectDatabase { @@ -47,7 +48,7 @@ impl ProjectDatabase { if let Some(path) = change.system_path() { if matches!( path.file_name(), - Some(".gitignore" | ".ignore" | "ruff.toml" | ".ruff.toml" | "pyproject.toml") + Some(".gitignore" | ".ignore" | "knot.toml" | "pyproject.toml") ) { // Changes to ignore files or settings can change the project structure or add/remove files. project_changed = true; @@ -144,6 +145,12 @@ impl ProjectDatabase { metadata.apply_cli_options(cli_options.clone()); } + if let Err(error) = metadata.apply_configuration_files(self.system()) { + tracing::error!( + "Failed to apply configuration files, continuing without applying them: {error}" + ); + } + let program_settings = metadata.to_program_settings(self.system()); let program = Program::get(self); @@ -201,9 +208,16 @@ impl ProjectDatabase { return WalkState::Continue; } - let mut paths = added_paths.lock().unwrap(); + if entry + .path() + .extension() + .and_then(PySourceType::try_from_extension) + .is_some() + { + let mut paths = added_paths.lock().unwrap(); - paths.push(entry.into_path()); + paths.push(entry.into_path()); + } WalkState::Continue }) diff --git a/crates/red_knot_project/src/metadata.rs b/crates/red_knot_project/src/metadata.rs index c7f2009c0e..38217e8979 100644 --- a/crates/red_knot_project/src/metadata.rs +++ b/crates/red_knot_project/src/metadata.rs @@ -1,3 +1,4 @@ +use configuration_file::{ConfigurationFile, ConfigurationFileError}; use red_knot_python_semantic::ProgramSettings; use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_python_ast::name::Name; @@ -10,6 +11,7 @@ use crate::metadata::value::ValueSource; use options::KnotTomlError; use options::Options; +mod configuration_file; pub mod options; pub mod pyproject; pub mod settings; @@ -24,6 +26,15 @@ pub struct ProjectMetadata { /// The raw options pub(super) options: Options, + + /// Paths of configurations other than the project's configuration that were combined into [`Self::options`]. + /// + /// This field stores the paths of the configuration files, mainly for + /// knowing which files to watch for changes. + /// + /// The path ordering doesn't imply precedence. + #[cfg_attr(test, serde(skip_serializing_if = "Vec::is_empty"))] + pub(super) extra_configuration_paths: Vec, } impl ProjectMetadata { @@ -32,6 +43,7 @@ impl ProjectMetadata { Self { name, root, + extra_configuration_paths: Vec::default(), options: Options::default(), } } @@ -64,6 +76,7 @@ impl ProjectMetadata { name, root, options, + extra_configuration_paths: Vec::new(), } } @@ -192,6 +205,10 @@ impl ProjectMetadata { &self.options } + pub fn extra_configuration_paths(&self) -> &[SystemPathBuf] { + &self.extra_configuration_paths + } + pub fn to_program_settings(&self, system: &dyn System) -> ProgramSettings { self.options.to_program_settings(self.root(), system) } @@ -201,9 +218,31 @@ impl ProjectMetadata { self.options = options.combine(std::mem::take(&mut self.options)); } - /// Combine the project options with the user options where project options take precedence. - pub fn apply_user_options(&mut self, options: Options) { - self.options.combine_with(options); + /// Applies the options from the configuration files to the project's options. + /// + /// This includes: + /// + /// * The user-level configuration + pub fn apply_configuration_files( + &mut self, + system: &dyn System, + ) -> Result<(), ConfigurationFileError> { + if let Some(user) = ConfigurationFile::user(system)? { + tracing::debug!( + "Applying user-level configuration loaded from `{path}`.", + path = user.path() + ); + self.apply_configuration_file(user); + } + + Ok(()) + } + + /// Applies a lower-precedence configuration files to the project's options. + fn apply_configuration_file(&mut self, options: ConfigurationFile) { + self.extra_configuration_paths + .push(options.path().to_owned()); + self.options.combine_with(options.into_options()); } } diff --git a/crates/red_knot_project/src/metadata/configuration_file.rs b/crates/red_knot_project/src/metadata/configuration_file.rs new file mode 100644 index 0000000000..03db373e36 --- /dev/null +++ b/crates/red_knot_project/src/metadata/configuration_file.rs @@ -0,0 +1,69 @@ +use std::sync::Arc; + +use ruff_db::system::{System, SystemPath, SystemPathBuf}; +use thiserror::Error; + +use crate::metadata::value::ValueSource; + +use super::options::{KnotTomlError, Options}; + +/// A `knot.toml` configuration file with the options it contains. +pub(crate) struct ConfigurationFile { + path: SystemPathBuf, + options: Options, +} + +impl ConfigurationFile { + /// Loads the user-level configuration file if it exists. + /// + /// Returns `None` if the file does not exist or if the concept of user-level configurations + /// doesn't exist on `system`. + pub(crate) fn user(system: &dyn System) -> Result, ConfigurationFileError> { + let Some(configuration_directory) = system.user_config_directory() else { + return Ok(None); + }; + + let knot_toml_path = configuration_directory.join("knot").join("knot.toml"); + + tracing::debug!( + "Searching for a user-level configuration at `{path}`", + path = &knot_toml_path + ); + + let Ok(knot_toml_str) = system.read_to_string(&knot_toml_path) else { + return Ok(None); + }; + + match Options::from_toml_str( + &knot_toml_str, + ValueSource::File(Arc::new(knot_toml_path.clone())), + ) { + Ok(options) => Ok(Some(Self { + path: knot_toml_path, + options, + })), + Err(error) => Err(ConfigurationFileError::InvalidKnotToml { + source: Box::new(error), + path: knot_toml_path, + }), + } + } + + /// Returns the path to the configuration file. + pub(crate) fn path(&self) -> &SystemPath { + &self.path + } + + pub(crate) fn into_options(self) -> Options { + self.options + } +} + +#[derive(Debug, Error)] +pub enum ConfigurationFileError { + #[error("{path} is not a valid `knot.toml`: {source}")] + InvalidKnotToml { + source: Box, + path: SystemPathBuf, + }, +} diff --git a/crates/red_knot_project/src/watch/project_watcher.rs b/crates/red_knot_project/src/watch/project_watcher.rs index 25b4f6f08d..d9fb2d9610 100644 --- a/crates/red_knot_project/src/watch/project_watcher.rs +++ b/crates/red_knot_project/src/watch/project_watcher.rs @@ -73,6 +73,13 @@ impl ProjectWatcher { .canonicalize_path(&project_path) .unwrap_or(project_path); + let config_paths = db + .project() + .metadata(db) + .extra_configuration_paths() + .iter() + .cloned(); + // Find the non-overlapping module search paths and filter out paths that are already covered by the project. // Module search paths are already canonicalized. let unique_module_paths = ruff_db::system::deduplicate_nested_paths( @@ -83,8 +90,11 @@ impl ProjectWatcher { .map(SystemPath::to_path_buf); // Now add the new paths, first starting with the project path and then - // adding the library search paths. - for path in std::iter::once(project_path).chain(unique_module_paths) { + // adding the library search paths, and finally the paths for configurations. + for path in std::iter::once(project_path) + .chain(unique_module_paths) + .chain(config_paths) + { // Log a warning. It's not worth aborting if registering a single folder fails because // Ruff otherwise stills works as expected. if let Err(error) = self.watcher.watch(&path) { diff --git a/crates/red_knot_python_semantic/Cargo.toml b/crates/red_knot_python_semantic/Cargo.toml index 47847e8307..b12b3637db 100644 --- a/crates/red_knot_python_semantic/Cargo.toml +++ b/crates/red_knot_python_semantic/Cargo.toml @@ -44,7 +44,7 @@ test-case = { workspace = true } memchr = { workspace = true } [dev-dependencies] -ruff_db = { workspace = true, features = ["testing"] } +ruff_db = { workspace = true, features = ["testing", "os"] } ruff_python_parser = { workspace = true } red_knot_test = { workspace = true } red_knot_vendored = { workspace = true } diff --git a/crates/red_knot_server/src/session.rs b/crates/red_knot_server/src/session.rs index 3ad3869cc7..6e370418be 100644 --- a/crates/red_knot_server/src/session.rs +++ b/crates/red_knot_server/src/session.rs @@ -68,7 +68,9 @@ impl Session { let system = LSPSystem::new(index.clone()); // TODO(dhruvmanila): Get the values from the client settings - let metadata = ProjectMetadata::discover(system_path, &system)?; + let mut metadata = ProjectMetadata::discover(system_path, &system)?; + metadata.apply_configuration_files(&system)?; + // TODO(micha): Handle the case where the program settings are incorrect more gracefully. workspaces.insert(path, ProjectDatabase::new(metadata, system)?); } diff --git a/crates/ruff_db/src/system.rs b/crates/ruff_db/src/system.rs index 91bcb03756..14946e8e59 100644 --- a/crates/ruff_db/src/system.rs +++ b/crates/ruff_db/src/system.rs @@ -1,7 +1,12 @@ pub use glob::PatternError; pub use memory_fs::MemoryFileSystem; + +#[cfg(all(feature = "testing", feature = "os"))] +pub use os::testing::UserConfigDirectoryOverrideGuard; + #[cfg(feature = "os")] pub use os::OsSystem; + use ruff_notebook::{Notebook, NotebookError}; use std::error::Error; use std::fmt::Debug; diff --git a/crates/ruff_db/src/system/os.rs b/crates/ruff_db/src/system/os.rs index fea57fea16..d1703f0952 100644 --- a/crates/ruff_db/src/system/os.rs +++ b/crates/ruff_db/src/system/os.rs @@ -24,6 +24,11 @@ pub struct OsSystem { #[derive(Default, Debug)] struct OsSystemInner { cwd: SystemPathBuf, + + /// Overrides the user's configuration directory for testing. + /// This is an `Option>` to allow setting an override of `None`. + #[cfg(feature = "testing")] + user_config_directory_override: std::sync::Mutex>>, } impl OsSystem { @@ -32,8 +37,11 @@ impl OsSystem { assert!(cwd.as_utf8_path().is_absolute()); Self { + // Spreading `..Default` because it isn't possible to feature gate the initializer of a single field. + #[allow(clippy::needless_update)] inner: Arc::new(OsSystemInner { cwd: cwd.to_path_buf(), + ..Default::default() }), } } @@ -100,6 +108,15 @@ impl System for OsSystem { #[cfg(not(target_arch = "wasm32"))] fn user_config_directory(&self) -> Option { + // In testing, we allow overriding the user configuration directory by using a + // thread local because overriding the environment variables breaks test isolation + // (tests run concurrently) and mutating environment variable in a multithreaded + // application is inherently unsafe. + #[cfg(feature = "testing")] + if let Ok(directory_override) = self.try_get_user_config_directory_override() { + return directory_override; + } + use etcetera::BaseStrategy as _; let strategy = etcetera::base_strategy::choose_base_strategy().ok()?; @@ -110,6 +127,11 @@ impl System for OsSystem { // `os` feature enabled (via `ruff_workspace` -> `ruff_graph` -> `ruff_db`). #[cfg(target_arch = "wasm32")] fn user_config_directory(&self) -> Option { + #[cfg(feature = "testing")] + if let Ok(directory_override) = self.try_get_user_config_directory_override() { + return directory_override; + } + None } @@ -336,6 +358,64 @@ fn not_found() -> std::io::Error { std::io::Error::new(std::io::ErrorKind::NotFound, "No such file or directory") } +#[cfg(feature = "testing")] +pub(super) mod testing { + + use crate::system::{OsSystem, SystemPathBuf}; + + impl OsSystem { + /// Overrides the user configuration directory for the current scope + /// (for as long as the returned override is not dropped). + pub fn with_user_config_directory( + &self, + directory: Option, + ) -> UserConfigDirectoryOverrideGuard { + let mut directory_override = self.inner.user_config_directory_override.lock().unwrap(); + let previous = directory_override.replace(directory); + + UserConfigDirectoryOverrideGuard { + previous, + system: self.clone(), + } + } + + /// Returns [`Ok`] if any override is set and [`Err`] otherwise. + pub(super) fn try_get_user_config_directory_override( + &self, + ) -> Result, ()> { + let directory_override = self.inner.user_config_directory_override.lock().unwrap(); + match directory_override.as_ref() { + Some(directory_override) => Ok(directory_override.clone()), + None => Err(()), + } + } + } + + /// A scoped override of the [user's configuration directory](crate::System::user_config_directory) for the [`OsSystem`]. + /// + /// Prefer overriding the user's configuration directory for tests that require + /// spawning a new process (e.g. CLI tests) by setting the `APPDATA` (windows) + /// or `XDG_CONFIG_HOME` (unix and other platforms) environment variables. + /// For example, by setting the environment variables when invoking the CLI with insta. + /// + /// Requires the `testing` feature. + #[must_use] + pub struct UserConfigDirectoryOverrideGuard { + previous: Option>, + system: OsSystem, + } + + impl Drop for UserConfigDirectoryOverrideGuard { + fn drop(&mut self) { + if let Ok(mut directory_override) = + self.system.inner.user_config_directory_override.try_lock() + { + *directory_override = self.previous.take(); + } + } + } +} + #[cfg(test)] mod tests { use tempfile::TempDir;