fix(dotnet): fix deep type conversion across the process boundary, intelisense docs, set target to netcoreapp2.1 (#772)

This PR fixes various issues due to how we convert collection elements, and how the type reference is passed when converting.


These two issues are similar and are due to nested Arrays of object not being casted properly:
Fixes aws/aws-cdk#3244
Fixes aws/aws-cdk#3672
Added a unit test to cover the case

This issue is due to Maps of Arrays of Anys not being casted properly:
Fixes aws/aws-cdk#3813 
Added a unit test to cover the case

Also added support for xml documentation which is now added to the NuGet packages and should allow intellisense to provide in IDE help:
Fixes #749
Fixes aws/aws-cdk#1846

Migrated the target framework to netcoreapp2.1 instead of netstandard2.0:
Fixes #714

Author: Hamza Assyad

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
-        <TargetFramework>netcoreapp2.0</TargetFramework>
+        <TargetFramework>netcoreapp2.1</TargetFramework>
 
         <IsPackable>false</IsPackable>
     </PropertyGroup>

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime.UnitTests/Deputy/Converters/FrameworkToJsiiConverterTests.cs

@@ -450,6 +450,118 @@ public void RecursivelyConvertsArrayElements()
                     }
                 );
             }
+            
+            [Fact(DisplayName = _Prefix + nameof(RecursivelyConvertsMapElementsWithMapOfAny))]
+            public void RecursivelyConvertsMapElementsWithMapOfAny()
+            {
+                var instance = new OptionalValue(new TypeReference(
+                    collection: new CollectionTypeReference(CollectionKind.Map,
+                        new TypeReference(
+                            collection: new CollectionTypeReference(CollectionKind.Map,
+                                new TypeReference(primitive: PrimitiveType.Any)
+                            )
+                        )
+                    )
+                ));
+
+                var frameworkMap = new Dictionary<string, IDictionary<string, object>>
+                {
+                    { "myKey1", new Dictionary<string, object> { { "mySubKey1", "myValue1" } } },
+                    { "myKey2", new Dictionary<string, object> { { "mySubKey2", "myValue2" } } },
+                };
+
+                // This will test the call to FrameworkToJsiiConverter.TryConvertCollectionElement()
+                // In the case of a of a Map of Map of Any
+                bool success = _converter.TryConvert(instance, _referenceMap, frameworkMap, out object actual);
+
+                Assert.True(success);
+                Assert.IsType<JObject>(actual);
+                Assert.Collection
+                (
+                    ((IEnumerable<KeyValuePair<string, JToken>>)actual).OrderBy(kvp => kvp.Key),
+                    kvp =>
+                    {
+                        Assert.Equal("myKey1", kvp.Key);
+                        Assert.IsType<JObject>(kvp.Value);
+                        Assert.Collection
+                        (
+                            ((IEnumerable<KeyValuePair<string, JToken>>)kvp.Value),
+                            subKvp =>
+                            {
+                                Assert.Equal("mySubKey1", subKvp.Key, ignoreLineEndingDifferences: true);
+                                Assert.Equal("myValue1", subKvp.Value);
+                            }
+                        );
+                    },
+                    kvp =>
+                    {
+                        Assert.Equal("myKey2", kvp.Key, ignoreLineEndingDifferences: true);
+                        Assert.IsType<JObject>(kvp.Value);
+                        Assert.Collection
+                        (
+                            ((IEnumerable<KeyValuePair<string, JToken>>)kvp.Value),
+                            subKvp =>
+                            {
+                                Assert.Equal("mySubKey2", subKvp.Key, ignoreLineEndingDifferences: true);
+                                Assert.Equal("myValue2", subKvp.Value);
+                            }
+                        );
+                    }
+                );
+            }
+            
+            [Fact(DisplayName = _Prefix + nameof(RecursivelyConvertsMapElementsWithArrayOfAny))]
+            public void RecursivelyConvertsMapElementsWithArrayOfAny()
+            {
+                var instance = new OptionalValue(new TypeReference
+                (
+                    collection: new CollectionTypeReference(CollectionKind.Map,
+                        new TypeReference
+                        (
+                            collection: new CollectionTypeReference(CollectionKind.Array,
+                                new TypeReference(primitive: PrimitiveType.Any)
+                            )
+                        )
+                    )
+                ));
+
+                var frameworkArray = new Dictionary<string, object>()
+                {
+                    {"key", new [] { "true" }},
+                    {"key2", new [] { false }},
+                };
+
+                // This will test the call to FrameworkToJsiiConverter.TryConvertCollectionElement()
+                // In the case of a of a Map of Array of Any
+                bool success = _converter.TryConvert(instance, _referenceMap, frameworkArray, out object actual);
+
+                Assert.True(success);
+                Assert.IsType<JObject>(actual);
+                Assert.Collection
+                (
+                    ((IEnumerable<KeyValuePair<string, JToken>>)actual).OrderBy(kvp => kvp.Key),
+                    kvp =>
+                    {
+                        Assert.Equal("key", kvp.Key);
+                        Assert.IsType<JArray>(kvp.Value);
+                        Assert.Collection
+                        (
+                            (JArray)kvp.Value,
+                            subValue => Assert.Equal("true", subValue)
+                        );
+                    },
+                    kvp =>
+                    {
+                        Assert.Equal("key2", kvp.Key, ignoreLineEndingDifferences: true);
+                        Assert.IsType<JArray>(kvp.Value);
+                        Assert.Collection
+                        (
+                            (JArray)kvp.Value,
+                            subValue => Assert.Equal(false, subValue)
+                        );
+                    }
+                );
+            }
 
             [Fact(DisplayName = _Prefix + nameof(ConvertsNullMap))]
             public void ConvertsNullMap()

packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/Converters/FrameworkToJsiiConverter.cs

@@ -157,7 +157,7 @@ protected override bool TryConvertJson(object value, out object result)
                 return true;
             }
 
-            if (value.GetType().IsAssignableFrom(typeof(JObject)))
+            if (value.GetType().IsAssignableFrom(typeof(JObject)) || value.GetType().IsAssignableFrom(typeof(JArray)))
             {
                 result = value;
                 return true;
@@ -223,7 +223,7 @@ protected override bool TryConvertArray(IReferenceMap referenceMap, TypeReferenc
             JArray resultArray = new JArray();
             foreach (object element in array)
             {
-                if (!TryConvert(elementType, referenceMap, element, out object convertedElement))
+                if (!TryConvertCollectionElement(element, referenceMap, elementType, out object convertedElement))
                 {
                     result = null;
                     return false;
@@ -264,15 +264,7 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
             {
                 object element = indexer.GetValue(value, new object[] {key});
 
-                TypeReference childElementType = InferType(referenceMap, element);
-
-                // We should not pass the parent element type as we are in a map
-                // A map<string, object> could be a map<string, map<string, object> etc
-                // If we pass the parent referenceMap then it will try to convert it as Any
-                // So by inferring the child element type we are always converting the correct type.
-                // See https://github.com/aws/aws-cdk/issues/2496
-                
-                if (!TryConvert(childElementType, referenceMap, element, out object convertedElement))
+                if (!TryConvertCollectionElement(element, referenceMap, elementType, out object convertedElement))
                 {
                     result = null;
                     return false;
@@ -285,6 +277,47 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
             return true;
         }
 
+        /// <summary>
+        /// Converts a collection element
+        /// </summary>
+        /// <param name="element">The element to convert in the collection</param>
+        /// <param name="referenceMap">The known references map</param>
+        /// <param name="elementType">The TypeReference of the element, as seen by Jsii</param>
+        /// <param name="convertedElement">out: the converted element</param>
+        /// <returns>True if the conversion was successful, false otherwise</returns>
+        private bool TryConvertCollectionElement(object element, IReferenceMap referenceMap, TypeReference elementType,
+            out object convertedElement)
+        {
+            if (element is IDictionary<string, object> || element is object[])
+            {
+                var objectType = InferType(referenceMap, element);
+                var nestedType = elementType.Primitive == PrimitiveType.Any ? elementType : objectType.Collection.ElementType;
+                switch (objectType.Collection?.Kind)
+                {
+                    case CollectionKind.Map:
+                        // We should not pass the parent element type as we are
+                        // in a map<string, object> containing another map.
+                        // If we pass the parent elementType then it will try to convert it as Any
+                        // So we can directly convert to another map here, and forgo the type hierarchy
+                        // induced by elementType
+                        // See https://github.com/aws/aws-cdk/issues/2496
+                        return TryConvertMap(referenceMap, nestedType, element,
+                                out convertedElement);
+                    case CollectionKind.Array:
+                        // The [object] could be another array. (ie Tags)
+                        // https://github.com/aws/aws-cdk/issues/3244
+                        return TryConvertArray(referenceMap, nestedType, element,
+                                out convertedElement);
+                    default:
+                        return TryConvert(elementType, referenceMap, element, out convertedElement);
+                }
+            }
+            else
+            {
+                return TryConvert(elementType, referenceMap, element, out convertedElement);
+            }
+        }
+
         protected override TypeReference InferType(IReferenceMap referenceMap, object value)
         {
             value = value ?? throw new ArgumentNullException(nameof(value));
@@ -328,7 +361,7 @@ TypeReference InferType(IReferenceMap referenceMap, Type type)
                 return new TypeReference(primitive: PrimitiveType.Date);
             }
 
-            if (type.IsAssignableFrom(typeof(JObject)))
+            if (type.IsAssignableFrom(typeof(JObject)) || type.IsAssignableFrom(typeof(JArray)))
             {
                 return new TypeReference(primitive: PrimitiveType.Json);
             }

packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts

@@ -50,8 +50,9 @@ export class FileGenerator {
         const rootNode = xmlbuilder.create('Project', {encoding: 'UTF-8', headless: true});
         rootNode.att("Sdk", "Microsoft.NET.Sdk");
         const propertyGroup = rootNode.ele("PropertyGroup");
-        propertyGroup.ele("TargetFramework", "netstandard2.0");
+        propertyGroup.ele("TargetFramework", "netcoreapp2.1");
         propertyGroup.ele("GeneratePackageOnBuild", "true");
+        propertyGroup.ele("GenerateDocumentationFile", "true");
         propertyGroup.ele("IncludeSymbols", "true");
         propertyGroup.ele("IncludeSource", "true");
         propertyGroup.ele("PackageVersion", this.getDecoratedVersion(assembly));
@@ -77,7 +78,7 @@ export class FileGenerator {
         }
 
         if (dotnetInfo!.iconUrl != null) {
-            propertyGroup.ele("IconUrl", dotnetInfo!.iconUrl);
+            propertyGroup.ele("PackageIconUrl", dotnetInfo!.iconUrl);
         }
 
         const itemGroup1 = rootNode.ele("ItemGroup");

packages/jsii-pacmak/test/expected.jsii-calc-base/dotnet/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj

@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0</PackageVersion>

packages/jsii-pacmak/test/expected.jsii-calc-lib/dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId.csproj

@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0-devpreview</PackageVersion>

packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj

@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0</PackageVersion>