Fix over-indexing of lists and restore string indexing (#9388)

* fix overindexing of lists * add null checks * fixes * revert assemblysharedinfo * fix tests * reverting unchanged file * turn on previously failing tests that pass after fix * revert public to protected * revert unchanged file * fix warning messages
DynamoDS · Jan 11, 2019 · e151cb9 · e151cb9
1 parent 523e71b
commit e151cb9
Show file tree

Hide file tree

Showing 26 changed files with 527 additions and 100 deletions.
diff --git a/src/DynamoCore/Graph/Nodes/Attributes.cs b/src/DynamoCore/Graph/Nodes/Attributes.cs
@@ -200,7 +200,7 @@ public class IsMetaNodeAttribute : Attribute { }
     public class IsDesignScriptCompatibleAttribute : Attribute { }
 
     /// <summary>
-    ///    The NodeDescriptionAttribute indicates this node is obsolete
+    ///    The NodeObsoleteAttribute indicates this node is obsolete
     /// </summary>
     [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)]
     public sealed class NodeObsoleteAttribute : IsObsoleteAttribute

diff --git a/src/Engine/ProtoCore/FFI/CLRDLLModule.cs b/src/Engine/ProtoCore/FFI/CLRDLLModule.cs
@@ -1387,6 +1387,10 @@ public FFIMethodAttributes(MethodInfo method, Dictionary<MethodInfo, Attribute[]
                 {
                     IsLacingDisabled = true; 
                 }
+                else if (attr is AllowArrayPromotionAttribute)
+                {
+                    AllowArrayPromotion = (attr as AllowArrayPromotionAttribute).IsAllowed;
+                }
             }
         }
 

diff --git a/src/Engine/ProtoCore/FFI/CLRFFIFunctionPointer.cs b/src/Engine/ProtoCore/FFI/CLRFFIFunctionPointer.cs
@@ -5,8 +5,11 @@
 using ProtoCore.DSASM;
 using ProtoCore.Utils;
 using Autodesk.DesignScript.Runtime;
+using DesignScript.Builtin;
 using ProtoCore.Properties;
 using ProtoCore.Exceptions;
+using IndexOutOfRangeException = DesignScript.Builtin.IndexOutOfRangeException;
+using KeyNotFoundException = DesignScript.Builtin.KeyNotFoundException;
 
 namespace ProtoFFI
 {
@@ -361,15 +364,35 @@ protected object InvokeFunctionPointerNoThrow(ProtoCore.Runtime.Context c, Inter
 
                         dsi.LogWarning(ProtoCore.Runtime.WarningID.InvalidArguments, msg);
                     }
+                    else if (exc is IndexOutOfRangeException)
+                    {
+                        dsi.LogWarning(ProtoCore.Runtime.WarningID.IndexOutOfRange, exc.Message);
+                    }
                     else if (exc is System.ArgumentException)
+                    {
                         dsi.LogWarning(ProtoCore.Runtime.WarningID.InvalidArguments, ErrorString(exc));
+                    }
                     else if (exc is System.NullReferenceException)
+                    {
                         dsi.LogWarning(ProtoCore.Runtime.WarningID.AccessViolation, ErrorString(null));
+                    }
+                    else if (exc is StringOverIndexingException)
+                    {
+                        dsi.LogWarning(ProtoCore.Runtime.WarningID.OverIndexing, exc.Message);
+                    }
+                    else if (exc is KeyNotFoundException)
+                    {
+                        dsi.LogWarning(ProtoCore.Runtime.WarningID.InvalidIndexing, exc.Message);
+                    }
                     else
+                    {
                         dsi.LogWarning(ProtoCore.Runtime.WarningID.AccessViolation, ErrorString(exc));
+                    }
                 }
                 else
