From 5dd7d1969f732e59cb34a79ef6ed1994f6e38fbb Mon Sep 17 00:00:00 2001 From: Ould Sid El Moctar Mohamed El Moctar Date: Tue, 28 Jan 2025 15:27:02 -0800 Subject: [PATCH 1/6] fix for bicep issue #15458 --- .../PlaceholderParametersBicepParamWriter.cs | 84 +++++++++---------- .../Emit/PlaceholderParametersJsonWriter.cs | 23 ++--- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs index c6d7e47c451..2cc0e824ec9 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs @@ -30,7 +30,7 @@ public void Write(TextWriter writer, string existingContent) var result = filteredParameterDeclarations .OfType() - .Select(e => new ParameterAssignmentSyntax(e.Keyword, e.Name, SyntaxFactory.AssignmentToken, this.GetValueForParameter(e))) + .Select(e => new ParameterAssignmentSyntax(e.Keyword, e.Name, SyntaxFactory.AssignmentToken, GetValueForParameter(e))) .SelectMany(e => new List() { e, SyntaxFactory.NewlineToken }); var processedSyntaxList = new List() @@ -47,7 +47,47 @@ public void Write(TextWriter writer, string existingContent) writer.WriteLine(output); } - private SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax) + private static SyntaxBase CheckFunctionCallsInObjectSyntax(ObjectSyntax objectSyntax) + { + var value = objectSyntax.Properties.Select(e => + { + if (e.Value is FunctionCallSyntax valueAsFunctionCallSyntax) + { + return SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCallSyntax.Name.IdentifierName)); + } + else if (e.Value is ArraySyntax valueAsFunctionArraySyntax) + { + var value = valueAsFunctionArraySyntax.Items.Select(f => + { + if (f.Value is FunctionCallSyntax valueAsFunctionCallSyntax) + { + return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCallSyntax.Name.IdentifierName)); + } + + return f; + }).ToList(); + + return SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", SyntaxFactory.CreateArray(value)); + } + else if (e.Value is ObjectSyntax valueAsObjectSyntax) + { + var syntax = CheckFunctionCallsInObjectSyntax(valueAsObjectSyntax); + var item = SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", syntax); + return item; + } + + return e; + }).ToList(); + + return SyntaxFactory.CreateObject(value); + } + + private static SyntaxBase CreateCommentSyntaxForFunctionCallSyntax(string functionName) + { + return SyntaxFactory.CreateInvalidSyntaxWithComment($" TODO : please fix the value assigned to this parameter `{functionName}()` "); + } + + public static SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax) { var defaultValue = SyntaxHelper.TryGetDefaultValue(syntax); if (defaultValue != null) @@ -102,45 +142,5 @@ private SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax) return SyntaxFactory.NewlineToken; } - - private SyntaxBase CheckFunctionCallsInObjectSyntax(ObjectSyntax objectSyntax) - { - var value = objectSyntax.Properties.Select(e => - { - if (e.Value is FunctionCallSyntax valueAsFunctionCallSyntax) - { - return SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCallSyntax.Name.IdentifierName)); - } - else if (e.Value is ArraySyntax valueAsFunctionArraySyntax) - { - var value = valueAsFunctionArraySyntax.Items.Select(f => - { - if (f.Value is FunctionCallSyntax valueAsFunctionCallSyntax) - { - return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCallSyntax.Name.IdentifierName)); - } - - return f; - }).ToList(); - - return SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", SyntaxFactory.CreateArray(value)); - } - else if (e.Value is ObjectSyntax valueAsObjectSyntax) - { - var syntax = CheckFunctionCallsInObjectSyntax(valueAsObjectSyntax); - var item = SyntaxFactory.CreateObjectProperty((e.Key as IdentifierSyntax)?.IdentifierName ?? "", syntax); - return item; - } - - return e; - }).ToList(); - - return SyntaxFactory.CreateObject(value); - } - - private SyntaxBase CreateCommentSyntaxForFunctionCallSyntax(string functionName) - { - return SyntaxFactory.CreateInvalidSyntaxWithComment($" TODO : please fix the value assigned to this parameter `{functionName}()` "); - } } } diff --git a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs index 485ac9a7514..eb91eef8088 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs @@ -97,25 +97,12 @@ private JToken GenerateTemplate(string contentVersion) { jsonWriter.WritePropertyName(parameterSymbol.Name); + var value = PlaceholderParametersBicepParamWriter.GetValueForParameter(parameterSymbol.DeclaringParameter); + jsonWriter.WriteStartObject(); - switch (parameterSymbol.Type.Name) - { - case "string": - emitter.EmitProperty("value", ""); - break; - case "int": - emitter.EmitProperty("value", () => jsonWriter.WriteValue(0)); - break; - case "bool": - emitter.EmitProperty("value", () => jsonWriter.WriteValue(false)); - break; - case "object": - emitter.EmitProperty("value", () => { jsonWriter.WriteStartObject(); jsonWriter.WriteEndObject(); }); - break; - case "array": - emitter.EmitProperty("value", () => { jsonWriter.WriteStartArray(); jsonWriter.WriteEndArray(); }); - break; - } + + emitter.EmitProperty("value", value); + jsonWriter.WriteEndObject(); } From df3a1b3f59038aa558ec816a53a6a6c44ea8493a Mon Sep 17 00:00:00 2001 From: ouldsid Date: Wed, 29 Jan 2025 16:09:09 -0800 Subject: [PATCH 2/6] fixing test to cover the fixed scenario --- src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs index 2080ee78597..d2d638526a5 100644 --- a/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs @@ -93,7 +93,7 @@ public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsA ""contentVersion"": ""1.0.0.0"", ""parameters"": { ""name"": { - ""value"": """" + ""value"": ""sampleparameter"" } } }".ReplaceLineEndings()); @@ -125,7 +125,7 @@ public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsA ""contentVersion"": ""1.0.0.0"", ""parameters"": { ""optional"": { - ""value"": """" + ""value"": ""sampleparameter"" }, ""required"": { ""value"": """" From eb0673bd165bc599978ffb097843c0d6fa132e01 Mon Sep 17 00:00:00 2001 From: ouldsid Date: Thu, 30 Jan 2025 10:49:02 -0800 Subject: [PATCH 3/6] fixed an exception in case of no text in the returned synthaxBase value --- src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs index eb91eef8088..236f58bbed3 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs @@ -101,7 +101,11 @@ private JToken GenerateTemplate(string contentVersion) jsonWriter.WriteStartObject(); - emitter.EmitProperty("value", value); + // emit value property only if the text is not null + if (value.ToString() != "\r\n") + { + emitter.EmitProperty("value", value); + } jsonWriter.WriteEndObject(); } From 1f6df0a30030588e858c1270a7326169b206aa6e Mon Sep 17 00:00:00 2001 From: ouldsid Date: Wed, 5 Feb 2025 11:20:01 -0800 Subject: [PATCH 4/6] comments --- .../GenerateParamsCommandTests.cs | 98 +++++++++++++++++++ .../PlaceholderParametersBicepParamWriter.cs | 17 +++- .../Emit/PlaceholderParametersJsonWriter.cs | 7 +- .../Intermediate/ExpressionBuilder.cs | 9 +- 4 files changed, 122 insertions(+), 9 deletions(-) diff --git a/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs index d2d638526a5..b6a216730c5 100644 --- a/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs @@ -767,5 +767,103 @@ public async Task GenerateParams_ExplicitOutputFormatBicepParam_ExplicitIncludeP ".ReplaceLineEndings()); } } + + [TestMethod] + public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsAll_OneObjectParameterWithDefaultValue_Should_Succeed() + { + var bicep = $@"param storageAccountConfig object = {{ + name: 'defaultStorageAccount' + sku: 'Standard_LRS' + kind: 'StorageV2' + }}"; + + var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext); + Directory.CreateDirectory(tempDirectory); + + var bicepFilePath = Path.Combine(tempDirectory, "built.bicep"); + File.WriteAllText(bicepFilePath, bicep); + + var (output, error, result) = await Bicep("generate-params", "--include-params", "all", bicepFilePath); + + var content = File.ReadAllText(Path.Combine(tempDirectory, "built.parameters.json")).ReplaceLineEndings(); + + content.Should().Be(@"{ + ""$schema"": ""https://schema.management.azure.com/schemas/2015-01-01/deploymentParameters.json#"", + ""contentVersion"": ""1.0.0.0"", + ""parameters"": { + ""storageAccountConfig"": { + ""value"": { + ""name"": ""defaultStorageAccount"", + ""sku"": ""Standard_LRS"", + ""kind"": ""StorageV2"" + } + } + } +}".ReplaceLineEndings()); + } + + [TestMethod] + public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsAll_OneArrayParameterWithDefaultValue_Should_Succeed() + { + var bicep = $@"param foo array = [1, 2, 3]"; + var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext); + Directory.CreateDirectory(tempDirectory); + var bicepFilePath = Path.Combine(tempDirectory, "built.bicep"); + File.WriteAllText(bicepFilePath, bicep); + var (output, error, result) = await Bicep("generate-params", "--include-params", "all", bicepFilePath); + var content = File.ReadAllText(Path.Combine(tempDirectory, "built.parameters.json")).ReplaceLineEndings(); + content.Should().Be(@"{ + ""$schema"": ""https://schema.management.azure.com/schemas/2015-01-01/deploymentParameters.json#"", + ""contentVersion"": ""1.0.0.0"", + ""parameters"": { + ""foo"": { + ""value"": [ + 1, + 2, + 3 + ] + } + } +}".ReplaceLineEndings()); + } + + [TestMethod] + public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsAll_OneParameterWithDefaultValueAsInvalidBicepSyntax_Should_Succeed() + { + var bicep = $@"param foo string = newGuid()"; + var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext); + Directory.CreateDirectory(tempDirectory); + var bicepFilePath = Path.Combine(tempDirectory, "built.bicep"); + File.WriteAllText(bicepFilePath, bicep); + var (output, error, result) = await Bicep("generate-params", "--include-params", "all", bicepFilePath); + var content = File.ReadAllText(Path.Combine(tempDirectory, "built.parameters.json")).ReplaceLineEndings(); + + content.Should().Be(@"{ + ""$schema"": ""https://schema.management.azure.com/schemas/2015-01-01/deploymentParameters.json#"", + ""contentVersion"": ""1.0.0.0"", + ""parameters"": { + ""foo"": { + ""value"": """" + } + } +}".ReplaceLineEndings()); + } + + [TestMethod] + public async Task GenerateParams_ExplicitOutputFormatBicepParam_ExplicitIncludeParamsAll_OneParameterWithDefaultValueAsInvalidBicepSyntax_Should_Succeed() + { + var bicep = $@"param foo string = newGuid()"; + var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext); + Directory.CreateDirectory(tempDirectory); + var bicepFilePath = Path.Combine(tempDirectory, "built.bicep"); + File.WriteAllText(bicepFilePath, bicep); + var (output, error, result) = await Bicep("generate-params", "--output-format", "bicepparam", "--include-params", "all", bicepFilePath); + var content = File.ReadAllText(Path.Combine(tempDirectory, "built.bicepparam")).ReplaceLineEndings(); + content.Should().Be(@"using './built.bicep' + +param foo = ? /* TODO : please fix the value assigned to this parameter `newGuid()` */ + +".ReplaceLineEndings()); + } } } diff --git a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs index 2cc0e824ec9..bbc985f42e0 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs @@ -82,19 +82,26 @@ private static SyntaxBase CheckFunctionCallsInObjectSyntax(ObjectSyntax objectSy return SyntaxFactory.CreateObject(value); } - private static SyntaxBase CreateCommentSyntaxForFunctionCallSyntax(string functionName) + private static SyntaxBase CreateCommentSyntaxForFunctionCallSyntax(string functionName, bool isJsonParamWriter = false) { - return SyntaxFactory.CreateInvalidSyntaxWithComment($" TODO : please fix the value assigned to this parameter `{functionName}()` "); + if (isJsonParamWriter) + { + return SyntaxFactory.CreateStringLiteral(""); + } + else + { + return SyntaxFactory.CreateInvalidSyntaxWithComment($" TODO : please fix the value assigned to this parameter `{functionName}()` "); + } } - public static SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax) + public static SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax, bool isJsonParamWriter = false) { var defaultValue = SyntaxHelper.TryGetDefaultValue(syntax); if (defaultValue != null) { if (defaultValue is FunctionCallSyntax defaultValueAsFunctionCall) { - return CreateCommentSyntaxForFunctionCallSyntax(defaultValueAsFunctionCall.Name.IdentifierName); + return CreateCommentSyntaxForFunctionCallSyntax(defaultValueAsFunctionCall.Name.IdentifierName, isJsonParamWriter); } else if (defaultValue is ArraySyntax defaultValueAsArray) { @@ -102,7 +109,7 @@ public static SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax) { if (e.Value is FunctionCallSyntax valueAsFunctionCall) { - return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName)); + return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName, isJsonParamWriter)); } return e; diff --git a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs index 236f58bbed3..531fd05a696 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs @@ -3,6 +3,7 @@ using System.Collections.Immutable; using Bicep.Core.Emit.Options; +using Bicep.Core.Parsing; using Bicep.Core.Semantics; using Bicep.Core.Syntax; using Microsoft.WindowsAzure.ResourceStack.Common.Json; @@ -97,12 +98,12 @@ private JToken GenerateTemplate(string contentVersion) { jsonWriter.WritePropertyName(parameterSymbol.Name); - var value = PlaceholderParametersBicepParamWriter.GetValueForParameter(parameterSymbol.DeclaringParameter); + var value = PlaceholderParametersBicepParamWriter.GetValueForParameter(parameterSymbol.DeclaringParameter, true); jsonWriter.WriteStartObject(); - // emit value property only if the text is not null - if (value.ToString() != "\r\n") + // emit value property only if the value is not a newline token + if (value is not Token { Type: TokenType.NewLine}) { emitter.EmitProperty("value", value); } diff --git a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs index b46784587dc..46f1f9c9b48 100644 --- a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs +++ b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs @@ -725,7 +725,14 @@ void completePreviousChunk() } else if (child is ArrayItemSyntax arrayItem) { - currentChunk.Add(ConvertWithoutLowering(arrayItem.Value)); + if (arrayItem.Value is not null && arrayItem.Value is ArrayItemSyntax arrayItemInside) + { + currentChunk.Add(ConvertWithoutLowering(arrayItemInside.Value)); + } + else + { + currentChunk.Add(ConvertWithoutLowering(arrayItem)); + } } } completePreviousChunk(); From 4bc280e215ff5f62711e3bd4ba4bb80c6375d36d Mon Sep 17 00:00:00 2001 From: ouldsid Date: Wed, 5 Feb 2025 15:18:47 -0800 Subject: [PATCH 5/6] fixing an issue introduced in my last iteration which introduced a fix for an existing bug in ExpressionBuilder -> ConvertArray --- src/Bicep.Core/Intermediate/ExpressionBuilder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs index 3936b7bdf0c..628a6ffc765 100644 --- a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs +++ b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs @@ -741,7 +741,10 @@ void completePreviousChunk() } else { - currentChunk.Add(ConvertWithoutLowering(arrayItem)); + if (arrayItem.Value is not null) + { + currentChunk.Add(ConvertWithoutLowering(arrayItem.Value)); + } } } } From dea5122d69fe3e666ee79dbde6761b3609647af8 Mon Sep 17 00:00:00 2001 From: ouldsid Date: Thu, 6 Feb 2025 16:06:45 -0800 Subject: [PATCH 6/6] moving the fix introduced in last iteration from ExpressionBuilder and add it to the correct location --- .../Emit/PlaceholderParametersBicepParamWriter.cs | 4 ++-- .../Emit/PlaceholderParametersJsonWriter.cs | 3 ++- src/Bicep.Core/Intermediate/ExpressionBuilder.cs | 12 +----------- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs index bbc985f42e0..03f40afb6d0 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs @@ -109,10 +109,10 @@ public static SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax, { if (e.Value is FunctionCallSyntax valueAsFunctionCall) { - return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName, isJsonParamWriter)); + return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName, isJsonParamWriter)).Value; } - return e; + return e.Value; }).ToList(); return SyntaxFactory.CreateArray(value); diff --git a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs index 531fd05a696..c862eb3f377 100644 --- a/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs +++ b/src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs @@ -102,7 +102,8 @@ private JToken GenerateTemplate(string contentVersion) jsonWriter.WriteStartObject(); - // emit value property only if the value is not a newline token + // Emit value property only if the value is not a newline token. + // GetValueForParameter return TokenType.NewLine when syntax of assigning the value is not correct if (value is not Token { Type: TokenType.NewLine}) { emitter.EmitProperty("value", value); diff --git a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs index 628a6ffc765..d0b0bc4755d 100644 --- a/src/Bicep.Core/Intermediate/ExpressionBuilder.cs +++ b/src/Bicep.Core/Intermediate/ExpressionBuilder.cs @@ -735,17 +735,7 @@ void completePreviousChunk() } else if (child is ArrayItemSyntax arrayItem) { - if (arrayItem.Value is not null && arrayItem.Value is ArrayItemSyntax arrayItemInside) - { - currentChunk.Add(ConvertWithoutLowering(arrayItemInside.Value)); - } - else - { - if (arrayItem.Value is not null) - { - currentChunk.Add(ConvertWithoutLowering(arrayItem.Value)); - } - } + currentChunk.Add(ConvertWithoutLowering(arrayItem.Value)); } } completePreviousChunk();