KCL None needs more specific serialization (#3720)

Previously, many KCL values could be deserialized as KCL None values.
This isn't good -- the only thing you should be able to possibly
deserialize as KCL None is KCL None.

Solution was to add a private field in KCL None, serialize it with a
special magic number value, and then check for that magic number when
deserializing
This commit is contained in:
Adam Chalmers
2024-08-29 13:41:32 -05:00
committed by GitHub
parent bbdca7421e
commit 36a6b8c0ea
2 changed files with 63 additions and 4 deletions

View File

@ -9,6 +9,8 @@ use crate::{
executor::{KclValue, SourceRange, UserVal},
};
const KCL_NONE_ID: &str = "KCL_NONE_ID";
/// KCL value for an optional parameter which was not given an argument.
/// (remember, parameters are in the function declaration,
/// arguments are in the function call/application).
@ -20,6 +22,45 @@ pub struct KclNone {
// TODO: Convert this to be an Option<SourceRange>.
pub start: usize,
pub end: usize,
#[serde(deserialize_with = "deser_private")]
#[ts(skip)]
#[schemars(skip)]
__private: Private,
}
impl KclNone {
pub fn new(start: usize, end: usize) -> Self {
Self {
start,
end,
__private: Private {},
}
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Bake, Default)]
#[databake(path = kcl_lib::ast::types)]
struct Private;
impl Serialize for Private {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(KCL_NONE_ID)
}
}
fn deser_private<'de, D>(deserializer: D) -> Result<Private, D::Error>
where
D: serde::Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
if s == KCL_NONE_ID {
Ok(Private {})
} else {
Err(serde::de::Error::custom("not a KCL none"))
}
}
impl From<&KclNone> for SourceRange {
@ -57,3 +98,24 @@ impl KclNone {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn other_types_will_not_deserialize() {
// This shouldn't deserialize into a KCL None,
// because it's missing the special Private tag.
let j = r#"{"start": 0, "end": 0}"#;
let _e = serde_json::from_str::<KclNone>(j).unwrap_err();
}
#[test]
fn serialize_then_deserialize() {
// Serializing, then deserializing a None should produce the same value.
let before = KclNone::default();
let j = serde_json::to_string_pretty(&before).unwrap();
let after: KclNone = serde_json::from_str(&j).unwrap();
assert_eq!(before, after);
}
}

View File

@ -2080,10 +2080,7 @@ fn assign_args_to_params(
if param.optional {
// If the corresponding parameter is optional,
// then it's fine, the user doesn't need to supply it.
let none = KclNone {
start: param.identifier.start,
end: param.identifier.end,
};
let none = KclNone::new(param.identifier.start, param.identifier.end);
fn_memory.add(
&param.identifier.name,
KclValue::from(&none),