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.

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

Fix type checking for sys.allowed #2503

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1880,4 +1880,21 @@ public void Test_Issue2262()
codeReplacement.Span.Should().Be(new TextSpan(212, 15));
codeReplacement.Text.Should().Be("partitionScheme");
}

[TestMethod]
// https://github.com/Azure/bicep/issues/2484
public void Test_Issue2484()
{
var result = CompilationHelper.Compile(@"
@sys.allowed([
'apple'
'banana'
])
param foo string = 'peach'
");

result.Should().HaveDiagnostics(new[] {
("BCP027", DiagnosticLevel.Error, "The parameter expects a default value of type \"'apple' | 'banana'\" but provided value is of type \"'peach'\"."),
});
}
} }
14 changes: 14 additions & 0 deletions src/Bicep.Core/TypeSystem/TypeAssignmentVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,20 @@ public override void VisitParameterDeclarationSyntax(ParameterDeclarationSyntax
*/
var allowedDecoratorSyntax = syntax.Decorators.FirstOrDefault(decoratorSyntax =>
{
if (decoratorSyntax.Expression is InstanceFunctionCallSyntax { BaseExpression: VariableAccessSyntax namespaceSyntax, Name: IdentifierSyntax nameSyntax } &&
namespaceSyntax.Name.IdentifierName == "sys" &&
nameSyntax.IdentifierName == "allowed")
{
/*
* This is a workaround to unambiguously get the syntax for "@sys.allowed(...)".
* We cannot get it by resolving the function symbol, since currently the binder
* does not bind an InstanceFunctionCallSyntax to a symbol. This does not affect
* decorator emitting though, because in the emitter we call SemanticModel.GetSymbolInfo()
* which adds special handling for InstanceFunctionCallSyntax.
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@majastrz do we have any plan to update the binder to bind InstanceFunctionCallSyntax, PropertyAccessSyntax and ObjectPropertySyntax?

/cc @miqm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, It's intentionally deferred with Anthony's latest changes to support methods and property hovers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it!

return true;
}

if (this.binder.GetSymbolInfo(decoratorSyntax.Expression) is FunctionSymbol functionSymbol)
{
var argumentTypes = this.GetRecoveredArgumentTypes(decoratorSyntax.Arguments).ToArray();
Expand Down