Repository: oli-obk/cargo_metadata (rust)

Issue:
Non numerical values for debuginfo cannot be parsed from cargo messages
Valid values for debuginfo level now also include strings "line-tables-only" and "line-directives-only", see [here](https://doc.rust-lang.org/cargo/reference/profiles.html#debug). 

`cargo` will emit those as part of json output. `profile` for an can look artifact look like this

```json
"profile":{"opt_level":"3","debuginfo":"line-tables-only","debug_assertions":false,"overflow_checks":false,"test":false},
```
I'm trying to parse it into `cargo_metadata::messages::Message` datatype and it fails to parse it, probably because 
[`ArtifactProfile`](https://docs.rs/cargo_metadata/latest/cargo_metadata/struct.ArtifactProfile.html) defines `debuginfo` as `Option<u32>`

https://github.com/pacak/cargo-show-asm/issues/187 - related ticket

Pull request:
Update debuginfo for change in 1.71
There was an unanticipated change in 1.71 that added stringly-typed debuginfo in the profile struct. After discussing with the cargo team, we have decided to keep this change even though it has caused breakage. Fortunately this only affects projects which have opted-in to the string-based variants. 

There isn't a clear alternative for how to represent this information (they can't be accurately mapped to numbers), and we don't have a way to version this JSON output (which would also be a breaking change if we changed the version). We currently don't have a good story for how to manage JSON compatibility of the compiler messages, which is something we should probably figure out at some time.

I decided to go ahead and map the values to their semantic meaning, with an escape hatch for unknown values that might be added in the future. Alternatively it could just be a two-variant enum (int/string), but adding the mapping wasn't too difficult. However, this means that the round-trip serialization isn't exact (null or missing gets mapped to 0). I don't expect that to be an issue, but I'm not sure why anyone would serialize these.

Fixes #240


Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 94a9fd4..80068a9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -102,7 +102,8 @@ pub use errors::{Error, Result};
 #[allow(deprecated)]
 pub use messages::parse_messages;
 pub use messages::{
-    Artifact, ArtifactProfile, BuildFinished, BuildScript, CompilerMessage, Message, MessageIter,
+    Artifact, ArtifactDebuginfo, ArtifactProfile, BuildFinished, BuildScript, CompilerMessage,
+    Message, MessageIter,
 };
 #[cfg(feature = "builder")]
 pub use messages::{
diff --git a/src/messages.rs b/src/messages.rs
index ea2abd2..028cd72 100644
--- a/src/messages.rs
+++ b/src/messages.rs
@@ -2,8 +2,8 @@ use super::{Diagnostic, PackageId, Target};
 use camino::Utf8PathBuf;
 #[cfg(feature = "builder")]
 use derive_builder::Builder;
-use serde::{Deserialize, Serialize};
-use std::fmt;
+use serde::{de, ser, Deserialize, Serialize};
+use std::fmt::{self, Write};
 use std::io::{self, BufRead, Read};
 
 /// Profile settings used to determine which compiler flags to use for a
@@ -15,8 +15,9 @@ use std::io::{self, BufRead, Read};
 pub struct ArtifactProfile {
     /// Optimization level. Possible values are 0-3, s or z.
     pub opt_level: String,
-    /// The amount of debug info. 0 for none, 1 for limited, 2 for full
-    pub debuginfo: Option<u32>,
+    /// The kind of debug information.
+    #[serde(default)]
+    pub debuginfo: ArtifactDebuginfo,
     /// State of the `cfg(debug_assertions)` directive, enabling macros like
     /// `debug_assert!`
     pub debug_assertions: bool,
@@ -26,6 +27,132 @@ pub struct ArtifactProfile {
     pub test: bool,
 }
 
+/// The kind of debug information included in the artifact.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[non_exhaustive]
+pub enum ArtifactDebuginfo {
+    /// No debug information.
+    None,
+    /// Line directives only.
+    LineDirectivesOnly,
+    /// Line tables only.
+    LineTablesOnly,
+    /// Debug information without type or variable-level information.
+    Limited,
+    /// Full debug information.
+    Full,
+    /// An unknown integer level.
+    ///
+    /// This may be produced by a version of rustc in the future that has
+    /// additional levels represented by an integer that are not known by this
+    /// version of `cargo_metadata`.
+    UnknownInt(i64),
+    /// An unknown string level.
+    ///
+    /// This may be produced by a version of rustc in the future that has
+    /// additional levels represented by a string that are not known by this
+    /// version of `cargo_metadata`.
+    UnknownString(String),
+}
+
+impl Default for ArtifactDebuginfo {
+    fn default() -> Self {
+        ArtifactDebuginfo::None
+    }
+}
+
+impl ser::Serialize for ArtifactDebuginfo {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        match self {
+            Self::None => 0.serialize(serializer),
+            Self::LineDirectivesOnly => "line-directives-only".serialize(serializer),
+            Self::LineTablesOnly => "line-tables-only".serialize(serializer),
+            Self::Limited => 1.serialize(serializer),
+            Self::Full => 2.serialize(serializer),
+            Self::UnknownInt(n) => n.serialize(serializer),
+            Self::UnknownString(s) => s.serialize(serializer),
+        }
+    }
+}
+
+impl<'de> de::Deserialize<'de> for ArtifactDebuginfo {
+    fn deserialize<D>(d: D) -> Result<ArtifactDebuginfo, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        struct Visitor;
+
+        impl<'de> de::Visitor<'de> for Visitor {
+            type Value = ArtifactDebuginfo;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+                formatter.write_str("an integer or string")
+            }
+
+            fn visit_i64<E>(self, value: i64) -> Result<ArtifactDebuginfo, E>
+            where
+                E: de::Error,
+            {
+                let debuginfo = match value {
+                    0 => ArtifactDebuginfo::None,
+                    1 => ArtifactDebuginfo::Limited,
+                    2 => ArtifactDebuginfo::Full,
+                    n => ArtifactDebuginfo::UnknownInt(n),
+                };
+                Ok(debuginfo)
+            }
+
+            fn visit_u64<E>(self, value: u64) -> Result<ArtifactDebuginfo, E>
+            where
+                E: de::Error,
+            {
+                self.visit_i64(value as i64)
+            }
+
+            fn visit_str<E>(self, value: &str) -> Result<ArtifactDebuginfo, E>
+            where
+                E: de::Error,
+            {
+                let debuginfo = match value {
+                    "none" => ArtifactDebuginfo::None,
+                    "limited" => ArtifactDebuginfo::Limited,
+                    "full" => ArtifactDebuginfo::Full,
+                    "line-directives-only" => ArtifactDebuginfo::LineDirectivesOnly,
+                    "line-tables-only" => ArtifactDebuginfo::LineTablesOnly,
+                    s => ArtifactDebuginfo::UnknownString(s.to_string()),
+                };
+                Ok(debuginfo)
+            }
+
+            fn visit_unit<E>(self) -> Result<ArtifactDebuginfo, E>
+            where
+                E: de::Error,
+            {
+                Ok(ArtifactDebuginfo::None)
+            }
+        }
+
+        d.deserialize_any(Visitor)
+    }
+}
+
+impl fmt::Display for ArtifactDebuginfo {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ArtifactDebuginfo::None => f.write_char('0'),
+            ArtifactDebuginfo::Limited => f.write_char('1'),
+            ArtifactDebuginfo::Full => f.write_char('2'),
+            ArtifactDebuginfo::LineDirectivesOnly => f.write_str("line-directives-only"),
+            ArtifactDebuginfo::LineTablesOnly => f.write_str("line-tables-only"),
+            ArtifactDebuginfo::UnknownInt(n) => write!(f, "{}", n),
+            ArtifactDebuginfo::UnknownString(s) => f.write_str(s),
+        }
+    }
+}
+
 /// A compiler-generated file.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "builder", derive(Builder))]


