From 23d57b29d22eebb356506360eeb0d37d12f7b709 Mon Sep 17 00:00:00 2001 From: Jonathan Eskew Date: Wed, 27 Mar 2024 13:04:12 -0400 Subject: [PATCH 1/2] Allow more flexibility in what can be mixed in to a user-defined tagged union --- ...serDefinedDiscriminatedObjectUnionTests.cs | 130 ++++++++++++++++ .../ResourceDerivedTypeResolverTests.cs | 19 ++- src/Bicep.Core/Emit/TemplateWriter.cs | 142 ++++++++++++------ .../TypeSystem/ResourceDerivedTypeResolver.cs | 6 + 4 files changed, 246 insertions(+), 51 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/UserDefinedDiscriminatedObjectUnionTests.cs b/src/Bicep.Core.IntegrationTests/UserDefinedDiscriminatedObjectUnionTests.cs index 179b4200af6..c21b23d8e81 100644 --- a/src/Bicep.Core.IntegrationTests/UserDefinedDiscriminatedObjectUnionTests.cs +++ b/src/Bicep.Core.IntegrationTests/UserDefinedDiscriminatedObjectUnionTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using Bicep.Core.Diagnostics; +using Bicep.Core.UnitTests; using Bicep.Core.UnitTests.Assertions; using Bicep.Core.UnitTests.Utils; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -643,5 +644,134 @@ public void DiscriminatedObjectUnions_SelfCycle_Inline_Optional() } """)); } + + [TestMethod] + public void Issue_13661() + { + var result = CompilationHelper.Compile( + ("main.bicep", """ + import * as t1 from 'app1.spec.bicep' + import * as t2 from 'app2.spec.bicep' + + @discriminator('Name') + type ApiDef = (t1.Api1Def | t2.Api2Def)? + + param Name string + param APIs ApiDef[] + + output test string = '${Name}-${APIs}' + """), + ("app1.spec.bicep", """ + @export() + type Api1Def = { + Name: 'Api1' + Settings: { + CustomSetting1: string + CustomSetting2: string + } + } + """), + ("app2.spec.bicep", """ + @export() + type Api2Def = { + Name: 'Api2' + Settings: { + CustomSetting3: string + } + } + """)); + + result.Template.Should().NotBeNull(); + result.Template.Should().HaveJsonAtPath("definitions.ApiDef.discriminator", """ + { + "propertyName": "Name", + "mapping": { + "Api1": { + "$ref": "#/definitions/_1.Api1Def" + }, + "Api2": { + "$ref": "#/definitions/_2.Api2Def" + } + } + } + """); + } + + [TestMethod] + public void User_defined_discriminated_objects_can_amend_resource_derived_discriminated_unions() + { + var result = CompilationHelper.Compile( + new ServiceBuilder().WithFeatureOverrides(new(TestContext, ResourceDerivedTypesEnabled: true)), + """ + @discriminator('computeType') + type taggedUnion = resource<'Microsoft.MachineLearningServices/workspaces/computes@2020-04-01'>.properties + | { computeType: 'foo', bar: string } + """); + + result.Template.Should().NotBeNull(); + result.Template.Should().HaveJsonAtPath("definitions.taggedUnion.discriminator", """ + { + "propertyName": "computeType", + "mapping": { + "DataFactory": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/DataFactory" + } + }, + "Databricks": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/Databricks" + } + }, + "VirtualMachine": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/VirtualMachine" + } + }, + "AmlCompute": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/AmlCompute" + } + }, + "AKS": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/AKS" + } + }, + "HDInsight": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/HDInsight" + } + }, + "DataLakeAnalytics": { + "type": "object", + "metadata": { + "__bicep_resource_derived_type!": "Microsoft.MachineLearningServices/workspaces/computes@2020-04-01#properties/properties/discriminator/mapping/DataLakeAnalytics" + } + }, + "foo": { + "type": "object", + "properties": { + "computeType": { + "type": "string", + "allowedValues": [ + "foo" + ] + }, + "bar": { + "type": "string" + } + } + } + } + } + """); + } } } diff --git a/src/Bicep.Core.UnitTests/TypeSystem/ResourceDerivedTypeResolverTests.cs b/src/Bicep.Core.UnitTests/TypeSystem/ResourceDerivedTypeResolverTests.cs index 126cb0b85bf..fe731bde7c5 100644 --- a/src/Bicep.Core.UnitTests/TypeSystem/ResourceDerivedTypeResolverTests.cs +++ b/src/Bicep.Core.UnitTests/TypeSystem/ResourceDerivedTypeResolverTests.cs @@ -292,13 +292,28 @@ public void Supports_pointers_to_partial_resource_body_types() ImmutableArray.Create(new ObjectType("dictionary", TypeSymbolValidationFlags.Default, ImmutableArray.Empty, - TypeFactory.CreateArrayType(targetType))), + TypeFactory.CreateArrayType(new DiscriminatedObjectType("taggedUnion", + TypeSymbolValidationFlags.Default, + "type", + ImmutableArray.Create( + new ObjectType("fooVariant", + TypeSymbolValidationFlags.Default, + ImmutableArray.Create( + new TypeProperty("type", TypeFactory.CreateStringLiteralType("foo")), + new TypeProperty("property", LanguageConstants.Int)), + null), + new ObjectType("barVariant", + TypeSymbolValidationFlags.Default, + ImmutableArray.Create( + new TypeProperty("type", TypeFactory.CreateStringLiteralType("bar")), + new TypeProperty("property", targetType)), + null)))))), TypeSymbolValidationFlags.Default))), null); var (sut, unhydratedTypeRef) = SetupResolver(hydrated); UnresolvedResourceDerivedType unresolved = new(unhydratedTypeRef, - ImmutableArray.Create("properties", "property", "prefixItems", "0", "additionalProperties", "items"), + ImmutableArray.Create("properties", "property", "prefixItems", "0", "additionalProperties", "items", "discriminator", "mapping", "bar", "properties", "property"), LanguageConstants.Any); var bound = sut.ResolveResourceDerivedTypes(unresolved); diff --git a/src/Bicep.Core/Emit/TemplateWriter.cs b/src/Bicep.Core/Emit/TemplateWriter.cs index ea452e0cec6..0b70dda0927 100644 --- a/src/Bicep.Core/Emit/TemplateWriter.cs +++ b/src/Bicep.Core/Emit/TemplateWriter.cs @@ -454,6 +454,9 @@ ImmutableArray SegmentsForIndex(long index) ImmutableArray SegmentsForItems() => PointerSegments.Add("items"); + + ImmutableArray SegmentsForVariant(string discriminatorValue) + => PointerSegments.AddRange("discriminator", "mapping", discriminatorValue); } private record ResourceDerivedTypeResolution(ResourceTypeReference RootResourceTypeReference, ImmutableArray PointerSegments, TypeSymbol DerivedType) : ITypeReferenceExpressionResolution @@ -827,67 +830,108 @@ private IEnumerable GetDiscriminatedUnionMappingEntrie foreach (var memberExpression in discriminatedObjectTypeExpr.MemberExpressions) { - var resolvedMemberExpression = memberExpression; - TypeExpression? typeAliasReferenceExpr = null; - if (memberExpression is TypeAliasReferenceExpression memberTypeAliasExpr - && declaredTypesByName.TryGetValue(memberTypeAliasExpr.Symbol.Name, out var declaredTypeExpression)) + var mappingsEnumerable = memberExpression switch { - resolvedMemberExpression = declaredTypeExpression.Value; - typeAliasReferenceExpr = memberTypeAliasExpr; - } + TypeAliasReferenceExpression or + SynthesizedTypeAliasReferenceExpression or + ImportedTypeReferenceExpression or + WildcardImportTypePropertyReferenceExpression or + ITypeReferenceAccessExpression => ResolveTypeReferenceExpression(memberExpression) switch + { + ResolvedInternalReference udtRef => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, udtRef.Declaration, udtRef), + ResourceDerivedTypeResolution rdtRef => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, rdtRef, memberExpression.SourceSyntax), + _ => throw new UnreachableException(), + }, + _ => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, memberExpression, null), + }; - if (memberExpression is SynthesizedTypeAliasReferenceExpression memberSynthesizedTypeAliasExpr - && declaredTypesByName.TryGetValue(memberSynthesizedTypeAliasExpr.Name, out declaredTypeExpression)) + foreach (var mapping in mappingsEnumerable) { - resolvedMemberExpression = declaredTypeExpression.Value; - typeAliasReferenceExpr = memberSynthesizedTypeAliasExpr; + yield return mapping; } + } + } - if (resolvedMemberExpression is ObjectTypeExpression objectUnionMemberExpr) - { - var memberObjectType = objectUnionMemberExpr.ExpressedObjectType; - - if (!memberObjectType.Properties.TryGetValue(discriminatorPropertyName, out var discriminatorTypeProperty) - || discriminatorTypeProperty.TypeReference.Type is not StringLiteralType discriminatorStringLiteral) - { - // This should have been caught during type checking - throw new ArgumentException("Invalid discriminated union type encountered during serialization."); - } - - ObjectExpression objectExpression; - - if (typeAliasReferenceExpr != null) - { - objectExpression = TypePropertiesForTypeExpression(typeAliasReferenceExpr); - } - else - { - objectExpression = GetTypePropertiesForObjectType(objectUnionMemberExpr); - } + private IEnumerable GetDiscriminatedUnionMappingEntries( + string discriminatorPropertyName, + TypeExpression memberDeclaration, + ITypeReferenceExpressionResolution? memberResolution) + { + if (memberDeclaration is ObjectTypeExpression objectUnionMemberExpr) + { + yield return ExpressionFactory.CreateObjectProperty( + DiscriminatorValue(discriminatorPropertyName, objectUnionMemberExpr.ExpressedObjectType), + memberResolution?.GetTypePropertiesForResolvedReferenceExpression(memberDeclaration.SourceSyntax) + ?? GetTypePropertiesForObjectType(objectUnionMemberExpr), + memberDeclaration.SourceSyntax); + } + else if (memberDeclaration is DiscriminatedObjectTypeExpression nestedDiscriminatedMemberExpr) + { + var nestedDiscriminatorPropertyName = nestedDiscriminatedMemberExpr.ExpressedDiscriminatedObjectType.DiscriminatorProperty.Name; - yield return ExpressionFactory.CreateObjectProperty(discriminatorStringLiteral.RawStringValue, objectExpression); - } - else if (resolvedMemberExpression is DiscriminatedObjectTypeExpression nestedDiscriminatedMemberExpr) + if (nestedDiscriminatorPropertyName != discriminatorPropertyName) { - var nestedDiscriminatorPropertyName = nestedDiscriminatedMemberExpr.ExpressedDiscriminatedObjectType.DiscriminatorProperty.Name; - - if (nestedDiscriminatorPropertyName != discriminatorPropertyName) - { - // This should have been caught during type checking - throw new ArgumentException("Invalid discriminated union type encountered during serialization."); - } + // This should have been caught during type checking + throw new ArgumentException("Invalid discriminated union type encountered during serialization."); + } - foreach (var nestedPropertyExpr in GetDiscriminatedUnionMappingEntries(nestedDiscriminatedMemberExpr)) - { - yield return nestedPropertyExpr; - } + foreach (var nestedPropertyExpr in GetDiscriminatedUnionMappingEntries(nestedDiscriminatedMemberExpr)) + { + yield return nestedPropertyExpr; } - else + } + else + { + // This should have been caught during type checking + throw new ArgumentException("Invalid discriminated union type encountered during serialization."); + } + } + + private IEnumerable GetDiscriminatedUnionMappingEntries( + string discriminatorPropertyName, + ResourceDerivedTypeResolution resolution, + SyntaxBase? sourceSyntax) + { + if (resolution.DerivedType is ObjectType @object) + { + yield return ExpressionFactory.CreateObjectProperty( + DiscriminatorValue(discriminatorPropertyName, @object), + resolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax), + sourceSyntax); + } + else if (resolution.DerivedType is DiscriminatedObjectType discriminatedObject) + { + foreach (var variant in discriminatedObject.UnionMembersByKey.Values) { - // This should have been caught during type checking - throw new ArgumentException("Invalid discriminated union type encountered during serialization."); + var discriminatorValue = DiscriminatorValue(discriminatorPropertyName, variant); + var variantResolution = new ResourceDerivedTypeResolution( + resolution.RootResourceTypeReference, + (resolution as ITypeReferenceExpressionResolution).SegmentsForVariant(discriminatorValue), + variant); + + yield return ExpressionFactory.CreateObjectProperty( + discriminatorValue, + variantResolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax), + sourceSyntax); } } + else + { + // This should have been caught during type checking + throw new ArgumentException("Invalid discriminated union type encountered during serialization."); + } + } + + private static string DiscriminatorValue(string discriminatorPropertyName, ObjectType @object) + { + if (!@object.Properties.TryGetValue(discriminatorPropertyName, out var discriminatorTypeProperty) + || discriminatorTypeProperty.TypeReference.Type is not StringLiteralType discriminatorStringLiteral) + { + // This should have been caught during type checking + throw new ArgumentException("Invalid discriminated union type encountered during serialization."); + } + + return discriminatorStringLiteral.RawStringValue; } private static Expression ToLiteralValue(ITypeReference literalType) => literalType.Type switch diff --git a/src/Bicep.Core/TypeSystem/ResourceDerivedTypeResolver.cs b/src/Bicep.Core/TypeSystem/ResourceDerivedTypeResolver.cs index e3bf4462b95..ce1043cb3b5 100644 --- a/src/Bicep.Core/TypeSystem/ResourceDerivedTypeResolver.cs +++ b/src/Bicep.Core/TypeSystem/ResourceDerivedTypeResolver.cs @@ -4,6 +4,7 @@ using System.Collections.Concurrent; using System.Collections.Immutable; using System.Diagnostics; +using Bicep.Core.Parsing; using Bicep.Core.Semantics; using Bicep.Core.Semantics.Namespaces; using Bicep.Core.TypeSystem.Types; @@ -77,6 +78,11 @@ private TypeSymbol ResolveType(IUnresolvedResourceDerivedType unresolved) @object.AdditionalPropertiesType is not null: current = @object.AdditionalPropertiesType.Type; continue; + case "discriminator" when current is DiscriminatedObjectType discriminatedObject && + unresolved.PointerSegments[++i].Equals("mapping", StringComparison.OrdinalIgnoreCase) && + discriminatedObject.UnionMembersByKey.TryGetValue(StringUtils.EscapeBicepString(unresolved.PointerSegments[++i]), out var variant): + current = variant; + continue; case "prefixitems" when current is TupleType tuple && int.TryParse(unresolved.PointerSegments[++i], out int index) && 0 <= index && From e2b37ee2d66ecbe1841c4dba5d58e8a9a4bc7f55 Mon Sep 17 00:00:00 2001 From: Jonathan Eskew Date: Wed, 27 Mar 2024 13:45:21 -0400 Subject: [PATCH 2/2] Regenerate baselines --- .../TypeDeclarations_LF/main.sourcemap.bicep | 36 +++++++++---------- src/Bicep.Core/Emit/TemplateWriter.cs | 18 ++++------ 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.sourcemap.bicep b/src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.sourcemap.bicep index efc1e67760d..0006414f769 100644 --- a/src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.sourcemap.bicep +++ b/src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.sourcemap.bicep @@ -510,24 +510,6 @@ type discriminatedUnion1 = typeA | typeB //@ } //@ } //@ }, -//@ "a": { -//@ "$ref": "#/definitions/typeA" -//@ }, -//@ "b": { -//@ "$ref": "#/definitions/typeB" -//@ }, -//@ "a": { -//@ "$ref": "#/definitions/typeA" -//@ }, -//@ "b": { -//@ "$ref": "#/definitions/typeB" -//@ }, -//@ "a": { -//@ "$ref": "#/definitions/typeA" -//@ }, -//@ "b": { -//@ "$ref": "#/definitions/typeB" -//@ }, @discriminator('type') type discriminatedUnion2 = { type: 'c', value: string } | { type: 'd', value: bool } @@ -659,6 +641,12 @@ type discriminatedUnion3 = discriminatedUnion1 | discriminatedUnion2 | { type: ' //@ "discriminator": { //@ "propertyName": "type", //@ "mapping": { +//@ "a": { +//@ "$ref": "#/definitions/typeA" +//@ }, +//@ "b": { +//@ "$ref": "#/definitions/typeB" +//@ }, //@ "e": { //@ "type": "object", //@ "properties": { @@ -684,6 +672,12 @@ type discriminatedUnion4 = discriminatedUnion1 | (discriminatedUnion2 | typeE) //@ "discriminator": { //@ "propertyName": "type", //@ "mapping": { +//@ "a": { +//@ "$ref": "#/definitions/typeA" +//@ }, +//@ "b": { +//@ "$ref": "#/definitions/typeB" +//@ }, //@ "e": { //@ "$ref": "#/definitions/typeE" //@ } @@ -855,6 +849,12 @@ type inlineDiscriminatedUnion3 = { //@ "discriminator": { //@ "propertyName": "type", //@ "mapping": { +//@ "a": { +//@ "$ref": "#/definitions/typeA" +//@ }, +//@ "b": { +//@ "$ref": "#/definitions/typeB" +//@ }, //@ } //@ } //@ } diff --git a/src/Bicep.Core/Emit/TemplateWriter.cs b/src/Bicep.Core/Emit/TemplateWriter.cs index 0b70dda0927..e000b6424d7 100644 --- a/src/Bicep.Core/Emit/TemplateWriter.cs +++ b/src/Bicep.Core/Emit/TemplateWriter.cs @@ -839,7 +839,7 @@ WildcardImportTypePropertyReferenceExpression or ITypeReferenceAccessExpression => ResolveTypeReferenceExpression(memberExpression) switch { ResolvedInternalReference udtRef => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, udtRef.Declaration, udtRef), - ResourceDerivedTypeResolution rdtRef => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, rdtRef, memberExpression.SourceSyntax), + ResourceDerivedTypeResolution rdtRef => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, rdtRef), _ => throw new UnreachableException(), }, _ => GetDiscriminatedUnionMappingEntries(discriminatorPropertyName, memberExpression, null), @@ -861,9 +861,8 @@ WildcardImportTypePropertyReferenceExpression or { yield return ExpressionFactory.CreateObjectProperty( DiscriminatorValue(discriminatorPropertyName, objectUnionMemberExpr.ExpressedObjectType), - memberResolution?.GetTypePropertiesForResolvedReferenceExpression(memberDeclaration.SourceSyntax) - ?? GetTypePropertiesForObjectType(objectUnionMemberExpr), - memberDeclaration.SourceSyntax); + memberResolution?.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax: null) + ?? GetTypePropertiesForObjectType(objectUnionMemberExpr)); } else if (memberDeclaration is DiscriminatedObjectTypeExpression nestedDiscriminatedMemberExpr) { @@ -887,17 +886,15 @@ WildcardImportTypePropertyReferenceExpression or } } - private IEnumerable GetDiscriminatedUnionMappingEntries( + private static IEnumerable GetDiscriminatedUnionMappingEntries( string discriminatorPropertyName, - ResourceDerivedTypeResolution resolution, - SyntaxBase? sourceSyntax) + ResourceDerivedTypeResolution resolution) { if (resolution.DerivedType is ObjectType @object) { yield return ExpressionFactory.CreateObjectProperty( DiscriminatorValue(discriminatorPropertyName, @object), - resolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax), - sourceSyntax); + resolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax: null)); } else if (resolution.DerivedType is DiscriminatedObjectType discriminatedObject) { @@ -911,8 +908,7 @@ WildcardImportTypePropertyReferenceExpression or yield return ExpressionFactory.CreateObjectProperty( discriminatorValue, - variantResolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax), - sourceSyntax); + variantResolution.GetTypePropertiesForResolvedReferenceExpression(sourceSyntax: null)); } } else