Skip to content

Commit

Permalink
Fix issue #4212 and #4382 (#4384)
Browse files Browse the repository at this point in the history
  • Loading branch information
shenglol authored Sep 14, 2021
1 parent eae6b03 commit de8b847
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ public void Parent_property_self_cycles_are_blocked()
{
var (template, diags, _) = CompilationHelper.Compile(@"
resource vmExt 'Microsoft.Compute/virtualMachines/extensions@2020-06-01' = {
name: 'vmExt'
parent: vmExt
location: 'eastus'
}
Expand Down
35 changes: 33 additions & 2 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ param zonesEnabled bool
("BCP036", DiagnosticLevel.Error, "The property \"name\" expected a value of type \"string\" but the provided value is of type \"null\"."),
("BCP036", DiagnosticLevel.Error, "The property \"scope\" expected a value of type \"resource | tenant\" but the provided value is of type \"null\"."),
("BCP036", DiagnosticLevel.Error, "The property \"name\" expected a value of type \"string\" but the provided value is of type \"null\"."),
("BCP036", DiagnosticLevel.Error, "The property \"parent\" expected a value of type \"resource\" but the provided value is of type \"null\"."),
("BCP036", DiagnosticLevel.Error, "The property \"parent\" expected a value of type \"Microsoft.Network/dnsZones\" but the provided value is of type \"null\"."),
});
}

Expand Down Expand Up @@ -2500,5 +2500,36 @@ public void Test_Issue4156()
["accessKey2"] = "[listKeys(resourceId('Microsoft.EventGrid/topics', format('{0}-ZZZ', variables('topics')[copyIndex()])), '2021-06-01-preview').key1]"
});
}

[TestMethod]
// https://github.com/Azure/bicep/issues/4212
public void Test_Issue4212()
{
var result = CompilationHelper.Compile(
("main.bicep", @"
module mod 'mod.bicep' = {
name: 'mod'
}
resource res 'Microsoft.Network/virtualNetworks/subnets@2020-11-01' existing = {
name: 'abc/def'
parent: mod
}
resource res2 'Microsoft.Network/virtualNetworks/subnets@2020-11-01' existing = {
name: 'res2'
parent: tenant()
}
output test string = res.id
"),
("mod.bicep", ""));

result.Should().HaveDiagnostics(new[]
{
("BCP036", DiagnosticLevel.Error, "The property \"parent\" expected a value of type \"Microsoft.Network/virtualNetworks\" but the provided value is of type \"module\"."),
("BCP036", DiagnosticLevel.Error, "The property \"parent\" expected a value of type \"Microsoft.Network/virtualNetworks\" but the provided value is of type \"tenant\"."),
});
}
}
}
}
10 changes: 9 additions & 1 deletion src/Bicep.Core/Semantics/Metadata/ResourceMetadataCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Immutable;
using System.Runtime.CompilerServices;
using Bicep.Core.Syntax;

namespace Bicep.Core.Semantics.Metadata
Expand All @@ -23,13 +24,15 @@ public ResourceMetadataCache(SemanticModel semanticModel)

