Repository: oli-obk/cargo_metadata (rust)

Issue:
Don't panic from WorkspaceDefaultMembers?
Hi, https://docs.rs/cargo_metadata/0.18.1/cargo_metadata/struct.WorkspaceDefaultMembers.html documents that it will panic on cargo <1.70, and it does.

For cargo-mutants, I'd like to use this but also support older versions of cargo, so it would be really nice if this was a runtime error rather than a panic. Or at least, if there was a documented version field that I could check before looking at it. Possibly I am missing some subtlety about why that's not feasible.

Thanks for a really helpful crate.

Pull request description:
Add method to check whether WorkspaceDefaultMembers has a value
This method checks whether the called `cargo-metadata` supports reading the workspace default members, which is false for versions of Cargo prior to 1.71

Alternatively, I could:
- add an Option variant method like `workspace_default_packages` directly on `Metadata`: `pub fn workspace_default_packages(&self) -> Option<Vec<&Package>>`, or
- reverse the name from `is_available` to `is_missing` (which would also allow the removal of the `doc(hidden)` function `workspace_default_members_is_missing`).


fixes #254



Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 5a51e83..2b7e94c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -157,8 +157,11 @@ pub struct Metadata {
     pub workspace_members: Vec<PackageId>,
     /// The list of default workspace members
     ///
-    /// This not available if running with a version of Cargo older than 1.71.
-    #[serde(default, skip_serializing_if = "workspace_default_members_is_missing")]
+    /// This is not available if running with a version of Cargo older than 1.71.
+    ///
+    /// You can check whether it is available or missing using respectively
+    /// [`WorkspaceDefaultMembers::is_available`] and [`WorkspaceDefaultMembers::is_missing`].
+    #[serde(default, skip_serializing_if = "WorkspaceDefaultMembers::is_missing")]
     pub workspace_default_members: WorkspaceDefaultMembers,
     /// Dependencies graph
     pub resolve: Option<Resolve>,
@@ -237,6 +240,32 @@ impl<'a> std::ops::Index<&'a PackageId> for Metadata {
 /// Dereferencing when running an older version of Cargo will panic.
 pub struct WorkspaceDefaultMembers(Option<Vec<PackageId>>);
 
+impl WorkspaceDefaultMembers {
+    /// Return `true` if the list of workspace default members is supported by
+    /// the called cargo-metadata version and `false` otherwise.
+    ///
+    /// In particular useful when parsing the output of `cargo-metadata` for
+    /// versions of Cargo < 1.71, as dereferencing [`WorkspaceDefaultMembers`]
+    /// for these versions will panic.
+    ///
+    /// Opposite of [`WorkspaceDefaultMembers::is_missing`].
+    pub fn is_available(&self) -> bool {
+        self.0.is_some()
+    }
+
+    /// Return `false` if the list of workspace default members is supported by
+    /// the called cargo-metadata version and `true` otherwise.
+    ///
+    /// In particular useful when parsing the output of `cargo-metadata` for
+    /// versions of Cargo < 1.71, as dereferencing [`WorkspaceDefaultMembers`]
+    /// for these versions will panic.
+    ///
+    /// Opposite of [`WorkspaceDefaultMembers::is_available`].
+    pub fn is_missing(&self) -> bool {
+        self.0.is_none()
+    }
+}
+
 impl core::ops::Deref for WorkspaceDefaultMembers {
     type Target = [PackageId];
 
@@ -247,18 +276,6 @@ impl core::ops::Deref for WorkspaceDefaultMembers {
     }
 }
 
-/// Return true if a valid value for [`WorkspaceDefaultMembers`] is missing, and
-/// dereferencing it would panic.
-///
-/// Internal helper for `skip_serializing_if` and test code. Might be removed in
-/// the future.
-#[doc(hidden)]
-pub fn workspace_default_members_is_missing(
-    workspace_default_members: &WorkspaceDefaultMembers,
-) -> bool {
-    workspace_default_members.0.is_none()
-}
-
 #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "builder", derive(Builder))]
 #[non_exhaustive]


Test patch:
diff --git a/tests/selftest.rs b/tests/selftest.rs
index e6dadf7..5270544 100644
--- a/tests/selftest.rs
+++ b/tests/selftest.rs
@@ -164,8 +164,6 @@ fn metadata_deps() {
 
 #[test]
 fn workspace_default_packages() {
-    use cargo_metadata::workspace_default_members_is_missing;
-
     let metadata = MetadataCommand::new()
         .manifest_path("Cargo.toml")
         .exec()
@@ -173,7 +171,7 @@ fn workspace_default_packages() {
     let workspace_packages = metadata.workspace_packages();
     // this will only trigger on cargo versions that expose
     // workspace_default_members (that is, cargo >= 1.71)
-    if !workspace_default_members_is_missing(&metadata.workspace_default_members) {
+    if metadata.workspace_default_members.is_available() {
         let default_packages = metadata.workspace_default_packages();
         assert_eq!(default_packages, workspace_packages);
     }
diff --git a/tests/test_samples.rs b/tests/test_samples.rs
index 2f406c5..3596693 100644
--- a/tests/test_samples.rs
+++ b/tests/test_samples.rs
@@ -5,8 +5,7 @@ extern crate serde_json;
 
 use camino::Utf8PathBuf;
 use cargo_metadata::{
-    workspace_default_members_is_missing, ArtifactDebuginfo, CargoOpt, DependencyKind, Edition,
-    Message, Metadata, MetadataCommand,
+    ArtifactDebuginfo, CargoOpt, DependencyKind, Edition, Message, Metadata, MetadataCommand,
 };
 
 /// Output from oldest version ever supported (1.24).
@@ -125,9 +124,9 @@ fn old_minimal() {
     assert_eq!(meta.workspace_metadata, serde_json::Value::Null);
     assert_eq!(meta.target_directory, "/foo/target");
 
-    assert!(workspace_default_members_is_missing(
-        &meta.workspace_default_members
-    ));
+    assert!(!meta.workspace_default_members.is_available());
+    assert!(meta.workspace_default_members.is_missing());
+
     let serialized = serde_json::to_value(meta).unwrap();
     assert!(!serialized
         .as_object()
@@ -724,6 +723,159 @@ fn missing_workspace_default_members() {
     let _ = &*meta.workspace_default_members;
 }
 
+#[test]
+fn workspace_default_members_is_available() {
+    // generated with cargo +1.71.0 metadata --format-version 1
+    let json = r#"
+{
+  "packages": [
+    {
+      "name": "basic",
+      "version": "0.1.0",
+      "id": "basic 0.1.0 (path+file:///example)",
+      "license": null,
+      "license_file": null,
+      "description": null,
+      "source": null,
+      "dependencies": [],
+      "targets": [
+        {
+          "kind": [
+            "lib"
+          ],
+          "crate_types": [
+            "lib"
+          ],
+          "name": "basic",
+          "src_path": "/example/src/lib.rs",
+          "edition": "2021",
+          "doc": true,
+          "doctest": true,
+          "test": true
+        }
+      ],
+      "features": {},
+      "manifest_path": "/example/Cargo.toml",
+      "metadata": null,
+      "publish": null,
+      "authors": [],
+      "categories": [],
+      "keywords": [],
+      "readme": null,
+      "repository": null,
+      "homepage": null,
+      "documentation": null,
+      "edition": "2021",
+      "links": null,
+      "default_run": null,
+      "rust_version": null
+    }
+  ],
+  "workspace_members": [
+    "basic 0.1.0 (path+file:///example)"
+  ],
+  "workspace_default_members": [
+    "basic 0.1.0 (path+file:///example)"
+  ],
+  "resolve": {
+    "nodes": [
+      {
+        "id": "basic 0.1.0 (path+file:///example)",
+        "dependencies": [],
+        "deps": [],
+        "features": []
+      }
+    ],
+    "root": "basic 0.1.0 (path+file:///example)"
+  },
+  "target_directory": "/example/target",
+  "version": 1,
+  "workspace_root": "/example",
+  "metadata": null
+}
+"#;
+
+    let meta: Metadata = serde_json::from_str(json).unwrap();
+
+    assert!(meta.workspace_default_members.is_available());
+    assert!(!meta.workspace_default_members.is_missing());
+}
+
+#[test]
+fn workspace_default_members_is_missing() {
+    // generated with cargo +1.70.0 metadata --format-version 1
+    let json = r#"
+{
+  "packages": [
+    {
+      "name": "basic",
+      "version": "0.1.0",
+      "id": "basic 0.1.0 (path+file:///example)",
+      "license": null,
+      "license_file": null,
+      "description": null,
+      "source": null,
+      "dependencies": [],
+      "targets": [
+        {
+          "kind": [
+            "lib"
+          ],
+          "crate_types": [
+            "lib"
+          ],
+          "name": "basic",
+          "src_path": "/example/src/lib.rs",
+          "edition": "2021",
+          "doc": true,
+          "doctest": true,
+          "test": true
+        }
+      ],
+      "features": {},
+      "manifest_path": "/example/Cargo.toml",
+      "metadata": null,
+      "publish": null,
+      "authors": [],
+      "categories": [],
+      "keywords": [],
+      "readme": null,
+      "repository": null,
+      "homepage": null,
+      "documentation": null,
+      "edition": "2021",
+      "links": null,
+      "default_run": null,
+      "rust_version": null
+    }
+  ],
+  "workspace_members": [
+    "basic 0.1.0 (path+file:///example)"
+  ],
+  "resolve": {
+    "nodes": [
+      {
+        "id": "basic 0.1.0 (path+file:///example)",
+        "dependencies": [],
+        "deps": [],
+        "features": []
+      }
+    ],
+    "root": "basic 0.1.0 (path+file:///example)"
+  },
+  "target_directory": "/example/target",
+  "version": 1,
+  "workspace_root": "/example",
+  "metadata": null
+}
+"#;
+
+    let meta: Metadata = serde_json::from_str(json).unwrap();
+
+    assert!(!meta.workspace_default_members.is_available());
+    assert!(meta.workspace_default_members.is_missing());
+}
+
 #[test]
 fn test_unknown_target_kind_and_crate_type() {
     // Both kind and crate_type set to a type not yet known