Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResultKind and ResultLevel appear to serialize incorrectly #738

Open
woodruffw opened this issue Oct 29, 2024 · 3 comments · May be fixed by #770
Open

ResultKind and ResultLevel appear to serialize incorrectly #738

woodruffw opened this issue Oct 29, 2024 · 3 comments · May be fixed by #770

Comments

@woodruffw
Copy link
Contributor

Hi there! First of all, thanks for this library -- it's been fantastic to use.

I'm trying to emit some well-formed SARIF in zizmor and I'm running into some weirdness with how ResultLevel and ResultKind are serialized. More precisely, they seem to always be serialized as null (i.e. serde_json::value::Value::Null), regardless of their actual value.

Here's a minimal failing reproducer:

#[cfg(test)]
mod tests {
    use serde_sarif::sarif::ResultKind;

    #[test]
    fn test_resultkind_serializes() {
        assert_eq!(
            serde_json::to_string(&ResultKind::Fail).unwrap(),
            "\"fail\""
        );
    }
}

The expected behavior is that this serializes to the JSON string "fail", but instead it serializes as null:

assertion `left == right` failed
  left: "null"
 right: "\"fail\""
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I took a quick look at the code, and it doesn't look wrong, but I'm not super familiar with strum (which seems to be controlling the serialization equivalents for each enum variant).

Let me know if there's any other information I can provide! I'll also keep debugging this on my end.

@woodruffw
Copy link
Contributor Author

I think I've root-caused this: these enums are defined with #[serde(untagged)], which is known to cause "bare" variants to serialize to null per serde-rs/serde#1560.

Instead of using untagged, these enums should be using #[serde(rename_all = "camelCase")]. I'll send a PR to make this change.

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 18, 2024

A follow-up problem here is that Result::level and Result::kind are both Option<Value> instead of Option<ResultLevel> and Option<ResultKind>, respectively. This results in API consumers needing to do a clunky round-trip through serde_json::Value.

https://docs.rs/serde-sarif/latest/serde_sarif/sarif/struct.Result.html#structfield.kind

@woodruffw woodruffw linked a pull request Dec 19, 2024 that will close this issue
@woodruffw
Copy link
Contributor Author

Opened #770 for the last piece above!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant