From 9304fdf4ecacfcd9f8bf76c850c276bfabcfaf66 Mon Sep 17 00:00:00 2001 From: purajit <7026198+purajit@users.noreply.github.com> Date: Mon, 17 Feb 2025 01:35:30 -0800 Subject: [PATCH] better error messages while loading configuration `extend`s (#15658) Co-authored-by: Micha Reiser --- Cargo.lock | 1 + crates/ruff/tests/format.rs | 2 + crates/ruff/tests/lint.rs | 141 +++++++++++++++++++++++++- crates/ruff_workspace/Cargo.toml | 10 +- crates/ruff_workspace/src/resolver.rs | 44 ++++++-- 5 files changed, 182 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9fbb5cceddd2a3..26895f7982a119 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3237,6 +3237,7 @@ dependencies = [ "glob", "globset", "ignore", + "indexmap", "is-macro", "itertools 0.14.0", "log", diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 43428c7327c627..a24b4e313f5cc5 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -816,6 +816,7 @@ if True: ----- stderr ----- ruff failed + Cause: Failed to load configuration `[RUFF-TOML-PATH]` Cause: Failed to parse [RUFF-TOML-PATH] Cause: TOML parse error at line 1, column 1 | @@ -855,6 +856,7 @@ format = "json" ----- stderr ----- ruff failed + Cause: Failed to load configuration `[RUFF-TOML-PATH]` Cause: Failed to parse [RUFF-TOML-PATH] Cause: TOML parse error at line 2, column 10 | diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index c13ba49136bea1..a082ab7cbb6c0c 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -15,8 +15,8 @@ use tempfile::TempDir; const BIN_NAME: &str = "ruff"; const STDIN_BASE_OPTIONS: &[&str] = &["check", "--no-cache", "--output-format", "concise"]; -fn tempdir_filter(tempdir: &TempDir) -> String { - format!(r"{}\\?/?", escape(tempdir.path().to_str().unwrap())) +fn tempdir_filter(path: impl AsRef) -> String { + format!(r"{}\\?/?", escape(path.as_ref().to_str().unwrap())) } #[test] @@ -609,6 +609,139 @@ fn extend_passed_via_config_argument() { "); } +#[test] +fn nonexistent_extend_file() -> Result<()> { + let tempdir = TempDir::new()?; + let project_dir = tempdir.path().canonicalize()?; + fs::write( + project_dir.join("ruff.toml"), + r#" +extend = "ruff2.toml" +"#, + )?; + + fs::write( + project_dir.join("ruff2.toml"), + r#" +extend = "ruff3.toml" +"#, + )?; + + insta::with_settings!({ + filters => vec![ + (tempdir_filter(&project_dir).as_str(), "[TMP]/"), + ("The system cannot find the file specified.", "No such file or directory") + ] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(["check"]).current_dir(project_dir), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to load extended configuration `[TMP]/ruff3.toml` (`[TMP]/ruff.toml` extends `[TMP]/ruff2.toml` extends `[TMP]/ruff3.toml`) + Cause: Failed to read [TMP]/ruff3.toml + Cause: No such file or directory (os error 2) + "); + }); + + Ok(()) +} + +#[test] +fn circular_extend() -> Result<()> { + let tempdir = TempDir::new()?; + let project_path = tempdir.path().canonicalize()?; + + fs::write( + project_path.join("ruff.toml"), + r#" +extend = "ruff2.toml" +"#, + )?; + fs::write( + project_path.join("ruff2.toml"), + r#" +extend = "ruff3.toml" +"#, + )?; + fs::write( + project_path.join("ruff3.toml"), + r#" +extend = "ruff.toml" +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&project_path).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)) + .args(["check"]) + .current_dir(project_path), + @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Circular configuration detected: `[TMP]/ruff.toml` extends `[TMP]/ruff2.toml` extends `[TMP]/ruff3.toml` extends `[TMP]/ruff.toml` + "); + }); + + Ok(()) +} + +#[test] +fn parse_error_extends() -> Result<()> { + let tempdir = TempDir::new()?; + let project_path = tempdir.path().canonicalize()?; + + fs::write( + project_path.join("ruff.toml"), + r#" +extend = "ruff2.toml" +"#, + )?; + fs::write( + project_path.join("ruff2.toml"), + r#" +[lint] +select = [E501] +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&project_path).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!( + Command::new(get_cargo_bin(BIN_NAME)) + .args(["check"]) + .current_dir(project_path), + @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to load extended configuration `[TMP]/ruff2.toml` (`[TMP]/ruff.toml` extends `[TMP]/ruff2.toml`) + Cause: Failed to parse [TMP]/ruff2.toml + Cause: TOML parse error at line 3, column 11 + | + 3 | select = [E501] + | ^ + invalid array + expected `]` + "); + }); + + Ok(()) +} + #[test] fn config_file_and_isolated() -> Result<()> { let tempdir = TempDir::new()?; @@ -2095,6 +2228,7 @@ fn flake8_import_convention_invalid_aliases_config_alias_name() -> Result<()> { ----- stderr ----- ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` Cause: Failed to parse [TMP]/ruff.toml Cause: TOML parse error at line 3, column 17 | @@ -2131,6 +2265,7 @@ fn flake8_import_convention_invalid_aliases_config_extend_alias_name() -> Result ----- stderr ----- ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` Cause: Failed to parse [TMP]/ruff.toml Cause: TOML parse error at line 3, column 17 | @@ -2167,6 +2302,7 @@ fn flake8_import_convention_invalid_aliases_config_module_name() -> Result<()> { ----- stderr ----- ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` Cause: Failed to parse [TMP]/ruff.toml Cause: TOML parse error at line 3, column 1 | @@ -2429,6 +2565,5 @@ fn a005_module_shadowing_strict_default() -> Result<()> { ----- stderr ----- "); }); - Ok(()) } diff --git a/crates/ruff_workspace/Cargo.toml b/crates/ruff_workspace/Cargo.toml index d7d247752b926f..5a185dc755c84b 100644 --- a/crates/ruff_workspace/Cargo.toml +++ b/crates/ruff_workspace/Cargo.toml @@ -21,12 +21,13 @@ ruff_macros = { workspace = true } ruff_python_ast = { workspace = true } ruff_python_formatter = { workspace = true, features = ["serde"] } ruff_python_semantic = { workspace = true, features = ["serde"] } -ruff_python_stdlib = {workspace = true} +ruff_python_stdlib = { workspace = true } ruff_source_file = { workspace = true } anyhow = { workspace = true } colored = { workspace = true } ignore = { workspace = true } +indexmap = { workspace = true } is-macro = { workspace = true } itertools = { workspace = true } log = { workspace = true } @@ -58,7 +59,12 @@ ignored = ["colored"] [features] default = [] -schemars = ["dep:schemars", "ruff_formatter/schemars", "ruff_python_formatter/schemars", "ruff_python_semantic/schemars"] +schemars = [ + "dep:schemars", + "ruff_formatter/schemars", + "ruff_python_formatter/schemars", + "ruff_python_semantic/schemars", +] [lints] workspace = true diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index 65a354c49dc7ef..f550f810f6ac17 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -7,8 +7,8 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::sync::RwLock; -use anyhow::Result; use anyhow::{anyhow, bail}; +use anyhow::{Context, Result}; use globset::{Candidate, GlobSet}; use ignore::{DirEntry, Error, ParallelVisitor, WalkBuilder, WalkState}; use itertools::Itertools; @@ -304,16 +304,39 @@ pub fn resolve_configuration( relativity: Relativity, transformer: &dyn ConfigurationTransformer, ) -> Result { - let mut seen = FxHashSet::default(); - let mut stack = vec![]; + let mut configurations = indexmap::IndexMap::new(); let mut next = Some(fs::normalize_path(pyproject)); while let Some(path) = next { - if seen.contains(&path) { - bail!("Circular dependency detected in pyproject.toml"); + if configurations.contains_key(&path) { + bail!(format!( + "Circular configuration detected: {chain}", + chain = configurations + .keys() + .chain([&path]) + .map(|p| format!("`{}`", p.display())) + .join(" extends "), + )); } // Resolve the current path. - let options = pyproject::load_options(&path)?; + let options = pyproject::load_options(&path).with_context(|| { + if configurations.is_empty() { + format!( + "Failed to load configuration `{path}`", + path = path.display() + ) + } else { + let chain = configurations + .keys() + .chain([&path]) + .map(|p| format!("`{}`", p.display())) + .join(" extends "); + format!( + "Failed to load extended configuration `{path}` ({chain})", + path = path.display() + ) + } + })?; let project_root = relativity.resolve(&path); let configuration = Configuration::from_options(options, Some(&path), project_root)?; @@ -329,14 +352,13 @@ pub fn resolve_configuration( // Keep track of (1) the paths we've already resolved (to avoid cycles), and (2) // the base configuration for every path. - seen.insert(path); - stack.push(configuration); + configurations.insert(path, configuration); } // Merge the configurations, in order. - stack.reverse(); - let mut configuration = stack.pop().unwrap(); - while let Some(extend) = stack.pop() { + let mut configurations = configurations.into_values(); + let mut configuration = configurations.next().unwrap(); + for extend in configurations { configuration = configuration.combine(extend); } Ok(transformer.transform(configuration))