DYN-5965: introduce new string from nodes that accept numeric format specifiers

src/Engine/ProtoCore/DSASM/Mirror/ExecutionMirror.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Text;
 using ProtoCore.Lang;
@@ -20,7 +21,7 @@ public SymbolNotFoundException(string symbolName)
         public string SymbolName { get; private set; } 
     }
 
-    //Status: Draft, experiment
+    //Status: Draft, experiment :)
 
     /// <summary>
     /// Provides reflective capabilities over the execution of a DSASM Executable
@@ -44,24 +45,91 @@ public ExecutionMirror(ProtoCore.DSASM.Executive exec, ProtoCore.RuntimeCore cor
             MirrorTarget = exec;
         }
 
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="val"></param>
+        /// <param name="heap"></param>
+        /// <param name="langblock"></param>
+        /// <param name="forPrint"></param>
+        /// <param name="formatSpecifier"></param>
+        /// <returns></returns>
+        public string GetStringValueUsingFormat(StackValue val,string formatSpecifier, Heap heap, int langblock, bool forPrint = false)
+        {
+            return GetStringValueImplementation(val,formatSpecifier, heap, langblock, -1, -1, forPrint);
+        }
+
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="val"></param>
+        /// <param name="heap"></param>
+        /// <param name="langblock"></param>
+        /// <param name="forPrint"></param>
+        /// <returns></returns>
+        [Obsolete]
+        //TODO this is used all over the place internally
+        //so it's good to keep it and likely make it internal
+        //until we want to change formatting everywhere we process strings.
         public string GetStringValue(StackValue val, Heap heap, int langblock, bool forPrint = false)
         {
             return GetStringValue(val, heap, langblock, -1, -1, forPrint);
         }
 
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="val"></param>
+        /// <param name="heap"></param>
+        /// <param name="langblock"></param>
+        /// <param name="maxArraySize"></param>
+        /// <param name="maxOutputDepth"></param>
+        /// <param name="forPrint"></param>
+        /// <returns></returns>
+        [Obsolete]
         public string GetStringValue(StackValue val, Heap heap, int langblock, int maxArraySize, int maxOutputDepth, bool forPrint = false)
         {
-            if (formatParams == null)
-                formatParams = new OutputFormatParameters(maxArraySize, maxOutputDepth);
+            //use F6 to match the existing behavior.
+            return GetStringValueImplementation(val, StringUtils.LEGACYFORMATTING, heap, langblock, maxArraySize, maxOutputDepth, forPrint);
+        }
+
+        private string GetStringValueImplementation(StackValue val, string formatSpecifier, Heap heap, int langblock, int maxArraySize,
+           int maxOutputDepth, bool forPrint = false)
+        {
+            var legacyBehavior = formatSpecifier == StringUtils.LEGACYFORMATTING;
+            formatParams ??= new OutputFormatParameters(maxArraySize, maxOutputDepth);
+            //if format is the dynamo format specifier then use prefs format.
+            if (formatSpecifier == StringUtils.DynamoPreferencesNumberFormat)
+            {
+                formatSpecifier = ProtoCore.Mirror.MirrorData.PrecisionFormat;
+            }
 
+
             if (val.IsInteger)
             {
-                return val.IntegerValue.ToString();
+                //in legacy mode we don't format ints.
+                if (legacyBehavior)
+                {
+                    return val.IntegerValue.ToString();
+                }
+                //TODO I am not sure if it's best to just return null or some message here
+                //or if we should catch the error higher up where we can log a runtime warning.
+                //currently we are doing the latter.
+
+                //return val.IntegerValue.SafeToStringWithFormat(formatSpecifier);
+                return val.IntegerValue.ToString(formatSpecifier);
+
             }
 
             if (val.IsDouble)
             {
-                return val.DoubleValue.ToString("F6");
+                //we always use f6 for doubles in legacy mode.
+                if (legacyBehavior)
+                {
+                    return val.DoubleValue.ToString("F6");
+                }
+                //return val.DoubleValue.SafeToStringWithFormat(formatSpecifier);
+                return val.DoubleValue.ToString(formatSpecifier);
             }
 
             if (val.IsNull)
@@ -77,7 +145,7 @@ public string GetStringValue(StackValue val, Heap heap, int langblock, int maxAr
             if (val.IsArray)
             {
                 HashSet<int> pointers = new HashSet<int> { val.ArrayPointer };
-                string arrTrace = GetArrayTrace(val, heap, langblock, pointers, forPrint);
+                string arrTrace = GetArrayTrace(val, formatSpecifier, heap, langblock, pointers, forPrint);
                 if (forPrint)
                     return "[" + arrTrace + "]";
 
@@ -125,6 +193,8 @@ public string GetStringValue(StackValue val, Heap heap, int langblock, int maxAr
             return "null"; // "Value not yet supported for tracing";
         }
 
+        //TODO - use format specifier when converting to string
+        //obsolete this method and introduce another.
         public string GetClassTrace(StackValue val, Heap heap, int langblock, bool forPrint)
         {
             if (!formatParams.ContinueOutputTrace())
@@ -212,7 +282,7 @@ public string GetClassTrace(StackValue val, Heap heap, int langblock, bool forPr
             }
         }
 
-        private string GetPointerTrace(StackValue ptr, Heap heap, int langblock, HashSet<int> pointers, bool forPrint)
+        private string GetPointerTrace(StackValue ptr, string formatSpecifier, Heap heap, int langblock, HashSet<int> pointers, bool forPrint)
         {
             if (pointers.Contains(ptr.ArrayPointer))
             {
@@ -223,13 +293,13 @@ private string GetPointerTrace(StackValue ptr, Heap heap, int langblock, HashSet
 
             if (forPrint)
             {
-                return "[" + GetArrayTrace(ptr, heap, langblock, pointers, forPrint) + "]";
+                return "[" + GetArrayTrace(ptr,formatSpecifier, heap, langblock, pointers, forPrint) + "]";
             }
 
-            return "[ " + GetArrayTrace(ptr, heap, langblock, pointers, forPrint) + " ]";
+            return "[ " + GetArrayTrace(ptr,formatSpecifier, heap, langblock, pointers, forPrint) + " ]";
         }
 
-        private string GetArrayTrace(StackValue svArray, Heap heap, int langblock, HashSet<int> pointers, bool forPrint)
+        private string GetArrayTrace(StackValue svArray, string formatSpecifier, Heap heap, int langblock, HashSet<int> pointers, bool forPrint)
         {
             if (!formatParams.ContinueOutputTrace())
                 return "...";
@@ -264,11 +334,11 @@ private string GetArrayTrace(StackValue svArray, Heap heap, int langblock, HashS
                 StackValue sv = array.GetValueFromIndex(n, runtimeCore);
                 if (sv.IsArray)
                 {
-                    arrayElements.Append(GetPointerTrace(sv, heap, langblock, pointers, forPrint));
+                    arrayElements.Append(GetPointerTrace(sv,formatSpecifier, heap, langblock, pointers, forPrint));
                 }
                 else
                 {
-                    arrayElements.Append(GetStringValue(array.GetValueFromIndex(n, runtimeCore), heap, langblock, forPrint));
+                    arrayElements.Append(GetStringValueUsingFormat(array.GetValueFromIndex(n, runtimeCore),formatSpecifier, heap, langblock, forPrint));
                 }
 
                 // If we need to truncate this array (halfArraySize > 0), and we have 

src/Engine/ProtoCore/Lang/BuiltInFunctionEndPoint.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -373,8 +373,26 @@ public override StackValue Execute(ProtoCore.Runtime.Context c, List<StackValue>
                 case BuiltInMethods.MethodID.ToString:
                 case BuiltInMethods.MethodID.ToStringFromObject:
                 case BuiltInMethods.MethodID.ToStringFromArray:
-                    ret = StringUtils.ConvertToString(formalParameters[0], runtimeCore, rmem);
-                    break;
+                    {
+                        ret = StringUtils.ConvertToString(formalParameters[0], runtimeCore, rmem);
+                        break;
+                    }
+                case BuiltInMethods.MethodID.ToStringFromObjectAndFormat:
+                case BuiltInMethods.MethodID.ToStringFromArrayAndFormat:
+                    {
+                        try
+                        {
+                            ret = StringUtils.ConvertToString(formalParameters, runtimeCore, rmem);
+                        }
+                        catch(System.FormatException fe)
+                        {
+                            runtimeCore.RuntimeStatus.LogWarning(WarningID.InvalidArguments, fe.Message);
+                            //TODO reuse this string instead of allocating a new one each time?
+                            //the message could be different...
+                            ret = StackValue.BuildString(fe.Message,runtimeCore.Heap);
+                        }
+                        break;
+                    }
                 case BuiltInMethods.MethodID.ImportData:
                     ret = ContextDataBuiltIns.ImportData(formalParameters[0], formalParameters[1], runtimeCore, interpreter, c);
                     break;

src/Engine/ProtoCore/Lang/BuiltInMethods.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Linq;
 using ProtoCore.AST.AssociativeAST;
 using ProtoCore.DSASM;
@@ -69,8 +69,13 @@ public enum MethodID
             NodeAstFailed,
             GC,
             ConditionalIf,
+            ToStringFromObjectAndFormat,
+            ToStringFromArrayAndFormat,
         }
 
+        //this array gets accessed using the MethodID enum
+        //so its order is important... could be a dictionary or attributes on the enums
+        //to avoid confusion, easy to mess this up.
         private static string[] methodNames = new string[]
         {
             "AllFalse",                 // kAllFalse
@@ -130,6 +135,8 @@ public enum MethodID
             Constants.kNodeAstFailed,   // kNodeAstFailed
             "__GC",                     // kGC
             Constants.kIfConditionalMethodName,
+            "__ToStringFromObjectAndFormat",     // kToStringFromObjectAndFormat
+            "__ToStringFromArrayAndFormat",      // kToStringFromArrayAndFormat
         };
 
         public static string GetMethodName(MethodID id)
@@ -797,6 +804,7 @@ public BuiltInMethods(Core core)
                         new KeyValuePair<string, ProtoCore.Type>("object", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var, 0)),
                     },
                     ID = BuiltInMethods.MethodID.ToStringFromObject,
+                    MethodAttributes = new MethodAttributes(true, false, @"This method is obsolete, please use 'ToStringFromObjectAndFormat' "),
                 },
 
                 new BuiltInMethod
@@ -807,7 +815,35 @@ public BuiltInMethods(Core core)
                     {
                         new KeyValuePair<string, Type>("list", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var)),
                     }.ToList(),
-                    ID = BuiltInMethods.MethodID.ToStringFromArray
+                    ID = BuiltInMethods.MethodID.ToStringFromArray,
+                    MethodAttributes = new MethodAttributes(true, false, @"This method is obsolete, please use 'ToStringFromArrayAndFormat' "),
+                },
+
+                new BuiltInMethod
+                {
+                    ReturnType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.String, 0),
+                    Parameters = new List<KeyValuePair<string, ProtoCore.Type>>
+                    {
+                        new KeyValuePair<string, ProtoCore.Type>("object", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var, 0)),
+                        new KeyValuePair<string, ProtoCore.Type>("formatSpecifier", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.String, 0)),
+                        //TODO (MJK)
+                        //if we wanted to support default args for builtins we would need to support these names being parsed as binary expressions instead
+                        //of just identifiers as is done today. We would need to add a new Parameters entry because all of this is public :(
+                        //or we could invoke the parser directly on these strings as is done for custom node symbols...
+                    },
+                    ID = BuiltInMethods.MethodID.ToStringFromObjectAndFormat,
+                },
+
+                new BuiltInMethod
+                {
+                    ReturnType = TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.String, 0),
+
+                    Parameters = new []
+                    {
+                        new KeyValuePair<string, Type>("list", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.Var)),
+                        new KeyValuePair<string, ProtoCore.Type>("formatSpecifier", TypeSystem.BuildPrimitiveTypeObject(PrimitiveType.String, 0)),
+                    }.ToList(),
+                    ID = BuiltInMethods.MethodID.ToStringFromArrayAndFormat
                 },
 
                 new BuiltInMethod
