From 2d8af43d873b4103a9ee5450af419f7ec33930c1 Mon Sep 17 00:00:00 2001
From: Martin Geisler <mgeisler@google.com>
Date: Wed, 6 Apr 2022 12:12:41 +0200
Subject: [PATCH] Check for `[workspace]` when finding workspace root

Before, the top-most `Cargo.toml` file was used. Now we use the
nearest `Cargo.toml` file which contain a line saying `[workspace]`.

This should avoid false positives when Cargo workspaces are nested
inside each other: in this case, the inner package will have an empty
`[workspace]` table.
---
 Cargo.toml                 |  3 +++
 src/lib.rs                 | 23 ++++++++++++++---------
 tests/nested-workspaces.rs | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 9 deletions(-)
 create mode 100644 tests/nested-workspaces.rs

diff --git a/Cargo.toml b/Cargo.toml
index 2c852c9..a4ed83b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -17,3 +17,6 @@ members = ["xtask"]
 [dependencies]
 once_cell = "1"
 dissimilar = "1"
+
+[dev-dependencies]
+tempfile = "3"
diff --git a/src/lib.rs b/src/lib.rs
index 1e93a3b..e25ed8c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -644,17 +644,22 @@ fn to_abs_ws_path(path: &Path) -> PathBuf {
     static WORKSPACE_ROOT: OnceCell<PathBuf> = OnceCell::new();
     WORKSPACE_ROOT
         .get_or_try_init(|| {
-            let my_manifest = env::var("CARGO_MANIFEST_DIR")?;
+            let manifest_dir = env::var("CARGO_MANIFEST_DIR")?;
 
             // Heuristic, see https://github.com/rust-lang/cargo/issues/3946
-            let workspace_root = Path::new(&my_manifest)
-                .ancestors()
-                .filter(|it| it.join("Cargo.toml").exists())
-                .last()
-                .unwrap()
-                .to_path_buf();
-
-            Ok(workspace_root)
+            let workspace_root = Path::new(&manifest_dir).ancestors().find(|it| {
+                match fs::read_to_string(it.join("Cargo.toml")) {
+                    Ok(cargo_toml) => cargo_toml.lines().any(|line| line.trim() == "[workspace]"),
+                    Err(_) => false, // no Cargo.toml
+                }
+            });
+
+            // Check if we found a workspace or if we should use
+            // manifest_dir.
+            match workspace_root {
+                Some(workspace_root) => Ok(workspace_root.to_path_buf()),
+                None => Ok(PathBuf::from(manifest_dir)),
+            }
         })
         .unwrap_or_else(|_: env::VarError| {
             panic!("No CARGO_MANIFEST_DIR env var and the path is relative: {}", path.display())
diff --git a/tests/nested-workspaces.rs b/tests/nested-workspaces.rs
new file mode 100644
index 0000000..24c7e86
--- /dev/null
+++ b/tests/nested-workspaces.rs
@@ -0,0 +1,36 @@
+// Because this test modifies CARGO_MANIFEST_DIR, it has to be run as
+// an integration test. This way we can isolate the modification to
+// just one process.
+
+use std::env;
+use std::fs;
+use std::path::Path;
+
+use expect_test::ExpectFile;
+
+#[test]
+fn test_nested_workspaces() -> Result<(), std::io::Error> {
+    let tmpdir = tempfile::tempdir()?;
+    let outer_toml = tmpdir.path().join("Cargo.toml");
+    let nested = tmpdir.path().join("nested");
+    let nested_src = nested.join("src");
+    let inner_toml = nested.join("Cargo.toml");
+
+    fs::write(&outer_toml, r#"[package]\nname = "foo""#)?;
+
+    fs::create_dir(&nested)?;
+    fs::create_dir(&nested_src)?;
+    fs::write(&inner_toml, r#"[package]\nname = "bar"\n[workspace]\n"#)?;
+
+    // Fabricate a ExpectFile as if had been created with expect_file!
+    // inside the nested project.
+    env::set_var("CARGO_MANIFEST_DIR", nested.as_os_str());
+    let expect_file = ExpectFile { path: Path::new("bar.txt").to_owned(), position: "src/lib.rs" };
+    // Populate nested/src/bar.txt.
+    env::set_var("UPDATE_EXPECT", "1");
+    expect_file.assert_eq("Expected content");
+
+    assert_eq!(fs::read_to_string(nested_src.join("bar.txt"))?, "Expected content");
+
+    Ok(())
+}