protected override ResourceMetadata? Calculate(SyntaxBase syntax)
{
RuntimeHelpers.EnsureSufficientExecutionStack();

switch (syntax)
{
case ResourceAccessSyntax _:
case VariableAccessSyntax _:
{
var symbol = semanticModel.GetSymbolInfo(syntax);
if (symbol is DeclaredSymbol declaredSymbol)
if (symbol is DeclaredSymbol declaredSymbol && semanticModel.Binder.TryGetCycle(declaredSymbol) is null)
{
return this.TryLookup(declaredSymbol.DeclaringSyntax);
}
Expand All @@ -46,6 +49,11 @@ public ResourceMetadataCache(SemanticModel semanticModel)
break;
}

if (semanticModel.Binder.TryGetCycle(symbol) is not null)
{
break;
}

if (semanticModel.Binder.GetNearestAncestor<ResourceDeclarationSyntax>(syntax) is {} nestedParentSyntax)
{
// nested resource parent syntax
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/TypeSystem/Az/AzResourceTypeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ static TypeProperty UpdateFlags(TypeProperty typeProperty, TypePropertyFlags fla
// add the 'parent' property for child resource types that are not nested inside a parent resource
if (!typeReference.IsRootType && !flags.HasFlag(ResourceTypeGenerationFlags.NestedResource))
{
var parentType = LanguageConstants.CreateResourceScopeReference(ResourceScope.Resource);
var parentType = new ResourceParentType(typeReference);
var parentFlags = TypePropertyFlags.WriteOnly | TypePropertyFlags.DeployTimeConstant | TypePropertyFlags.DisallowAny | TypePropertyFlags.LoopVariant;

properties = properties.SetItem(LanguageConstants.ResourceParentPropertyName, new TypeProperty(LanguageConstants.ResourceParentPropertyName, parentType, parentFlags));
Expand Down
25 changes: 25 additions & 0 deletions src/Bicep.Core/TypeSystem/ResourceParentType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Linq;
using Bicep.Core.Extensions;
using Bicep.Core.Resources;

namespace Bicep.Core.TypeSystem
{
public class ResourceParentType : TypeSymbol
{
public ResourceParentType(ResourceTypeReference childTypeReference)
: base(GetFullyQualifiedParentTypeName(childTypeReference))
{
this.ChildTypeReference = childTypeReference;
}

public ResourceTypeReference ChildTypeReference { get; }

public override TypeKind TypeKind => TypeKind.Resource;

private static string GetFullyQualifiedParentTypeName(ResourceTypeReference childTypeReference) =>
$"{childTypeReference.Namespace}/{childTypeReference.Types.Take(childTypeReference.Types.Length - 1).ConcatString("/")}";
}
}
38 changes: 21 additions & 17 deletions src/Bicep.Core/TypeSystem/TypeValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,72 +86,76 @@ public static bool AreTypesAssignable(TypeSymbol sourceType, TypeSymbol targetTy
return true;
}

switch (targetType)
switch (sourceType, targetType)
{
case AnyType _:
case (_, AnyType):
// values of all types can be assigned to the "any" type
return true;

case IScopeReference targetScope:
case (IScopeReference, IScopeReference):
// checking for valid combinations of scopes happens after type checking. this allows us to provide a richer & more intuitive error message.
return sourceType is IScopeReference;
return true;

case UnionType union when ReferenceEquals(union, LanguageConstants.ResourceOrResourceCollectionRefItem):
case (_, UnionType targetUnion) when ReferenceEquals(targetUnion, LanguageConstants.ResourceOrResourceCollectionRefItem):
return sourceType is IScopeReference || sourceType is ArrayType { Item: IScopeReference };

case TypeSymbol _ when sourceType is ResourceType sourceResourceType:
case (ResourceType sourceResourceType, ResourceParentType targetResourceParentType):
// Assigning a resource to a parent property.
return sourceResourceType.TypeReference.IsParentOf(targetResourceParentType.ChildTypeReference);

case (ResourceType sourceResourceType, _):
// When assigning a resource, we're really assigning the value of the resource body.
return AreTypesAssignable(sourceResourceType.Body.Type, targetType);

case TypeSymbol _ when sourceType is ModuleType sourceModuleType:
case (ModuleType sourceModuleType, _):
// When assigning a module, we're really assigning the value of the module body.
return AreTypesAssignable(sourceModuleType.Body.Type, targetType);

case StringLiteralType _ when sourceType is StringLiteralType:
case (StringLiteralType, StringLiteralType):
// The name *is* the escaped string value, so we must have an exact match.
return targetType.Name == sourceType.Name;

case StringLiteralType _ when sourceType is PrimitiveType:
case (PrimitiveType, StringLiteralType):
// We allow string to string literal assignment only in the case where the "AllowLooseStringAssignment" validation flag has been set.
// This is to allow parameters without 'allowed' values to be assigned to fields expecting enums.
// At some point we may want to consider flowing the enum type backwards to solve this more elegantly.
return sourceType.ValidationFlags.HasFlag(TypeSymbolValidationFlags.AllowLooseStringAssignment) && sourceType.Name == LanguageConstants.String.Name;

case PrimitiveType _ when sourceType is StringLiteralType:
case (StringLiteralType, PrimitiveType):
// string literals can be assigned to strings
return targetType.Name == LanguageConstants.String.Name;

case PrimitiveType _ when sourceType is PrimitiveType:
case (PrimitiveType, PrimitiveType):
// both types are primitive
// compare by type name
return string.Equals(sourceType.Name, targetType.Name, StringComparison.Ordinal);

case ObjectType _ when sourceType is ObjectType:
case (ObjectType, ObjectType):
// both types are objects
// this function does not implement any schema validation, so this is far as we go
return true;

case ArrayType _ when sourceType is ArrayType:
case (ArrayType, ArrayType):
// both types are arrays
// this function does not validate item types
return true;

case DiscriminatedObjectType _ when sourceType is DiscriminatedObjectType:
case (DiscriminatedObjectType, DiscriminatedObjectType):
// validation left for later
return true;

case DiscriminatedObjectType _ when sourceType is ObjectType:
case (ObjectType, DiscriminatedObjectType):
// validation left for later
return true;

case TypeSymbol _ when sourceType is UnionType sourceUnion:
case (UnionType sourceUnion, _):
// union types are guaranteed to be flat

// TODO: Replace with some sort of set intersection
// are all source type members assignable to the target type?
return sourceUnion.Members.All(sourceMember => AreTypesAssignable(sourceMember.Type, targetType) == true);

case UnionType targetUnion:
case (_, UnionType targetUnion):
// the source type should be a singleton type
Debug.Assert(!(sourceType is UnionType), "!(sourceType is UnionType)");

Expand Down

0 comments on commit de8b847

Please sign in to comment.