Skip to content

Commit

Permalink
Catch recursion in parameterized type invocations
Browse files Browse the repository at this point in the history
  • Loading branch information
jeskew committed Dec 18, 2024
1 parent 77f4bf4 commit d6576a0
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 4 deletions.
15 changes: 15 additions & 0 deletions src/Bicep.Core.IntegrationTests/UserDefinedTypeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,4 +1823,19 @@ param siteProperties resourceInput<'Microsoft.Web/sites@2022-09-01'>.properties
"""The property "availabilityState" is read-only. Expressions cannot be assigned to read-only properties."""
);
}

[DataTestMethod]
[DataRow("type resourceInput = resourceInput<'Microsoft.Compute/virtualMachines'>")] // should be caught at syntax level
[DataRow("type resourceInput = resourceInput<'Microsoft.Compute/virtualMachines'>.properties")] // should be caught by type manager
public void Parameterized_type_recursion_raises_diagnostic(string template)
{
var result = CompilationHelper.Compile(
new ServiceBuilder().WithFeatureOverrides(new(TestContext, ResourceDerivedTypesEnabled: true)),
template);

result.Should().HaveDiagnostics(new[]
{
("BCP298", DiagnosticLevel.Error, "This type definition includes itself as required component, which creates a constraint that cannot be fulfilled."),
});
}
}
12 changes: 12 additions & 0 deletions src/Bicep.Core/TypeSystem/CyclicCheckVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,18 @@ public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax)
base.VisitFunctionCallSyntax(syntax);
}

public override void VisitParameterizedTypeInstantiationSyntax(ParameterizedTypeInstantiationSyntax syntax)
{
if (!currentDeclarations.TryPeek(out var currentDeclaration))
{
// we're not inside a declaration, so there should be no risk of a cycle
return;
}

declarationAccessDict[currentDeclaration].Add(syntax);
base.VisitParameterizedTypeInstantiationSyntax(syntax);
}

public override void VisitArrayTypeMemberSyntax(ArrayTypeMemberSyntax syntax)
=> WithSelfReferencePermitted(() => base.VisitArrayTypeMemberSyntax(syntax), selfReferencePermitted: true);

Expand Down
12 changes: 12 additions & 0 deletions src/Bicep.Core/TypeSystem/CyclicTypeCheckVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ public override void VisitTypeVariableAccessSyntax(TypeVariableAccessSyntax synt
base.VisitTypeVariableAccessSyntax(syntax);
}

public override void VisitParameterizedTypeInstantiationSyntax(ParameterizedTypeInstantiationSyntax syntax)
{
// If this reference is not nested within a type container, it would have been detected based on syntax alone in CyclicCheckVisitor.
// To avoid doubling up on diagnostics, skip recording cycles on top-level accesses
if (enteredTypeContainer)
{
declarationAccesses.Add(syntax);
}

base.VisitParameterizedTypeInstantiationSyntax(syntax);
}

public override void VisitArrayTypeSyntax(ArrayTypeSyntax syntax)
=> WithEnteredTypeContainerState(() => base.VisitArrayTypeSyntax(syntax), enteredTypeContainer: true);

Expand Down
7 changes: 3 additions & 4 deletions src/Bicep.Core/TypeSystem/DeclaredTypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,6 @@ private DeclaredTypeAssignment GetTypeType(TypeDeclarationSyntax syntax)

private TypeSymbol GetUserDefinedTypeType(TypeAliasSymbol symbol)
{
// Even if the declared type is invalid because of a illegal cycle, we still want to visit (and cache the type of) nested elements.
var declaredType = GetTypeFromTypeSyntax(symbol.DeclaringType.Value);

if (binder.TryGetCycle(symbol) is { } cycle)
{
var builder = DiagnosticBuilder.ForPosition(symbol.DeclaringType.Name);
Expand All @@ -272,7 +269,9 @@ private TypeSymbol GetUserDefinedTypeType(TypeAliasSymbol symbol)
return ErrorType.Create(diagnostic);
}

return ApplyTypeModifyingDecorators(DisallowNamespaceTypes(declaredType.Type, symbol.DeclaringType.Value), symbol.DeclaringType);
return ApplyTypeModifyingDecorators(
DisallowNamespaceTypes(GetTypeFromTypeSyntax(symbol.DeclaringType.Value).Type, symbol.DeclaringType.Value),
symbol.DeclaringType);
}

private static ITypeReference DisallowNamespaceTypes(ITypeReference typeReference, SyntaxBase syntax) => typeReference switch
Expand Down

0 comments on commit d6576a0

Please sign in to comment.