KCL literals are typed, not JSON values (#971)

We now control the KCL type system instead of reusing JSON's type system.

src/wasm-lib/kcl/src/ast/types.rs

@@ -7,14 +7,18 @@ use parse_display::{Display, FromStr};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_json::Map;
+use serde_json::Value as JValue;
 use tower_lsp::lsp_types::{CompletionItem, CompletionItemKind, DocumentSymbol, Range as LspRange, SymbolKind};
 
+pub use self::literal_value::LiteralValue;
 use crate::{
     errors::{KclError, KclErrorDetails},
     executor::{ExecutorContext, MemoryItem, Metadata, PipeInfo, ProgramMemory, SourceRange, UserVal},
     parser::PIPE_OPERATOR,
 };
 
+mod literal_value;
+
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
 #[ts(export)]
 #[serde(rename_all = "camelCase")]
@@ -1312,24 +1316,18 @@ impl VariableDeclarator {
 pub struct Literal {
     pub start: usize,
     pub end: usize,
-    pub value: serde_json::Value,
+    pub value: LiteralValue,
     pub raw: String,
 }
 
 impl_value_meta!(Literal);
 
-impl From<Literal> for Value {
-    fn from(literal: Literal) -> Self {
-        Value::Literal(Box::new(literal))
-    }
-}
-
 impl Literal {
-    pub fn new(value: serde_json::Value) -> Self {
+    pub fn new(value: LiteralValue) -> Self {
         Self {
             start: 0,
             end: 0,
-            raw: value.to_string(),
+            raw: JValue::from(value.clone()).to_string(),
             value,
         }
     }
@@ -1343,11 +1341,19 @@ impl Literal {
     }
 
     fn recast(&self) -> String {
-        if let serde_json::Value::String(value) = &self.value {
+        match self.value {
-            let quote = if self.raw.trim().starts_with('"') { '"' } else { '\'' };
+            LiteralValue::Fractional(x) => {
-            format!("{}{}{}", quote, value, quote)
+                if x.fract() == 0.0 {
+                    format!("{x:?}")
                 } else {
-            self.value.to_string()
+                    self.raw.clone()
+                }
+            }
+            LiteralValue::IInteger(_) => self.raw.clone(),
+            LiteralValue::String(ref s) => {
+                let quote = if self.raw.trim().starts_with('"') { '"' } else { '\'' };
+                format!("{quote}{s}{quote}")
+            }
         }
     }
 }
@@ -1355,7 +1361,7 @@ impl Literal {
 impl From<Literal> for MemoryItem {
     fn from(literal: Literal) -> Self {
         MemoryItem::UserVal(UserVal {
-            value: literal.value.clone(),
+            value: JValue::from(literal.value.clone()),
             meta: vec![Metadata {
                 source_range: literal.into(),
             }],
@@ -1366,7 +1372,7 @@ impl From<Literal> for MemoryItem {
 impl From<&Box<Literal>> for MemoryItem {
     fn from(literal: &Box<Literal>) -> Self {
         MemoryItem::UserVal(UserVal {
-            value: literal.value.clone(),
+            value: JValue::from(literal.value.clone()),
             meta: vec![Metadata {
                 source_range: literal.into(),
             }],
@@ -1967,17 +1973,21 @@ impl MemberExpression {
             LiteralIdentifier::Identifier(identifier) => identifier.name.to_string(),
             LiteralIdentifier::Literal(literal) => {
                 let value = literal.value.clone();
-                // Parse this as a string.
+                match value {
-                if let serde_json::Value::String(string) = value {
+                    LiteralValue::IInteger(x) if x >= 0 => return self.get_result_array(memory, x as usize),
-                    string
+                    LiteralValue::IInteger(x) => {
-                } else if let serde_json::Value::Number(_) = &value {
+                        return Err(KclError::Syntax(KclErrorDetails {
-                    // It can also be a number if we are getting a member of an array.
+                            source_ranges: vec![self.into()],
-                    return self.get_result_array(memory, parse_json_number_as_usize(&value, self.into())?);
+                            message: format!("invalid index: {x}"),
-                } else {
+                        }))
-                    return Err(KclError::Semantic(KclErrorDetails {
+                    }
-                        message: format!("Expected string literal or number for property name, found {:?}", value),
+                    LiteralValue::Fractional(x) => {
-                        source_ranges: vec![literal.into()],
+                        return Err(KclError::Syntax(KclErrorDetails {
-                    }));
+                            source_ranges: vec![self.into()],
+                            message: format!("invalid index: {x}"),
+                        }))
+                    }
+                    LiteralValue::String(s) => s,
                 }
             }
         };
@@ -2209,22 +2219,6 @@ pub fn parse_json_number_as_f64(j: &serde_json::Value, source_range: SourceRange
     }
 }
 
-pub fn parse_json_number_as_usize(j: &serde_json::Value, source_range: SourceRange) -> Result<usize, KclError> {
-    if let serde_json::Value::Number(n) = &j {
-        Ok(n.as_i64().ok_or_else(|| {
-            KclError::Syntax(KclErrorDetails {
-                source_ranges: vec![source_range],
-                message: format!("Invalid index: {}", j),
-            })
-        })? as usize)
-    } else {
-        Err(KclError::Syntax(KclErrorDetails {
-            source_ranges: vec![source_range],
-            message: format!("Invalid index: {}", j),
-        }))
-    }
-}
-
 pub fn parse_json_value_as_string(j: &serde_json::Value) -> Option<String> {
     if let serde_json::Value::String(n) = &j {
         Some(n.clone())
@@ -3289,4 +3283,40 @@ const thickness = sqrt(distance * p * FOS * 6 / (sigmaAllow * width))"#;
         let recasted = program.recast(&Default::default(), 0);
         assert_eq!(recasted.trim(), some_program_string);
     }
+
+    #[test]
+    fn recast_literal() {
+        use winnow::Parser;
+        for (i, (raw, expected, reason)) in [
+            (
+                "5.0",
+                "5.0",
+                "fractional numbers should stay fractional, i.e. don't reformat this to '5'",
+            ),
+            (
+                "5",
+                "5",
+                "integers should stay integral, i.e. don't reformat this to '5.0'",
+            ),
+            (
+                "5.0000000",
+                "5.0",
+                "if the number is f64 but not fractional, use its canonical format",
+            ),
+            ("5.1", "5.1", "straightforward case works"),
+        ]
+        .into_iter()
+        .enumerate()
+        {
+            let tokens = crate::token::lexer(raw);
+            let literal = crate::parser::parser_impl::unsigned_number_literal
+                .parse(&tokens)
+                .unwrap();
+            assert_eq!(
+                literal.recast(),
+                expected,
+                "failed test {i}, which is testing that {reason}"
+            );
+        }
+    }
 }

src/wasm-lib/kcl/src/ast/types/literal_value.rs

@@ -0,0 +1,70 @@
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
+use serde_json::Value as JValue;
+
+use super::{Literal, Value};
+
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
+#[ts(export)]
+#[serde(untagged, rename_all = "snake_case")]
+pub enum LiteralValue {
+    IInteger(i64),
+    Fractional(f64),
+    String(String),
+}
+
+impl From<Literal> for Value {
+    fn from(literal: Literal) -> Self {
+        Value::Literal(Box::new(literal))
+    }
+}
+
+impl From<LiteralValue> for JValue {
+    fn from(value: LiteralValue) -> Self {
+        match value {
+            LiteralValue::IInteger(x) => x.into(),
+            LiteralValue::Fractional(x) => x.into(),
+            LiteralValue::String(x) => x.into(),
+        }
+    }
+}
+
+impl From<f64> for LiteralValue {
+    fn from(value: f64) -> Self {
+        Self::Fractional(value)
+    }
+}
+
+impl From<i64> for LiteralValue {
+    fn from(value: i64) -> Self {
+        Self::IInteger(value)
+    }
+}
+
+impl From<String> for LiteralValue {
+    fn from(value: String) -> Self {
+        Self::String(value)
+    }
+}
+
+impl From<u32> for LiteralValue {
+    fn from(value: u32) -> Self {
+        Self::IInteger(value as i64)
+    }
+}
+impl From<u16> for LiteralValue {
+    fn from(value: u16) -> Self {
+        Self::IInteger(value as i64)
+    }
+}
+impl From<u8> for LiteralValue {
+    fn from(value: u8) -> Self {
+        Self::IInteger(value as i64)
+    }
+}
+impl From<&'static str> for LiteralValue {
+    fn from(value: &'static str) -> Self {
+        // TODO: Make this Cow<str>
+        Self::String(value.to_owned())
+    }
+}

src/wasm-lib/kcl/src/parser.rs

@@ -1,7 +1,7 @@
 use crate::{ast::types::Program, errors::KclError, token::Token};
 
 mod math;
-mod parser_impl;
+pub(crate) mod parser_impl;
 
 pub const PIPE_SUBSTITUTION_OPERATOR: &str = "%";
 pub const PIPE_OPERATOR: &str = "|>";

src/wasm-lib/kcl/src/parser/parser_impl.rs

@@ -1,4 +1,3 @@
-use serde_json::{Number as JNumber, Value as JValue};
 use winnow::{
     combinator::{alt, delimited, opt, peek, preceded, repeat, separated0, terminated},
     dispatch,
@@ -10,10 +9,10 @@ use winnow::{
 use crate::{
     ast::types::{
         ArrayExpression, BinaryExpression, BinaryOperator, BinaryPart, BodyItem, CallExpression, CommentStyle,
-        ExpressionStatement, FunctionExpression, Identifier, Literal, LiteralIdentifier, MemberExpression,
+        ExpressionStatement, FunctionExpression, Identifier, Literal, LiteralIdentifier, LiteralValue,
-        MemberObject, NonCodeMeta, NonCodeNode, NonCodeValue, ObjectExpression, ObjectProperty, PipeExpression,
+        MemberExpression, MemberObject, NonCodeMeta, NonCodeNode, NonCodeValue, ObjectExpression, ObjectProperty,
-        PipeSubstitution, Program, ReturnStatement, UnaryExpression, UnaryOperator, Value, VariableDeclaration,
+        PipeExpression, PipeSubstitution, Program, ReturnStatement, UnaryExpression, UnaryOperator, Value,
-        VariableDeclarator, VariableKind,
+        VariableDeclaration, VariableDeclarator, VariableKind,
     },
     errors::{KclError, KclErrorDetails},
     executor::SourceRange,
@@ -216,7 +215,7 @@ pub fn string_literal(i: TokenSlice) -> PResult<Literal> {
         .try_map(|token: Token| match token.token_type {
             TokenType::String => {
                 let s = token.value[1..token.value.len() - 1].to_string();
-                Ok((JValue::String(s), token))
+                Ok((LiteralValue::from(s), token))
             }
             _ => Err(KclError::Syntax(KclErrorDetails {
                 source_ranges: token.as_source_ranges(),
@@ -234,12 +233,12 @@ pub fn string_literal(i: TokenSlice) -> PResult<Literal> {
 }
 
 /// Parse a KCL literal number, with no - sign.
-fn unsigned_number_literal(i: TokenSlice) -> PResult<Literal> {
+pub(crate) fn unsigned_number_literal(i: TokenSlice) -> PResult<Literal> {
     let (value, token) = any
         .try_map(|token: Token| match token.token_type {
             TokenType::Number => {
-                if let Ok(x) = token.value.parse::<i64>() {
+                if let Ok(x) = token.value.parse::<u64>() {
-                    return Ok((JValue::Number(JNumber::from(x)), token));
+                    return Ok((LiteralValue::IInteger(x as i64), token));
                 }
                 let x: f64 = token.value.parse().map_err(|_| {
                     KclError::Syntax(KclErrorDetails {
@@ -248,13 +247,7 @@ fn unsigned_number_literal(i: TokenSlice) -> PResult<Literal> {
                     })
                 })?;
 
-                match JNumber::from_f64(x) {
+                Ok((LiteralValue::Fractional(x), token))
-                    Some(n) => Ok((JValue::Number(n), token)),
-                    None => Err(KclError::Syntax(KclErrorDetails {
-                        source_ranges: token.as_source_ranges(),
-                        message: format!("Invalid float: {}", token.value),
-                    })),
-                }
             }
             _ => Err(KclError::Syntax(KclErrorDetails {
                 source_ranges: token.as_source_ranges(),
@@ -404,10 +397,11 @@ fn integer_range(i: TokenSlice) -> PResult<Vec<Value>> {
     let (_token1, ceiling) = integer.parse_next(i)?;
     Ok((floor..=ceiling)
         .map(|num| {
+            let num = num as i64;
             Value::Literal(Box::new(Literal {
                 start: token0.start,
                 end: token0.end,
-                value: JValue::Number(num.into()),
+                value: num.into(),
                 raw: num.to_string(),
             }))
         })
@@ -1459,7 +1453,7 @@ const mySk1 = startSketchAt([0, 0])"#;
                         argument: Value::Literal(Box::new(Literal {
                             start: 32,
                             end: 33,
-                            value: JValue::Number(JNumber::from(2)),
+                            value: 2u32.into(),
                             raw: "2".to_owned(),
                         })),
                     })],
@@ -1614,7 +1608,7 @@ const mySk1 = startSketchAt([0, 0])"#;
             BinaryPart::Literal(Box::new(Literal {
                 start: 9,
                 end: 10,
-                value: JValue::Number(JNumber::from(3)),
+                value: 3u32.into(),
                 raw: "3".to_owned(),
             }))
         );
@@ -1685,11 +1679,11 @@ const mySk1 = startSketchAt([0, 0])"#;
             let BinaryPart::Literal(left) = actual.left else {
                 panic!("should be expression");
             };
-            assert_eq!(left.value, serde_json::Value::Number(1.into()));
+            assert_eq!(left.value, 1u32.into());
             let BinaryPart::Literal(right) = actual.right else {
                 panic!("should be expression");
             };
-            assert_eq!(right.value, serde_json::Value::Number(2.into()));
+            assert_eq!(right.value, 2u32.into());
         }
     }
 
@@ -1868,12 +1862,10 @@ const mySk1 = startSketchAt([0, 0])"#;
         let parsed_literal = literal.parse(&tokens).unwrap();
         assert_eq!(
             parsed_literal.value,
-            JValue::String(
             "
            // a comment
              "
-                .to_owned()
+            .into()
-            )
         );
     }
 
@@ -1978,13 +1970,13 @@ const mySk1 = startSketchAt([0, 0])"#;
             left: BinaryPart::Literal(Box::new(Literal {
                 start: 0,
                 end: 1,
-                value: serde_json::Value::Number(serde_json::Number::from(5)),
+                value: 5u32.into(),
                 raw: "5".to_owned(),
             })),
             right: BinaryPart::Literal(Box::new(Literal {
                 start: 4,
                 end: 7,
-                value: serde_json::Value::String("a".to_owned()),
+                value: "a".into(),
                 raw: r#""a""#.to_owned(),
             })),
         };
@@ -2091,14 +2083,14 @@ const mySk1 = startSketchAt([0, 0])"#;
                     left: BinaryPart::Literal(Box::new(Literal {
                         start: 0,
                         end: 1,
-                        value: serde_json::Value::Number(serde_json::Number::from(5)),
+                        value: 5u32.into(),
                         raw: "5".to_string(),
                     })),
                     operator: BinaryOperator::Add,
                     right: BinaryPart::Literal(Box::new(Literal {
                         start: 3,
                         end: 4,
-                        value: serde_json::Value::Number(serde_json::Number::from(6)),
+                        value: 6u32.into(),
                         raw: "6".to_string(),
                     })),
                 })),
@@ -2377,67 +2369,67 @@ e
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 0.into(),
+                                value: 0u32.into(),
                                 raw: "0".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 1.into(),
+                                value: 1u32.into(),
                                 raw: "1".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 2.into(),
+                                value: 2u32.into(),
                                 raw: "2".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 3.into(),
+                                value: 3u32.into(),
                                 raw: "3".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 4.into(),
+                                value: 4u32.into(),
                                 raw: "4".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 5.into(),
+                                value: 5u32.into(),
                                 raw: "5".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 6.into(),
+                                value: 6u32.into(),
                                 raw: "6".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 7.into(),
+                                value: 7u32.into(),
                                 raw: "7".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 8.into(),
+                                value: 8u32.into(),
                                 raw: "8".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 9.into(),
+                                value: 9u32.into(),
                                 raw: "9".to_string(),
                             })),
                             Value::Literal(Box::new(Literal {
                                 start: 17,
                                 end: 18,
-                                value: 10.into(),
+                                value: 10u32.into(),
                                 raw: "10".to_string(),
                             })),
                         ],