[DYN-2588] Enable ArbitraryDimensionArrayImport attribute to function return values, apply this to Dictionary.Values getter (#13055)

aparajit-pratap · web-flow · commit 2f4a2701ac0e · 2022-06-27T13:52:04.000-04:00
* add allowrankreduction attr to Dictionary.Values

* update tests

* enable arbitrarydimensionarrayimport attribute for return values, apply to Dictionary.Values

* revert changed tests as there's no change in behaviour

* add unit test

* support return value arbitrary dimension attribute for all methods, add test
diff --git a/src/Engine/ProtoCore/FFI/CLRDLLModule.cs b/src/Engine/ProtoCore/FFI/CLRDLLModule.cs
@@ -634,18 +634,6 @@ private static bool SupressesImport(MemberInfo member, Dictionary<MethodInfo, At
             return false;
         }
 
-        private bool AllowsRankReduction(MethodInfo method)
-        {
-            object[] atts = method.GetCustomAttributes(false);
-            foreach (var item in atts)
-            {
-                if (item is AllowRankReductionAttribute)
-                    return true;
-            }
-
-            return false;
-        }
-
         private ProtoCore.AST.AssociativeAST.VarDeclNode ParseFieldDeclaration(FieldInfo f)
         {
             if (null == f || SupressesImport(f))
@@ -716,7 +704,7 @@ private AssociativeNode ParseMethod(MethodInfo method)
             }
             func.Signature = ParseArgumentSignature(method);
 
-            if ((retype.IsIndexable && mattrs.AllowRankReduction)
+            if ((retype.IsIndexable && (mattrs.AllowRankReduction || mattrs.ArbitraryDimensionArrayImport))
                 || (typeof(object).Equals(method.ReturnType)))
             {
                 retype.rank = Constants.kArbitraryRank;
@@ -1261,14 +1249,15 @@ public FFIClassAttributes(Type type)
     /// </summary>
     public class FFIMethodAttributes : ProtoCore.AST.AssociativeAST.MethodAttributes
     {
-        private Attribute[] attributes;
+        private List<Attribute> attributes;
         /// <summary>
         /// FFI method attributes.
         /// </summary>
         public IEnumerable<Attribute> Attributes
         {
             get { return attributes; }
         }
+        internal bool ArbitraryDimensionArrayImport { get; }
         public bool AllowRankReduction { get; protected set; }
         public bool RequireTracing { get; protected set; }
 
@@ -1342,13 +1331,22 @@ public FFIMethodAttributes(MethodBase method, Dictionary<MethodInfo, Attribute[]
             }
 
             Attribute[] atts;
+            attributes = new List<Attribute>();
             if (method is MethodInfo mInfo && getterAttributes != null && getterAttributes.TryGetValue(mInfo, out atts))
             {
-                attributes = atts;
+                attributes.AddRange(atts);
             }
             else
             {
-                attributes = method.GetCustomAttributes(false).Cast<Attribute>().ToArray();
+                attributes.AddRange(method.GetCustomAttributes(false).Cast<Attribute>());
+            }
+            if(method is MethodInfo minfo)
+            {
+                var returnTypeAtts = minfo.ReturnTypeCustomAttributes;
+                if (returnTypeAtts != null)
+                {
+                    attributes.AddRange(returnTypeAtts.GetCustomAttributes(false).Cast<Attribute>());
+                }
             }
 
             foreach (var attr in attributes)
@@ -1400,6 +1398,10 @@ public FFIMethodAttributes(MethodBase method, Dictionary<MethodInfo, Attribute[]
                 {
                     AllowArrayPromotion = (attr as AllowArrayPromotionAttribute).IsAllowed;
                 }
+                else if(attr is ArbitraryDimensionArrayImportAttribute)
+                {
+                    ArbitraryDimensionArrayImport = true;
+                }
             }
         }
     }
diff --git a/src/Libraries/DesignScriptBuiltin/Dictionary.cs b/src/Libraries/DesignScriptBuiltin/Dictionary.cs
@@ -59,9 +59,11 @@ public IDictionary<string, object> Components()
             ///     Produces the values in a Dictionary.
             /// </summary>
             /// <returns name="values">Values of the dictionary</returns>
-            // [AllowRankReduction]
-            // TODO: Consider adding this attribute in 3.0 (DYN-1697)
-            public IEnumerable<object> Values => D.Values;
+            public IEnumerable<object> Values
+            {
+                [return: ArbitraryDimensionArrayImport]
+                get { return D.Values; }
+            }
 
             /// <summary>
             ///     The number of key value pairs in a Dictionary.
diff --git a/test/DynamoCoreTests/Nodes/DictionaryTests.cs b/test/DynamoCoreTests/Nodes/DictionaryTests.cs
@@ -78,5 +78,18 @@ public void EmptyDictionaryToString()
             var dict = Dictionary.ByKeysValues(new string[0], new object[0]);
             Assert.AreEqual(dict.ToString(), "{}");
         }
+
+        [Test]
+        public void Dictionary_ListOfValues_ReturnedAsExpected()
+        {
+            var dynFilePath = Path.Combine(TestDirectory, @"core\dictionary\Dictionary.Values2.dyn");
+            OpenModel(dynFilePath);
+
+            var validationData1 = new List<string> { "foo" };
+            var validationData2 = new object[] { 1, new[] { 1, 2 } };
+
+            AssertPreviewValue("f64c1972520144d7a6d342937584c47e", validationData2);
+            AssertPreviewValue("bc957838571c4c56af2bb714f9f40bd7", validationData1);
+        }
     }
 }
