Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Preserve distinction between {} and object in compiled JSON #13597

Merged
merged 2 commits into from Mar 13, 2024
Merged

Preserve distinction between {} and object in compiled JSON #13597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f77e383
Preserve distinction between `{}` and `object` in compiled JSON
jeskew Mar 12, 2024
591bd06
Update baselines
jeskew Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
View file
Open in desktop
Expand Up @@ -5898,4 +5898,39 @@ public void Test_Issue13534()
("BCP036", DiagnosticLevel.Warning, """The property "gateway" expected a value of type "string" but the provided value is of type "object"."""),
});
}

// https://github.com/Azure/bicep/issues/13556
[TestMethod]
public void Distinction_between_empty_and_untyped_objects_should_survive_compilation_to_JSON()
{
var originalTypesTemplate = """
@export()
type emptyObject = {}

@export()
type untypedObject = object
""";

var compiledTypesTemplate = CompilationHelper.Compile(originalTypesTemplate).Template;
compiledTypesTemplate.Should().NotBeNull();

var result = CompilationHelper.Compile(
("types.bicep", originalTypesTemplate),
("types.json", compiledTypesTemplate!.ToString()),
("main.bicep", """
import * as fromBicep from 'types.bicep'
import * as fromJson from 'types.json'

param a fromBicep.emptyObject = {fizz: 'buzz'}
param b fromBicep.untypedObject = {foo: 'bar'}
param c fromJson.emptyObject = {snap: 'crackle'}
param d fromJson.untypedObject = {wishy: 'washy'}
"""));

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP037", DiagnosticLevel.Warning, """The property "fizz" is not allowed on objects of type "{ }". No other properties are allowed."""),
("BCP037", DiagnosticLevel.Warning, """The property "snap" is not allowed on objects of type "{ }". No other properties are allowed."""),
});
}
}
12 changes: 12 additions & 0 deletions src/Bicep.Core.IntegrationTests/UserDefinedTypeTests.cs
View file
Open in desktop
Expand Up @@ -450,6 +450,18 @@ public void Warning_should_be_shown_when_setting_unknown_properties_on_unsealed_
});
}

[TestMethod]
public void Empty_object_should_be_distinguishable_from_untyped_object_in_compiled_JSON()
{
var result = CompilationHelper.Compile("""
type emptyObject = {}
type untypedObect = object
""");

result.Template.Should().HaveValueAtPath("definitions.emptyObject.properties", new JObject());
result.Template.Should().NotHaveValueAtPath("definitions.untypedObject.properties");
}

[TestMethod]
public void Error_should_be_shown_when_setting_unknown_properties_that_do_not_match_additional_properties_type()
{
Expand Down
5 changes: 4 additions & 1 deletion src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.json
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "7210691896706139249"
"templateHash": "8162009081861809153"
}
},
"definitions": {
Expand Down Expand Up @@ -194,6 +194,7 @@
},
"stringStringDictionary": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "string"
}
Expand Down Expand Up @@ -665,6 +666,7 @@
},
"discriminatedUnionInlineAdditionalProps1": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "object",
"discriminator": {
Expand All @@ -682,6 +684,7 @@
},
"discriminatedUnionInlineAdditionalProps2": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "object",
"discriminator": {
Expand Down
3 changes: 3 additions & 0 deletions src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.sourcemap.bicep
View file
Open in desktop
Expand Up @@ -305,6 +305,7 @@ type tupleSecondItem = tuple[1]
type stringStringDictionary = {
//@ "stringStringDictionary": {
//@ "type": "object",
//@ "properties": {},
//@ },
*: string
//@ "additionalProperties": {
Expand Down Expand Up @@ -903,6 +904,7 @@ type discriminatorUnionAsPropertyType = {
type discriminatedUnionInlineAdditionalProps1 = {
//@ "discriminatedUnionInlineAdditionalProps1": {
//@ "type": "object",
//@ "properties": {},
//@ },
@discriminator('type')
*: typeA | typeB
Expand All @@ -925,6 +927,7 @@ type discriminatedUnionInlineAdditionalProps1 = {
type discriminatedUnionInlineAdditionalProps2 = {
//@ "discriminatedUnionInlineAdditionalProps2": {
//@ "type": "object",
//@ "properties": {},
//@ },
@discriminator('type')
*: (typeA | typeB)?
Expand Down
5 changes: 4 additions & 1 deletion src/Bicep.Core.Samples/Files/baselines/TypeDeclarations_LF/main.symbolicnames.json
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "7210691896706139249"
"templateHash": "8162009081861809153"
}
},
"definitions": {
Expand Down Expand Up @@ -194,6 +194,7 @@
},
"stringStringDictionary": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "string"
}
Expand Down Expand Up @@ -665,6 +666,7 @@
},
"discriminatedUnionInlineAdditionalProps1": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "object",
"discriminator": {
Expand All @@ -682,6 +684,7 @@
},
"discriminatedUnionInlineAdditionalProps2": {
"type": "object",
"properties": {},
"additionalProperties": {
"type": "object",
"discriminator": {
Expand Down
24 changes: 24 additions & 0 deletions src/Bicep.Core.UnitTests/Semantics/ArmTemplateSemanticModelTests.cs
View file
Open in desktop
Expand Up @@ -514,6 +514,30 @@ public void Resource_derivation_metadata_causes_tagged_union_variant_to_be_loade
fallbackType.Properties["name"].TypeReference.Type.Should().Be(TypeFactory.CreateStringLiteralType("default"));
}

[TestMethod]
public void Empty_properties_constraint_causes_loaded_object_to_use_FallbackProperty_flag_on_additional_properties()
{
var parameterType = GetLoadedParameterType("""
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"languageVersion": "2.0",
"resources": {},
"parameters": {
"emptyObject": {
"type": "object",
"properties": {}
}
}
}
""",
"emptyObject");

var objectParameterType = parameterType.Should().BeAssignableTo<ObjectType>().Subject;
objectParameterType.AdditionalPropertiesType.Should().Be(LanguageConstants.Any);
objectParameterType.AdditionalPropertiesFlags.Should().HaveFlag(TypePropertyFlags.FallbackProperty);
}

private static TypeSymbol GetLoadedParameterType(string jsonTemplate, string parameterName)
{
var model = LoadModel(jsonTemplate);
Expand Down
5 changes: 1 addition & 4 deletions src/Bicep.Core/Emit/TemplateWriter.cs
View file
Open in desktop
Expand Up @@ -745,10 +745,7 @@ private ObjectExpression GetTypePropertiesForObjectType(ObjectTypeExpression exp
propertySchemata.Add(ExpressionFactory.CreateObjectProperty(property.PropertyName, propertySchema, property.SourceSyntax));
}

if (propertySchemata.Any())
{
properties.Add(ExpressionFactory.CreateObjectProperty("properties", ExpressionFactory.CreateObject(propertySchemata, expression.SourceSyntax)));
}
properties.Add(ExpressionFactory.CreateObjectProperty("properties", ExpressionFactory.CreateObject(propertySchemata, expression.SourceSyntax)));

if (expression.AdditionalPropertiesExpression is { } addlPropsType)
{
Expand Down
44 changes: 24 additions & 20 deletions src/Bicep.Core/TypeSystem/ArmTemplateTypeLoader.cs
View file
Open in desktop
Expand Up @@ -243,38 +243,42 @@ private static TypeSymbol GetObjectType(SchemaValidationContext context, ITempla
return new DiscriminatedObjectType(string.Join(" | ", TypeHelper.GetOrderedTypeNames(variants)), flags, discriminator.PropertyName.Value, variants);
}

return GetObjectType(context, schemaNode.Properties.CoalesceEnumerable(), schemaNode.Required?.Value, schemaNode.AdditionalProperties, flags);
return GetObjectType(context, schemaNode.Properties, schemaNode.Required?.Value, schemaNode.AdditionalProperties, flags);
}

private static TypeSymbol GetObjectType(SchemaValidationContext context,
IEnumerable<KeyValuePair<string, TemplateTypeDefinition>> properties,
IEnumerable<KeyValuePair<string, TemplateTypeDefinition>>? properties,
IEnumerable<string>? requiredProperties,
TemplateBooleanOrSchemaNode? additionalProperties,
TypeSymbolValidationFlags flags)
{
var requiredProps = requiredProperties is not null ? ImmutableHashSet.CreateRange(requiredProperties) : null;

ObjectTypeNameBuilder nameBuilder = new();
List<TypeProperty> propertyList = new();
List<TypeProperty>? propertyList = null;
ITypeReference? additionalPropertiesType = LanguageConstants.Any;
TypePropertyFlags additionalPropertiesFlags = TypePropertyFlags.FallbackProperty;

foreach (var (propertyName, schema) in properties)
if (properties is not null)
{
// depending on the language version, either only properties included in schemaNode.Required are required,
// or all of them are (but some may be nullable)
var required = context.TemplateLanguageVersion?.HasFeature(TemplateLanguageFeature.NullableParameters) == true
|| (requiredProps?.Contains(propertyName) ?? false);
var propertyFlags = required ? TypePropertyFlags.Required : TypePropertyFlags.None;
var description = schema.Metadata?.Value is JObject metadataObject &&
metadataObject.TryGetValue(LanguageConstants.MetadataDescriptionPropertyName, out var descriptionToken) &&
descriptionToken is JValue { Value: string descriptionString }
? descriptionString
: null;

var (type, typeName) = GetDeferrableTypeInfo(context, schema);
propertyList.Add(new(propertyName, type, propertyFlags, description));
nameBuilder.AppendProperty(propertyName, typeName);
propertyList = new();
foreach (var (propertyName, schema) in properties)
{
// depending on the language version, either only properties included in schemaNode.Required are required,
// or all of them are (but some may be nullable)
var required = context.TemplateLanguageVersion?.HasFeature(TemplateLanguageFeature.NullableParameters) == true
|| (requiredProps?.Contains(propertyName) ?? false);
var propertyFlags = required ? TypePropertyFlags.Required : TypePropertyFlags.None;
var description = schema.Metadata?.Value is JObject metadataObject &&
metadataObject.TryGetValue(LanguageConstants.MetadataDescriptionPropertyName, out var descriptionToken) &&
descriptionToken is JValue { Value: string descriptionString }
? descriptionString
: null;

var (type, typeName) = GetDeferrableTypeInfo(context, schema);
propertyList.Add(new(propertyName, type, propertyFlags, description));
nameBuilder.AppendProperty(propertyName, typeName);
}
}

if (additionalProperties is not null)
Expand All @@ -293,12 +297,12 @@ private static TypeSymbol GetObjectType(SchemaValidationContext context, ITempla
}
}

if (propertyList.Count == 0 && additionalProperties is null)
if (propertyList is null && additionalProperties is null)
{
return flags.HasFlag(TypeSymbolValidationFlags.IsSecure) ? LanguageConstants.SecureObject : LanguageConstants.Object;
}

return new ObjectType(nameBuilder.ToString(), flags, propertyList, additionalPropertiesType, additionalPropertiesFlags);
return new ObjectType(nameBuilder.ToString(), flags, propertyList.CoalesceEnumerable(), additionalPropertiesType, additionalPropertiesFlags);
}

private class SansMetadata : ITemplateSchemaNode
Expand Down