Skip to content

Commit

Permalink
Address comments from #771 review (#807)
Browse files Browse the repository at this point in the history
* Fixes for comments on #771

* Simplify CanConvertToArmJson logic
  • Loading branch information
anthony-c-martin authored Nov 4, 2020
1 parent b845482 commit eb3d221
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 40 deletions.
6 changes: 3 additions & 3 deletions src/Bicep.Core.Samples/Files/Variables_LF/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@
"isFalse": "[not(variables('isTrue'))]",
"someText": "[if(variables('isTrue'), concat('a', concat('b', 'c')), 'someText')]",
"scopesWithoutArmRepresentation": {
"tenant": "[json('{}')]",
"subscription": "[json('{}')]",
"resourceGroup": "[json('{}')]"
"tenant": "[createObject()]",
"subscription": "[createObject()]",
"resourceGroup": "[createObject()]"
}
},
"resources": []
Expand Down
48 changes: 29 additions & 19 deletions src/Bicep.Core/Emit/ExpressionConverter.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Arm.Expression.Expressions;
using Bicep.Core.Resources;
Expand Down Expand Up @@ -71,29 +74,14 @@ public LanguageExpression ConvertExpression(SyntaxBase expression)
Array.Empty<LanguageExpression>());

case FunctionCallSyntax function:
var returnValueType = context.SemanticModel.GetTypeInfo(function);
if (returnValueType is IResourceScopeType resourceScopeType && !ScopeHelper.CanConvertToArmJson(resourceScopeType))
{
// return an empty object - there is no ARM equivalent to return
return new FunctionExpression("json", new LanguageExpression [] { new JTokenExpression("{}") }, new LanguageExpression[0]);
}

return ConvertFunction(
function.Name.IdentifierName,
function.Arguments.Select(a => ConvertExpression(a.Expression)).ToArray());

case InstanceFunctionCallSyntax instanceFunctionCall:
var namespaceSymbol = context.SemanticModel.GetSymbolInfo(instanceFunctionCall.BaseExpression);

Assert(namespaceSymbol is NamespaceSymbol, $"BaseExpression must be a NamespaceSymbol, instead got: '{namespaceSymbol?.Kind}'");

var returnValueType2 = context.SemanticModel.GetTypeInfo(instanceFunctionCall);
if (returnValueType2 is IResourceScopeType resourceScopeType2 && !ScopeHelper.CanConvertToArmJson(resourceScopeType2))
{
// return an empty object - there is no ARM equivalent to return
return new FunctionExpression("json", new LanguageExpression [] { new JTokenExpression("{}") }, new LanguageExpression[0]);
}

return ConvertFunction(
instanceFunctionCall.Name.IdentifierName,
instanceFunctionCall.Arguments.Select(a => ConvertExpression(a.Expression)).ToArray());
Expand Down Expand Up @@ -374,9 +362,31 @@ private static LanguageExpression ConvertFunction(string functionName, LanguageE
return arguments.Single();
}

if (ShouldReplaceUnsupportedFunction(functionName, arguments, out var replacementExpression))
{
return replacementExpression;
}

return new FunctionExpression(functionName, arguments, Array.Empty<LanguageExpression>());
}

private static bool ShouldReplaceUnsupportedFunction(string functionName, LanguageExpression[] arguments, [NotNullWhen(true)] out LanguageExpression? replacementExpression)
{
switch (functionName)
{
// These functions have not yet been implemented in ARM. For now, we will just return an empty object if they are accessed directly.
case "tenant":
case "managementGroup":
case "subscription" when arguments.Length > 0:
case "resourceGroup" when arguments.Length > 0:
replacementExpression = GetCreateObjectExpression();
return true;
}

replacementExpression = null;
return false;
}

private FunctionExpression ConvertArray(ArraySyntax syntax)
{
// we are using the createArray() function as a proxy for an array literal
Expand All @@ -402,12 +412,12 @@ private FunctionExpression ConvertObject(ObjectSyntax syntax)
}

// we are using the createObject() funciton as a proy for an object literal
return new FunctionExpression(
"createObject",
parameters,
Array.Empty<LanguageExpression>());
return GetCreateObjectExpression(parameters);
}

private static FunctionExpression GetCreateObjectExpression(params LanguageExpression[] parameters)
=> new FunctionExpression("createObject", parameters, Array.Empty<LanguageExpression>());