Test patch:
diff --git a/tests/test_samples.rs b/tests/test_samples.rs
index 3c747c5..91d06d6 100644
--- a/tests/test_samples.rs
+++ b/tests/test_samples.rs
@@ -4,7 +4,9 @@ extern crate semver;
 extern crate serde_json;
 
 use camino::Utf8PathBuf;
-use cargo_metadata::{CargoOpt, DependencyKind, Edition, Metadata, MetadataCommand};
+use cargo_metadata::{
+    ArtifactDebuginfo, CargoOpt, DependencyKind, Edition, Message, Metadata, MetadataCommand,
+};
 
 #[test]
 fn old_minimal() {
@@ -645,3 +647,46 @@ fn basic_workspace_root_package_exists() {
         "ex_bin"
     );
 }
+
+#[test]
+fn debuginfo_variants() {
+    // Checks behavior for the different debuginfo variants.
+    let variants = [
+        ("0", ArtifactDebuginfo::None),
+        ("1", ArtifactDebuginfo::Limited),
+        ("2", ArtifactDebuginfo::Full),
+        (
+            "\"line-directives-only\"",
+            ArtifactDebuginfo::LineDirectivesOnly,
+        ),
+        ("\"line-tables-only\"", ArtifactDebuginfo::LineTablesOnly),
+        ("3", ArtifactDebuginfo::UnknownInt(3)),
+        (
+            "\"abc\"",
+            ArtifactDebuginfo::UnknownString("abc".to_string()),
+        ),
+        ("null", ArtifactDebuginfo::None),
+    ];
+    for (value, expected) in variants {
+        let s = r#"{"reason":"compiler-artifact","package_id":"cargo_metadata 0.16.0 (path+file:////cargo_metadata)","manifest_path":"/cargo_metadata/Cargo.toml","target":{"kind":["lib"],"crate_types":["lib"],"name":"cargo_metadata","src_path":"/cargo_metadata/src/lib.rs","edition":"2018","doc":true,"doctest":true,"test":true},"profile":{"opt_level":"0","debuginfo":DEBUGINFO,"debug_assertions":true,"overflow_checks":true,"test":false},"features":["default"],"filenames":["/cargo_metadata/target/debug/deps/libcargo_metadata-27f582f7187b9a2c.rmeta"],"executable":null,"fresh":false}"#;
+        let message: Message = serde_json::from_str(&s.replace("DEBUGINFO", value)).unwrap();
+        match message {
+            Message::CompilerArtifact(artifact) => {
+                assert_eq!(artifact.profile.debuginfo, expected);
+                let de_s = serde_json::to_string(&artifact.profile.debuginfo).unwrap();
+                // Note: Roundtrip does not retain null value.
+                if value == "null" {
+                    assert_eq!(artifact.profile.debuginfo.to_string(), "0");
+                    assert_eq!(de_s, "0");
+                } else {
+                    assert_eq!(
+                        artifact.profile.debuginfo.to_string(),
+                        value.trim_matches('"')
+                    );
+                    assert_eq!(de_s, value);
+                }
+            }
+            _ => panic!("unexpected {:?}", message),
+        }
+    }
+}