@@ -915,4 +951,4 @@ public BuiltInMethods(Core core)
             };
         }
     }
-}
+}

src/Engine/ProtoCore/Utils/StringUtils.cs

@@ -1,12 +1,16 @@
 using ProtoCore.DSASM;
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Text.RegularExpressions;
 
 namespace ProtoCore.Utils
 {
     public static class StringUtils
     {
+        internal const string DynamoPreferencesNumberFormat = nameof(DynamoPreferencesNumberFormat);
+        internal const string LEGACYFORMATTING = nameof(LEGACYFORMATTING);
+
         public static int CompareString(StackValue s1, StackValue s2, RuntimeCore runtimeCore)
         {
             if (!s1.IsString || !s2.IsString)
@@ -20,21 +24,72 @@ public static int CompareString(StackValue s1, StackValue s2, RuntimeCore runtim
             return string.Compare(str1, str2);
         }
 
+        /// <summary>
+        /// Wraps ToString(format) in a try catch. Will return empty string if format is not valid for the target type.
+        /// </summary>
+        /// <typeparam name="T"></typeparam>
+        /// <param name="target"></param>
+        /// <param name="Format"></param>
+        /// <returns>Will return empty string if format is not valid for the target type.</returns>
+        internal static string SafeToStringWithFormat<T>(this T target, string Format)
+            where T : IFormattable
+        {
+            try
+            {
+                return target.ToString(Format, null);
+            }
+            catch
+            {
+                return string.Empty;
+            }
+        }
+
         public static string GetStringValue(StackValue sv, RuntimeCore runtimeCore)
         {
             ProtoCore.DSASM.Mirror.ExecutionMirror mirror = new DSASM.Mirror.ExecutionMirror(new ProtoCore.DSASM.Executive(runtimeCore), runtimeCore);
             return mirror.GetStringValue(sv, runtimeCore.RuntimeMemory.Heap, 0, true);
         }
 
+        //used by legacy ToString methods without format specifier.
         public static StackValue ConvertToString(StackValue sv, RuntimeCore runtimeCore, ProtoCore.Runtime.RuntimeMemory rmem)
+        {
+            //maintain old behavior of existing string conversion nodes by passing null for formatSpecifier.
+            return ConvertToStringInternal(sv, runtimeCore, null);
+        }
+        //used by new ToString methods with format specifier.
+        internal static StackValue ConvertToString(IEnumerable<StackValue> args, RuntimeCore runtimeCore, ProtoCore.Runtime.RuntimeMemory rmem)
+        {
+            //TODO dislike this as a default...
+            var formatSpecifier = DynamoPreferencesNumberFormat;
+            var sv = args.ElementAt(0);
+            if (args.Count() > 1)
+            {   //TODO performance concern?
+                formatSpecifier = GetStringValue(args.ElementAt(1),runtimeCore);
+            }
+            return ConvertToStringInternal(sv, runtimeCore, formatSpecifier);
+        }
+
+        private static StackValue ConvertToStringInternal(StackValue sv, RuntimeCore runtimeCore, string formatSpecifier)
         {
             StackValue returnSV;
             //TODO: Change Execution mirror class to have static methods, so that an instance does not have to be created
-            ProtoCore.DSASM.Mirror.ExecutionMirror mirror = new DSASM.Mirror.ExecutionMirror(new ProtoCore.DSASM.Executive(runtimeCore), runtimeCore);
-            returnSV = ProtoCore.DSASM.StackValue.BuildString(mirror.GetStringValue(sv, runtimeCore.RuntimeMemory.Heap, 0, true), runtimeCore.RuntimeMemory.Heap);
+            ProtoCore.DSASM.Mirror.ExecutionMirror mirror =
+                new DSASM.Mirror.ExecutionMirror(new ProtoCore.DSASM.Executive(runtimeCore), runtimeCore);
+            if (formatSpecifier == null)
+            {
+               returnSV = ProtoCore.DSASM.StackValue.BuildString(
+               mirror.GetStringValue(sv, runtimeCore.RuntimeMemory.Heap, 0, true), runtimeCore.RuntimeMemory.Heap);
+            }
+            else
+            {
+                returnSV = ProtoCore.DSASM.StackValue.BuildString(
+                    mirror.GetStringValueUsingFormat(sv, formatSpecifier, runtimeCore.RuntimeMemory.Heap, 0, true), runtimeCore.RuntimeMemory.Heap);
+            }
             return returnSV;
         }
 
+
+
         public static StackValue ConcatString(StackValue op1, StackValue op2, RuntimeCore runtimeCore)
         {
             var v1 = runtimeCore.RuntimeMemory.Heap.ToHeapObject<DSString>(op1).Value;