Add number with units formatting as KCL (#5195)

* Add number with units formatting as KCL

* Change type assertion helper to check what we need

* Fix rectangle unit test

src/lang/modifyAst.test.ts

@@ -5,6 +5,8 @@ import {
   Identifier,
   SourceRange,
   topLevelRange,
+  LiteralValue,
+  Literal,
 } from './wasm'
 import {
   createLiteral,
@@ -37,10 +39,26 @@ beforeAll(async () => {
 })
 
 describe('Testing createLiteral', () => {
-  it('should create a literal', () => {
+  it('should create a literal number without units', () => {
     const result = createLiteral(5)
     expect(result.type).toBe('Literal')
     expect((result as any).value.value).toBe(5)
+    expect((result as any).value.suffix).toBe('None')
+    expect((result as Literal).raw).toBe('5')
+  })
+  it('should create a literal number with units', () => {
+    const lit: LiteralValue = { value: 5, suffix: 'Mm' }
+    const result = createLiteral(lit)
+    expect(result.type).toBe('Literal')
+    expect((result as any).value.value).toBe(5)
+    expect((result as any).value.suffix).toBe('Mm')
+    expect((result as Literal).raw).toBe('5mm')
+  })
+  it('should create a literal boolean', () => {
+    const result = createLiteral(false)
+    expect(result.type).toBe('Literal')
+    expect((result as Literal).value).toBe(false)
+    expect((result as Literal).raw).toBe('false')
   })
 })
 describe('Testing createIdentifier', () => {

src/lang/modifyAst.ts

@@ -20,6 +20,7 @@ import {
   SourceRange,
   sketchFromKclValue,
   isPathToNodeNumber,
+  formatNumber,
 } from './wasm'
 import {
   isNodeSafeToReplacePath,
@@ -743,11 +744,26 @@ export function splitPathAtPipeExpression(pathToNode: PathToNode): {
   return splitPathAtPipeExpression(pathToNode.slice(0, -1))
 }
 
+/**
+ * Note: This depends on WASM, but it's not async.  Callers are responsible for
+ * awaiting init of the WASM module.
+ */
 export function createLiteral(value: LiteralValue | number): Node<Literal> {
-  const raw = `${value}`
   if (typeof value === 'number') {
     value = { value, suffix: 'None' }
   }
+  let raw: string
+  if (typeof value === 'string') {
+    // TODO: Should we handle escape sequences?
+    raw = `${value}`
+  } else if (typeof value === 'boolean') {
+    raw = `${value}`
+  } else if (typeof value.value === 'number' && value.suffix === 'None') {
+    // Fast path for numbers when there are no units.
+    raw = `${value.value}`
+  } else {
+    raw = formatNumber(value.value, value.suffix)
+  }
   return {
     type: 'Literal',
     start: 0,

src/lang/util.ts

@@ -72,14 +72,14 @@ export function isBinaryExpression(e: any): e is BinaryExpression {
   return e && e.type === 'BinaryExpression'
 }
 
-export function isLiteralValueNotStringAndBoolean(
+export function isLiteralValueNumber(
   e: LiteralValue
 ): e is { value: number; suffix: NumericSuffix } {
   return (
-    typeof e !== 'string' &&
-    typeof e !== 'boolean' &&
-    e &&
+    typeof e === 'object' &&
     'value' in e &&
-    'suffix' in e
+    typeof e.value === 'number' &&
+    'suffix' in e &&
+    typeof e.suffix === 'string'
   )
 }

src/lang/wasm.test.ts

@@ -1,5 +1,5 @@
 import { err } from 'lib/trap'
-import { initPromise, parse, ParseResult } from './wasm'
+import { formatNumber, initPromise, parse, ParseResult } from './wasm'
 import { enginelessExecutor } from 'lib/testHelpers'
 import { Node } from 'wasm-lib/kcl/bindings/Node'
 import { Program } from '../wasm-lib/kcl/bindings/Program'
@@ -20,3 +20,12 @@ it('can execute parsed AST', async () => {
   expect(err(execState)).toEqual(false)
   expect(execState.memory.get('x')?.value).toEqual(1)
 })
+
+it('formats numbers with units', () => {
+  expect(formatNumber(1, 'None')).toEqual('1')
+  expect(formatNumber(1, 'Count')).toEqual('1_')
+  expect(formatNumber(1, 'Mm')).toEqual('1mm')
+  expect(formatNumber(1, 'Inch')).toEqual('1in')
+  expect(formatNumber(0.5, 'Mm')).toEqual('0.5mm')
+  expect(formatNumber(-0.5, 'Mm')).toEqual('-0.5mm')
+})

src/lang/wasm.ts

@@ -2,6 +2,7 @@ import {
   init,
   parse_wasm,
   recast_wasm,
+  format_number,
   execute,
   kcl_lint,
   modify_ast_for_sketch_wasm,
@@ -54,6 +55,7 @@ import { ArtifactCommand } from 'wasm-lib/kcl/bindings/Artifact'
 import { ArtifactGraph as RustArtifactGraph } from 'wasm-lib/kcl/bindings/Artifact'
 import { Artifact } from './std/artifactGraph'
 import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils'
+import { NumericSuffix } from 'wasm-lib/kcl/bindings/NumericSuffix'
 
 export type { Artifact } from 'wasm-lib/kcl/bindings/Artifact'
 export type { ArtifactCommand } from 'wasm-lib/kcl/bindings/Artifact'
@@ -639,6 +641,13 @@ export const recast = (ast: Program): string | Error => {
   return recast_wasm(JSON.stringify(ast))
 }
 
+/**
+ * Format a number with suffix as KCL.
+ */
+export function formatNumber(value: number, suffix: NumericSuffix): string {
+  return format_number(value, JSON.stringify(suffix))
+}
+
 export const makeDefaultPlanes = async (
   engineCommandManager: EngineCommandManager
 ): Promise<DefaultPlanes> => {

src/lib/rectangleTool.test.ts

@@ -4,6 +4,7 @@ import {
   assertParse,
   topLevelRange,
   VariableDeclaration,
+  initPromise,
 } from 'lang/wasm'
 import { updateCenterRectangleSketch } from './rectangleTool'
 import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils'
@@ -11,6 +12,10 @@ import { getNodeFromPath } from 'lang/queryAst'
 import { findUniqueName } from 'lang/modifyAst'
 import { err, trap } from './trap'
 
+beforeAll(async () => {
+  await initPromise
+})
+
 describe('library rectangleTool helper functions', () => {
   describe('updateCenterRectangleSketch', () => {
     // regression test for https://github.com/KittyCAD/modeling-app/issues/5157

src/lib/rectangleTool.ts

@@ -20,7 +20,7 @@ import {
   isArrayExpression,
   isLiteral,
   isBinaryExpression,
-  isLiteralValueNotStringAndBoolean,
+  isLiteralValueNumber,
 } from 'lang/util'
 
 /**
@@ -146,9 +146,9 @@ export function updateCenterRectangleSketch(
     if (isArrayExpression(arrayExpression)) {
       const literal = arrayExpression.elements[0]
       if (isLiteral(literal)) {
-        if (isLiteralValueNotStringAndBoolean(literal.value)) {
+        if (isLiteralValueNumber(literal.value)) {
           callExpression.arguments[0] = createArrayExpression([
-            JSON.parse(JSON.stringify(literal)),
+            createLiteral(literal.value),
             createLiteral(Math.abs(twoX)),
           ])
         }

src/lib/wasm_lib_wrapper.ts

@@ -10,6 +10,7 @@
 import {
   parse_wasm as ParseWasm,
   recast_wasm as RecastWasm,
+  format_number as FormatNumber,
   execute as Execute,
   kcl_lint as KclLint,
   modify_ast_for_sketch_wasm as ModifyAstForSketch,
@@ -51,6 +52,9 @@ export const parse_wasm: typeof ParseWasm = (...args) => {
 export const recast_wasm: typeof RecastWasm = (...args) => {
   return getModule().recast_wasm(...args)
 }
+export const format_number: typeof FormatNumber = (...args) => {
+  return getModule().format_number(...args)
+}
 export const execute: typeof Execute = (...args) => {
   return getModule().execute(...args)
 }

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

@@ -120,6 +120,11 @@ pub mod std_utils {
     pub use crate::std::utils::{get_tangential_arc_to_info, is_points_ccw_wasm, TangentialArcInfoInput};
 }
 
+pub mod pretty {
+    pub use crate::parsing::token::NumericSuffix;
+    pub use crate::unparser::format_number;
+}
+
 use serde::{Deserialize, Serialize};
 
 #[allow(unused_imports)]

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

@@ -8,6 +8,7 @@ use crate::parsing::{
         LiteralValue, MemberExpression, MemberObject, Node, NonCodeNode, NonCodeValue, ObjectExpression, Parameter,
         PipeExpression, Program, TagDeclarator, UnaryExpression, VariableDeclaration, VariableKind,
     },
+    token::NumericSuffix,
     PIPE_OPERATOR,
 };
 
@@ -370,6 +371,11 @@ impl VariableDeclaration {
     }
 }
 
+// Used by TS.
+pub fn format_number(value: f64, suffix: NumericSuffix) -> String {
+    format!("{value}{suffix}")
+}
+
 impl Literal {
     fn recast(&self) -> String {
         match self.value {

src/wasm-lib/src/wasm.rs

@@ -5,7 +5,8 @@ use std::sync::Arc;
 use futures::stream::TryStreamExt;
 use gloo_utils::format::JsValueSerdeExt;
 use kcl_lib::{
-    exec::IdGenerator, CacheInformation, CoreDump, EngineManager, ExecState, ModuleId, OldAstState, Point2d, Program,
+    exec::IdGenerator, pretty::NumericSuffix, CacheInformation, CoreDump, EngineManager, ExecState, ModuleId,
+    OldAstState, Point2d, Program,
 };
 use tokio::sync::RwLock;
 use tower_lsp::{LspService, Server};
@@ -251,6 +252,14 @@ pub fn recast_wasm(json_str: &str) -> Result<JsValue, JsError> {
     Ok(JsValue::from_serde(&program.recast())?)
 }
 
+#[wasm_bindgen]
+pub fn format_number(value: f64, suffix_json: &str) -> Result<String, JsError> {
+    console_error_panic_hook::set_once();
+
+    let suffix: NumericSuffix = serde_json::from_str(suffix_json).map_err(JsError::from)?;
+    Ok(kcl_lib::pretty::format_number(value, suffix))
+}
+
 #[wasm_bindgen]
 pub struct ServerConfig {
     into_server: js_sys::AsyncIterator,