diff --git a/test/Engine/FFITarget/TestCSharpAttribute.cs b/test/Engine/FFITarget/TestCSharpAttribute.cs
@@ -1,4 +1,6 @@
 ﻿using System;
+using System.Collections.Generic;
+using Autodesk.DesignScript.Runtime;
 
 namespace FFITarget
 {
@@ -50,5 +52,11 @@ public string Test2()
         {
             return "this node is obsolete.";
         }
+
+        [return: ArbitraryDimensionArrayImport]
+        public static IEnumerable<object> TestReturnAttribute()
+        {
+            return new object[] { 1.3, new double[] { 4.5, 7.8 } };
+        }
     }
 }
diff --git a/test/Engine/ProtoTest/FFITests/FFITest.cs b/test/Engine/ProtoTest/FFITests/FFITest.cs
@@ -193,5 +193,18 @@ public void TestToStringDoesNotThrow()
             Assert.DoesNotThrow(() => { mirror.GetStringValue(x,this.core.Heap, 0); }); 
             
         }
+
+        [Test]
+        public void TestReturnArbitraryDimensionAttribute()
+        {
+            string code = @"
+import (TestCSharpAttribute from ""FFITarget.dll"");
+x = TestCSharpAttribute.TestReturnAttribute();
+";
+            var mirror = thisTest.RunScriptSource(code);
+            var x = mirror.GetValue("x");
+            thisTest.Verify("x", new object[] { 1.3, new double[] { 4.5, 7.8 } });
+
+        }
     }
 }
diff --git a/test/core/dictionary/Dictionary.Values2.dyn b/test/core/dictionary/Dictionary.Values2.dyn

Original file line number	Diff line number	Diff line change
`@@ -634,18 +634,6 @@ private static bool SupressesImport(MemberInfo member, Dictionary<MethodInfo, At`
`634`	`634`	`return false;`
`635`	`635`	`}`
`636`	`636`
`637`		`- private bool AllowsRankReduction(MethodInfo method)`
`638`		`- {`
`639`		`- object[] atts = method.GetCustomAttributes(false);`
`640`		`- foreach (var item in atts)`
`641`		`- {`
`642`		`- if (item is AllowRankReductionAttribute)`
`643`		`- return true;`
`644`		`- }`
`645`		`-`
`646`		`- return false;`
`647`		`- }`
`648`		`-`
`649`	`637`	`private ProtoCore.AST.AssociativeAST.VarDeclNode ParseFieldDeclaration(FieldInfo f)`
`650`	`638`	`{`
`651`	`639`	`if (null == f \|\| SupressesImport(f))`
`@@ -716,7 +704,7 @@ private AssociativeNode ParseMethod(MethodInfo method)`
`716`	`704`	`}`
`717`	`705`	`func.Signature = ParseArgumentSignature(method);`
`718`	`706`
`719`		`- if ((retype.IsIndexable && mattrs.AllowRankReduction)`
	`707`	`+ if ((retype.IsIndexable && (mattrs.AllowRankReduction \|\| mattrs.ArbitraryDimensionArrayImport))`
`720`	`708`	`\|\| (typeof(object).Equals(method.ReturnType)))`
`721`	`709`	`{`
`722`	`710`	`retype.rank = Constants.kArbitraryRank;`
`@@ -1261,14 +1249,15 @@ public FFIClassAttributes(Type type)`
`1261`	`1249`	`/// </summary>`
`1262`	`1250`	`public class FFIMethodAttributes : ProtoCore.AST.AssociativeAST.MethodAttributes`
`1263`	`1251`	`{`
`1264`		`- private Attribute[] attributes;`
	`1252`	`+ private List<Attribute> attributes;`
`1265`	`1253`	`/// <summary>`
`1266`	`1254`	`/// FFI method attributes.`
`1267`	`1255`	`/// </summary>`
`1268`	`1256`	`public IEnumerable<Attribute> Attributes`
`1269`	`1257`	`{`
`1270`	`1258`	`get { return attributes; }`
`1271`	`1259`	`}`
	`1260`	`+ internal bool ArbitraryDimensionArrayImport { get; }`
`1272`	`1261`	`public bool AllowRankReduction { get; protected set; }`
`1273`	`1262`	`public bool RequireTracing { get; protected set; }`
`1274`	`1263`
`@@ -1342,13 +1331,22 @@ public FFIMethodAttributes(MethodBase method, Dictionary<MethodInfo, Attribute[]`
`1342`	`1331`	`}`
`1343`	`1332`
`1344`	`1333`	`Attribute[] atts;`
	`1334`	`+ attributes = new List<Attribute>();`
