Skip to content

Commit

Permalink
Block calls to scope functions in .biecpparam files (#11903)
Browse files Browse the repository at this point in the history
Resolves #11902 

The error message currently shown when a scope function is used in a
.bicepparam file is unhelpful. These functions should only be defined in
`.bicep` files so that template authors get actionable error messages if
they try to use one.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/11903)
  • Loading branch information
jeskew committed Sep 21, 2023
1 parent f3276e7 commit e964970
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 12 deletions.
56 changes: 56 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5103,4 +5103,60 @@ public void Test_Issue11883()

result.Should().NotHaveAnyDiagnostics();
}

// https://github.com/Azure/bicep/issues/11902
[TestMethod]
public void Test_Issue11902()
{
var result = CompilationHelper.CompileParams(
("main.bicep", """
param rgName string
"""),
("parameters.bicepparam", """
using 'main.bicep'

var rg = resourceGroup().name
param rgName = rg
"""));

result.Should().HaveDiagnostics(new[]
{
("BCP057", DiagnosticLevel.Error, "The name \"resourceGroup\" does not exist in the current context."),
("BCP062", DiagnosticLevel.Error, "The referenced declaration with name \"rg\" is not valid."),
});
}

// https://github.com/Azure/bicep/issues/11902
[TestMethod]
public void Array_access_into_for_expression_should_not_cause_stack_overflow()
{
var result = CompilationHelper.CompileParams(
("main.bicep", """
param rgName string
"""),
("parameters.bicepparam", """
using 'main.bicep'

var groups = [
{
name: 'foo'
abrv: 'f'
}
{
name: 'bar'
abrv: 'b'
}
]

var rg = [for group in groups: group.name == resourceGroup().name ? group : []][0].abrv
param rgName = rg
"""));

result.Should().HaveDiagnostics(new[]
{
("BCP138", DiagnosticLevel.Error, "For-expressions are not supported in this context. For-expressions may be used as values of resource, module, variable, and output declarations, or values of resource and module properties."),
("BCP057", DiagnosticLevel.Error, "The name \"resourceGroup\" does not exist in the current context."),
("BCP062", DiagnosticLevel.Error, "The referenced declaration with name \"rg\" is not valid."),
});
}
}
22 changes: 11 additions & 11 deletions src/Bicep.Core/Semantics/Namespaces/AzNamespaceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,17 +290,6 @@ private static IEnumerable<(FunctionOverload functionOverload, ResourceScope all

private static IEnumerable<FunctionOverload> GetAzOverloads(ResourceScope resourceScope, BicepSourceFileKind sourceFileKind)
{
foreach (var (functionOverload, allowedScopes) in GetScopeFunctions())
{
// we only include it if it's valid at all of the scopes that the template is valid at
if (resourceScope == (resourceScope & allowedScopes))
{
yield return functionOverload;
}

// TODO: add banned function to explain why a given function isn't available
}

if (sourceFileKind == BicepSourceFileKind.ParamsFile)
{
yield return new FunctionOverloadBuilder(GetSecretFunctionName)
Expand Down Expand Up @@ -348,6 +337,17 @@ private static IEnumerable<FunctionOverload> GetAzOverloads(ResourceScope resour
}
else
{
foreach (var (functionOverload, allowedScopes) in GetScopeFunctions())
{
// we only include it if it's valid at all of the scopes that the template is valid at
if (resourceScope == (resourceScope & allowedScopes))
{
yield return functionOverload;
}

// TODO: add banned function to explain why a given function isn't available
}

// TODO: Add schema for return type
yield return new FunctionOverloadBuilder("deployment")
.WithReturnType(GetDeploymentReturnType(resourceScope))
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/TypeSystem/DeclaredTypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ private DeclaredTypeAssignment GetTestType(TestDeclarationSyntax syntax)
Stack<AccessExpressionSyntax> chainedAccesses = syntax.ToAccessExpressionStack();
var baseAssignment = chainedAccesses.Peek() switch
{
PropertyAccessSyntax access when access.BaseExpression is ForSyntax
AccessExpressionSyntax access when access.BaseExpression is ForSyntax
// in certain parser recovery scenarios, the parser can produce a PropertyAccessSyntax operating on a ForSyntax
// this leads to a stack overflow which we don't really want, so let's short circuit here.
=> null,
Expand Down

0 comments on commit e964970

Please sign in to comment.