private LanguageExpression ConvertBinary(BinaryOperationSyntax syntax)
{
LanguageExpression operand1 = ConvertExpression(syntax.LeftExpression);
Expand Down
9 changes: 0 additions & 9 deletions src/Bicep.Core/Emit/ScopeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,5 @@ private static IReadOnlyDictionary<string, LanguageExpression> ToPropertyDiction
resourceGroup: expressionConverter.ConvertExpression(scopeType.Arguments[1].Expression)),
_ => null,
};

public static bool CanConvertToArmJson(IResourceScopeType resourceScopeType)
=> resourceScopeType switch {
TenantScopeType _ => false,
ManagementGroupScopeType _ => false,
SubscriptionScopeType subscriptionScopeType => subscriptionScopeType.Arguments.Length == 0,
ResourceGroupScopeType resourceGroupScopeType => resourceGroupScopeType.Arguments.Length == 0,
_ => true,
};
}
}
7 changes: 5 additions & 2 deletions src/Bicep.Core/Parser/ParseDiagnosticsVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ public override void VisitProgramSyntax(ProgramSyntax syntax)
this.diagnosticWriter.WriteMultiple(syntax.LexerDiagnostics);

var targetScopeSyntaxes = syntax.Children.OfType<TargetScopeSyntax>().ToList();
foreach (var targetScope in targetScopeSyntaxes.Skip(1))
if (targetScopeSyntaxes.Count > 1)
{
this.diagnosticWriter.Write(targetScope.Keyword, x => x.TargetScopeMultipleDeclarations());
foreach (var targetScope in targetScopeSyntaxes)
{
this.diagnosticWriter.Write(targetScope.Keyword, x => x.TargetScopeMultipleDeclarations());
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Parser/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ private static RecoveryFlags GetSuppressionFlag(SyntaxBase precedingNode)
private SyntaxBase TargetScope()
{
var keyword = ExpectKeyword(LanguageConstants.TargetScopeKeyword);
var assignmentToken = this.Expect(TokenType.Assignment, b => b.ExpectedCharacter("="));
var assignment = this.WithRecovery(this.Assignment, RecoveryFlags.None, TokenType.NewLine);
var value = this.WithRecovery(() => this.Expression(allowComplexLiterals: true), RecoveryFlags.None, TokenType.NewLine);

return new TargetScopeSyntax(keyword, assignmentToken, value);
return new TargetScopeSyntax(keyword, assignment, value);
}

private SyntaxBase ParameterDeclaration()
Expand Down
7 changes: 4 additions & 3 deletions src/Bicep.Core/Syntax/TargetScopeSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ namespace Bicep.Core.Syntax
{
public class TargetScopeSyntax : SyntaxBase, IDeclarationSyntax
{
public TargetScopeSyntax(Token keyword, Token assignment, SyntaxBase value)
public TargetScopeSyntax(Token keyword, SyntaxBase assignment, SyntaxBase value)
{
AssertKeyword(keyword, nameof(keyword), LanguageConstants.TargetScopeKeyword);
AssertTokenType(assignment, nameof(assignment), TokenType.Assignment);
AssertSyntaxType(assignment, nameof(assignment), typeof(Token), typeof(SkippedTriviaSyntax));
AssertTokenType(assignment as Token, nameof(assignment), TokenType.Assignment);

this.Keyword = keyword;
this.Assignment = assignment;
Expand All @@ -20,7 +21,7 @@ public TargetScopeSyntax(Token keyword, Token assignment, SyntaxBase value)

public Token Keyword { get; }

public Token Assignment { get; }
public SyntaxBase Assignment { get; }

public SyntaxBase Value { get; }

Expand Down
3 changes: 1 addition & 2 deletions src/Bicep.Core/TypeSystem/ModuleType.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System.Collections.Generic;

namespace Bicep.Core.TypeSystem
{
Expand All @@ -12,7 +11,7 @@ public ModuleType(string name, ITypeReference body)
Body = body;
}

public override TypeKind TypeKind => TypeKind.Resource;
public override TypeKind TypeKind => TypeKind.Module;

public ITypeReference Body { get; }

Expand Down
5 changes: 5 additions & 0 deletions src/Bicep.Core/TypeSystem/TypeKind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,10 @@ public enum TypeKind
/// A reference to a resource scope.
/// </summary>
ResourceScopeReference,

/// <summary>
/// Module type
/// </summary>
Module,
}
}

0 comments on commit eb3d221

Please sign in to comment.