`1345`	`1335`	`if (method is MethodInfo mInfo && getterAttributes != null && getterAttributes.TryGetValue(mInfo, out atts))`
`1346`	`1336`	`{`
`1347`		`- attributes = atts;`
	`1337`	`+ attributes.AddRange(atts);`
`1348`	`1338`	`}`
`1349`	`1339`	`else`
`1350`	`1340`	`{`
`1351`		`- attributes = method.GetCustomAttributes(false).Cast<Attribute>().ToArray();`
	`1341`	`+ attributes.AddRange(method.GetCustomAttributes(false).Cast<Attribute>());`
	`1342`	`+ }`
	`1343`	`+ if(method is MethodInfo minfo)`
	`1344`	`+ {`
	`1345`	`+ var returnTypeAtts = minfo.ReturnTypeCustomAttributes;`
	`1346`	`+ if (returnTypeAtts != null)`
	`1347`	`+ {`
	`1348`	`+ attributes.AddRange(returnTypeAtts.GetCustomAttributes(false).Cast<Attribute>());`
	`1349`	`+ }`
`1352`	`1350`	`}`
`1353`	`1351`
`1354`	`1352`	`foreach (var attr in attributes)`
`@@ -1400,6 +1398,10 @@ public FFIMethodAttributes(MethodBase method, Dictionary<MethodInfo, Attribute[]`
`1400`	`1398`	`{`
`1401`	`1399`	`AllowArrayPromotion = (attr as AllowArrayPromotionAttribute).IsAllowed;`
`1402`	`1400`	`}`
	`1401`	`+ else if(attr is ArbitraryDimensionArrayImportAttribute)`
	`1402`	`+ {`
	`1403`	`+ ArbitraryDimensionArrayImport = true;`
	`1404`	`+ }`
`1403`	`1405`	`}`
`1404`	`1406`	`}`
`1405`	`1407`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,5 +78,18 @@ public void EmptyDictionaryToString()`
`78`	`78`	`var dict = Dictionary.ByKeysValues(new string[0], new object[0]);`
`79`	`79`	`Assert.AreEqual(dict.ToString(), "{}");`
`80`	`80`	`}`
	`81`	`+`
	`82`	`+ [Test]`
	`83`	`+ public void Dictionary_ListOfValues_ReturnedAsExpected()`
	`84`	`+ {`
	`85`	`+ var dynFilePath = Path.Combine(TestDirectory, @"core\dictionary\Dictionary.Values2.dyn");`
	`86`	`+ OpenModel(dynFilePath);`
	`87`	`+`
	`88`	`+ var validationData1 = new List<string> { "foo" };`
	`89`	`+ var validationData2 = new object[] { 1, new[] { 1, 2 } };`
	`90`	`+`
	`91`	`+ AssertPreviewValue("f64c1972520144d7a6d342937584c47e", validationData2);`
	`92`	`+ AssertPreviewValue("bc957838571c4c56af2bb714f9f40bd7", validationData1);`
	`93`	`+ }`
`81`	`94`	`}`
`82`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,6 @@`
`1`	`1`	`using System;`
	`2`	`+using System.Collections.Generic;`
	`3`	`+using Autodesk.DesignScript.Runtime;`
`2`	`4`
`3`	`5`	`namespace FFITarget`
`4`	`6`	`{`
`@@ -50,5 +52,11 @@ public string Test2()`
`50`	`52`	`{`
`51`	`53`	`return "this node is obsolete.";`
`52`	`54`	`}`
	`55`	`+`
	`56`	`+ [return: ArbitraryDimensionArrayImport]`
	`57`	`+ public static IEnumerable<object> TestReturnAttribute()`
	`58`	`+ {`
	`59`	`+ return new object[] { 1.3, new double[] { 4.5, 7.8 } };`
	`60`	`+ }`
`53`	`61`	`}`
`54`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -193,5 +193,18 @@ public void TestToStringDoesNotThrow()`
`193`	`193`	`Assert.DoesNotThrow(() => { mirror.GetStringValue(x,this.core.Heap, 0); });`
`194`	`194`
`195`	`195`	`}`
	`196`	`+`
	`197`	`+ [Test]`
	`198`	`+ public void TestReturnArbitraryDimensionAttribute()`
	`199`	`+ {`
	`200`	`+ string code = @"`
	`201`	`+import (TestCSharpAttribute from ""FFITarget.dll"");`
	`202`	`+x = TestCSharpAttribute.TestReturnAttribute();`
	`203`	`+";`
	`204`	`+ var mirror = thisTest.RunScriptSource(code);`
	`205`	`+ var x = mirror.GetValue("x");`
	`206`	`+ thisTest.Verify("x", new object[] { 1.3, new double[] { 4.5, 7.8 } });`
	`207`	`+`
	`208`	`+ }`
`196`	`209`	`}`
`197`	`210`	`}`