better error messages while loading configuration extends (#15658)

Co-authored-by: Micha Reiser <[email protected]>

Author: purajit

Cargo.lock

@@ -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
           |

crates/ruff/tests/format.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<Path>) -> 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(())
 }

crates/ruff/tests/lint.rs

@@ -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

crates/ruff_workspace/Cargo.toml

@@ -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<Configuration> {
-    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))