From eb3d221626753acffcaa304232f44a747fc297d3 Mon Sep 17 00:00:00 2001 From: Anthony Martin Date: Tue, 3 Nov 2020 16:51:27 -0800 Subject: [PATCH] Address comments from #771 review (#807) * Fixes for comments on #771 * Simplify CanConvertToArmJson logic --- .../Files/Variables_LF/main.json | 6 +-- src/Bicep.Core/Emit/ExpressionConverter.cs | 48 +++++++++++-------- src/Bicep.Core/Emit/ScopeHelper.cs | 9 ---- .../Parser/ParseDiagnosticsVisitor.cs | 7 ++- src/Bicep.Core/Parser/Parser.cs | 4 +- src/Bicep.Core/Syntax/TargetScopeSyntax.cs | 7 +-- src/Bicep.Core/TypeSystem/ModuleType.cs | 3 +- src/Bicep.Core/TypeSystem/TypeKind.cs | 5 ++ 8 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/Bicep.Core.Samples/Files/Variables_LF/main.json b/src/Bicep.Core.Samples/Files/Variables_LF/main.json index 35db500984e..7861962570c 100644 --- a/src/Bicep.Core.Samples/Files/Variables_LF/main.json +++ b/src/Bicep.Core.Samples/Files/Variables_LF/main.json @@ -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": [] diff --git a/src/Bicep.Core/Emit/ExpressionConverter.cs b/src/Bicep.Core/Emit/ExpressionConverter.cs index bc756e3f795..a801c5ad61b 100644 --- a/src/Bicep.Core/Emit/ExpressionConverter.cs +++ b/src/Bicep.Core/Emit/ExpressionConverter.cs @@ -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; @@ -71,29 +74,14 @@ public LanguageExpression ConvertExpression(SyntaxBase expression) Array.Empty()); 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()); @@ -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()); } + 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 @@ -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()); + return GetCreateObjectExpression(parameters); } + private static FunctionExpression GetCreateObjectExpression(params LanguageExpression[] parameters) + => new FunctionExpression("createObject", parameters, Array.Empty()); + private LanguageExpression ConvertBinary(BinaryOperationSyntax syntax) { LanguageExpression operand1 = ConvertExpression(syntax.LeftExpression); diff --git a/src/Bicep.Core/Emit/ScopeHelper.cs b/src/Bicep.Core/Emit/ScopeHelper.cs index 5e07f4dfbb0..869d8e04e74 100644 --- a/src/Bicep.Core/Emit/ScopeHelper.cs +++ b/src/Bicep.Core/Emit/ScopeHelper.cs @@ -80,14 +80,5 @@ private static IReadOnlyDictionary 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, - }; } } \ No newline at end of file diff --git a/src/Bicep.Core/Parser/ParseDiagnosticsVisitor.cs b/src/Bicep.Core/Parser/ParseDiagnosticsVisitor.cs index c534e8d50b5..7d78a1a80f6 100644 --- a/src/Bicep.Core/Parser/ParseDiagnosticsVisitor.cs +++ b/src/Bicep.Core/Parser/ParseDiagnosticsVisitor.cs @@ -27,9 +27,12 @@ public override void VisitProgramSyntax(ProgramSyntax syntax) this.diagnosticWriter.WriteMultiple(syntax.LexerDiagnostics); var targetScopeSyntaxes = syntax.Children.OfType().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()); + } } } diff --git a/src/Bicep.Core/Parser/Parser.cs b/src/Bicep.Core/Parser/Parser.cs index 2d8908c303a..4b7f4d609f4 100644 --- a/src/Bicep.Core/Parser/Parser.cs +++ b/src/Bicep.Core/Parser/Parser.cs @@ -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() diff --git a/src/Bicep.Core/Syntax/TargetScopeSyntax.cs b/src/Bicep.Core/Syntax/TargetScopeSyntax.cs index 70a6d70a326..05a49f7c894 100644 --- a/src/Bicep.Core/Syntax/TargetScopeSyntax.cs +++ b/src/Bicep.Core/Syntax/TargetScopeSyntax.cs @@ -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; @@ -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; } diff --git a/src/Bicep.Core/TypeSystem/ModuleType.cs b/src/Bicep.Core/TypeSystem/ModuleType.cs index 5d53488e98b..4006f823807 100644 --- a/src/Bicep.Core/TypeSystem/ModuleType.cs +++ b/src/Bicep.Core/TypeSystem/ModuleType.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Collections.Generic; namespace Bicep.Core.TypeSystem { @@ -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; } diff --git a/src/Bicep.Core/TypeSystem/TypeKind.cs b/src/Bicep.Core/TypeSystem/TypeKind.cs index c8a3897bcf8..2b9ccc9d4f4 100644 --- a/src/Bicep.Core/TypeSystem/TypeKind.cs +++ b/src/Bicep.Core/TypeSystem/TypeKind.cs @@ -58,5 +58,10 @@ public enum TypeKind /// A reference to a resource scope. /// ResourceScopeReference, + + /// + /// Module type + /// + Module, } } \ No newline at end of file