+                {
                     dsi.LogWarning(ProtoCore.Runtime.WarningID.AccessViolation, ErrorString(ex));
+                }
             }
             catch (System.Reflection.TargetParameterCountException ex)
             {
@@ -420,7 +443,9 @@ protected object InvokeFunctionPointerNoThrow(ProtoCore.Runtime.Context c, Inter
                     dsi.LogWarning(ProtoCore.Runtime.WarningID.InvalidArguments, ErrorString(ex.InnerException));
                 }
                 else
+                {
                     dsi.LogWarning(ProtoCore.Runtime.WarningID.InvalidArguments, ErrorString(ex));
+                }
             }
             catch (Exception ex)
             {
@@ -440,9 +465,11 @@ private string ErrorString(System.Exception ex)
                 return ex.Message;
 
             string msg = (ex == null) ? "" : ex.Message;
+
             if (string.IsNullOrEmpty(msg) || msg.Contains("operation failed"))
                 return string.Format(Resources.OperationFailType1, ReflectionInfo.DeclaringType.Name, ReflectionInfo.Name);
 
+
             return string.Format(Resources.OperationFailType2, ReflectionInfo.DeclaringType.Name, ReflectionInfo.Name, msg);
         }
 

diff --git a/src/Engine/ProtoCore/FFI/CLRObjectMarshaler.cs b/src/Engine/ProtoCore/FFI/CLRObjectMarshaler.cs
@@ -1332,7 +1332,7 @@ void core_Dispose(ProtoCore.RuntimeCore sender)
         {
             CLRObjectMarshaler marshaller = null;
             if (!mObjectMarshlers.TryGetValue(sender, out marshaller))
-                throw new KeyNotFoundException();
+                throw new System.Collections.Generic.KeyNotFoundException();
 
             mObjectMarshlers.Remove(sender);
 

diff --git a/src/Engine/ProtoCore/Lang/FunctionEndPoint.cs b/src/Engine/ProtoCore/Lang/FunctionEndPoint.cs
@@ -245,6 +245,17 @@ public int ComputeTypeDistance(List<StackValue> args, ProtoCore.DSASM.ClassTable
 
         public int GetConversionDistance(List<StackValue> reducedParamSVs, ProtoCore.DSASM.ClassTable classTable, bool allowArrayPromotion, RuntimeCore runtimeCore)
         {
+            // If the replication strategy allows array promotion, first check for the case
+            // where it could be disabled using the [AllowArrayPromotion(false)] attribute
+            // and if so set it from the attribute.
+            if (allowArrayPromotion)
+            {
+                var ma = procedureNode.MethodAttribute;
+                if (ma != null)
+                {
+                    allowArrayPromotion = ma.AllowArrayPromotion;
+                }
+            }
             int dist = ComputeTypeDistance(reducedParamSVs, classTable, runtimeCore, allowArrayPromotion);
             if (dist >= 0 && dist != (int)ProcedureDistance.MaxDistance) //Is it possible to convert to this type?
             {

diff --git a/src/Engine/ProtoCore/Parser/AssociativeAST.cs b/src/Engine/ProtoCore/Parser/AssociativeAST.cs
@@ -1663,6 +1663,7 @@ public IEnumerable<string> ReturnKeys
         public string ObsoleteMessage { get; protected set; }
         public bool IsObsolete { get { return !string.IsNullOrEmpty(ObsoleteMessage); } }
         public bool IsLacingDisabled { get; protected set; }
+        public bool AllowArrayPromotion { get; protected set; } = true;
 
         /// <summary>
         /// Returns/Sets description for the method.

diff --git a/src/Libraries/DesignScriptBuiltin/Builtin.cs b/src/Libraries/DesignScriptBuiltin/Builtin.cs
@@ -2,8 +2,10 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
+using System.Reflection;
 using System.Runtime.InteropServices;
 using Autodesk.DesignScript.Runtime;
+using Builtin.Properties;
 
 namespace DesignScript
 {
@@ -16,9 +18,17 @@ public static class Get
 
             public static object ValueAtIndex(Dictionary dictionary, string key)
             {
-                return dictionary.ValueAtKey(key);
+                try
+                {
+                    return dictionary.ValueAtKey(key);
+                }
+                catch (System.Collections.Generic.KeyNotFoundException e)
+                {
+                    throw new KeyNotFoundException(e.Message);
+                }
             }
 
+            [AllowArrayPromotion(false)]
             public static object ValueAtIndex(IList list, int index)
             {
                 while (index < 0)
@@ -27,7 +37,34 @@ public static object ValueAtIndex(IList list, int index)
                     if (count == 0) break;
                     index += count;
                 }
-                return list[index];
+
+                try
+                {
+                    return list[index];
+                }
+                catch (ArgumentOutOfRangeException)
+                {
+                    throw new IndexOutOfRangeException(DesignScriptBuiltin.IndexOutOfRangeExceptionMessage);
+                }
+            }
+
+            public static object ValueAtIndex(string stringList, int index)
+            {
+                while (index < 0)
+                {
+                    var count = stringList.Length;
+                    if (count == 0) break;
+                    index += count;
+                }
+
+                try
+                {
+                    return stringList[index];
+                }
+                catch (System.IndexOutOfRangeException)
+                {
+                    throw new StringOverIndexingException(DesignScriptBuiltin.StringOverIndexingExceptionMessage);
+                }
             }
         }
     }

diff --git a/src/Libraries/DesignScriptBuiltin/DesignScriptBuiltin.csproj b/src/Libraries/DesignScriptBuiltin/DesignScriptBuiltin.csproj
@@ -54,7 +54,13 @@
     </Compile>
     <Compile Include="Builtin.cs" />
     <Compile Include="Dictionary.cs" />
+    <Compile Include="IndexingExceptions.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="Properties\DesignScriptBuiltin.Designer.cs">
+      <AutoGen>True</AutoGen>
+      <DesignTime>True</DesignTime>
+      <DependentUpon>DesignScriptBuiltin.resx</DependentUpon>
+    </Compile>
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />
@@ -67,6 +73,14 @@
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="DesignScriptBuiltinImages.resx" />
+    <EmbeddedResource Include="Properties\DesignScriptBuiltin.en-US.resx">
+      <SubType>Designer</SubType>
+    </EmbeddedResource>
+    <EmbeddedResource Include="Properties\DesignScriptBuiltin.resx">
+      <Generator>ResXFileCodeGenerator</Generator>
+      <LastGenOutput>DesignScriptBuiltin.Designer.cs</LastGenOutput>
+      <SubType>Designer</SubType>
+    </EmbeddedResource>
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
   <Target Name="AfterBuild" Condition=" '$(OS)' != 'Unix' ">
@@ -80,6 +94,6 @@
     </GetAssemblyIdentity>
     <!-- Generate customization dll -->
     <GenerateResource SdkToolsPath="$(TargetFrameworkSDKToolsDirectory)" UseSourcePath="true" Sources="$(ProjectDir)DesignScriptBuiltinImages.resx" OutputResources="$(ProjectDir)DesignScriptBuiltinImages.resources" References="$(FrameworkAssembliesPath)System.Drawing.dll" />
-    <AL SdkToolsPath="$(TargetFrameworkSDKToolsDirectory)"  TargetType="library" EmbedResources="$(ProjectDir)DesignScriptBuiltinImages.resources" OutputAssembly="$(OutDir)DesignScriptBuiltin.customization.dll" Version="%(AssemblyInfo.Version)" />
+    <AL SdkToolsPath="$(TargetFrameworkSDKToolsDirectory)" TargetType="library" EmbedResources="$(ProjectDir)DesignScriptBuiltinImages.resources" OutputAssembly="$(OutDir)DesignScriptBuiltin.customization.dll" Version="%(AssemblyInfo.Version)" />
   </Target>
 </Project>
diff --git a/src/Libraries/DesignScriptBuiltin/IndexingExceptions.cs b/src/Libraries/DesignScriptBuiltin/IndexingExceptions.cs
@@ -0,0 +1,28 @@
+using System;
+
+namespace DesignScript.Builtin
+{
+    public class KeyNotFoundException : Exception
+    {
+        public KeyNotFoundException(string message)
+            : base(message)
+        {
+        }
+    }
+
+    public class IndexOutOfRangeException : Exception
+    {
+        public IndexOutOfRangeException(string message)
+            : base(message)
+        {
+        }
+    }
+
+    public class StringOverIndexingException : Exception
+    {
+        public StringOverIndexingException(string message)
+            : base(message)
+        {
+        }
+    }
+}
diff --git a/src/Libraries/DesignScriptBuiltin/Properties/DesignScriptBuiltin.Designer.cs b/src/Libraries/DesignScriptBuiltin/Properties/DesignScriptBuiltin.Designer.cs
-Original file line number
+Diff line change
@@ Expand Up @@
                     {
                         IsLacingDisabled = true;
                     }
+                    else if (attr is AllowArrayPromotionAttribute)
+                    {
+                        AllowArrayPromotion = (attr as AllowArrayPromotionAttribute).IsAllowed;
+                    }
                 }
             }
@@ Expand Down @@