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.

Sign up for GitHub

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 for bicep issue #15458 #16237

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+161 −63
Open

fix for bicep issue #15458 #16237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5dd7d19
fix for bicep issue #15458
Jan 28, 2025
a25423b
merge main and make sure that the integrated test covering the fixed …
ouldsid Jan 29, 2025
df3a1b3
fixing test to cover the fixed scenario
ouldsid Jan 30, 2025
eb0673b
fixed an exception in case of no text in the returned synthaxBase value
ouldsid Jan 30, 2025
1f6df0a
comments
ouldsid Feb 5, 2025
06a4e9e
Fix a merge issue
ouldsid Feb 5, 2025
4bc280e
fixing an issue introduced in my last iteration which introduced a fi…
ouldsid Feb 5, 2025
dea5122
moving the fix introduced in last iteration from ExpressionBuilder an…
ouldsid Feb 7, 2025
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
102 changes: 100 additions & 2 deletions src/Bicep.Cli.IntegrationTests/GenerateParamsCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsA
""contentVersion"": ""1.0.0.0"",
""parameters"": {
""name"": {
""value"": """"
""value"": ""sampleparameter""
}
}
}".ReplaceLineEndings());
Expand Down Expand Up @@ -125,7 +125,7 @@ public async Task GenerateParams_ImplicitOutputFormatJson_ExplicitIncludeParamsA
""contentVersion"": ""1.0.0.0"",
""parameters"": {
""optional"": {
""value"": """"
""value"": ""sampleparameter""
},
""required"": {
""value"": """"
Expand Down Expand Up @@ -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());
}
}
}
97 changes: 52 additions & 45 deletions src/Bicep.Core/Emit/PlaceholderParametersBicepParamWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void Write(TextWriter writer, string existingContent)

var result = filteredParameterDeclarations
.OfType<ParameterDeclarationSyntax>()
.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<SyntaxBase>() { e, SyntaxFactory.NewlineToken });

var processedSyntaxList = new List<SyntaxBase>()
Expand All @@ -47,25 +47,72 @@ 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, bool isJsonParamWriter = false)
{
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, 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)
{
var value = defaultValueAsArray.Items.Select(e =>
{
if (e.Value is FunctionCallSyntax valueAsFunctionCall)
{
return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName));
return SyntaxFactory.CreateArrayItem(CreateCommentSyntaxForFunctionCallSyntax(valueAsFunctionCall.Name.IdentifierName, isJsonParamWriter)).Value;
}

return e;
return e.Value;
}).ToList();

return SyntaxFactory.CreateArray(value);
Expand Down Expand Up @@ -102,45 +149,5 @@ private SyntaxBase GetValueForParameter(ParameterDeclarationSyntax syntax)

return SyntaxFactory.NewlineToken;
}

private SyntaxBase CheckFunctionCallsInObjectSyntax(ObjectSyntax objectSyntax)
jeskew marked this conversation as resolved.
Show resolved Hide resolved
{
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}()` ");
}
}
}
25 changes: 9 additions & 16 deletions src/Bicep.Core/Emit/PlaceholderParametersJsonWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -97,25 +98,17 @@ private JToken GenerateTemplate(string contentVersion)
{
jsonWriter.WritePropertyName(parameterSymbol.Name);

var value = PlaceholderParametersBicepParamWriter.GetValueForParameter(parameterSymbol.DeclaringParameter, true);

jsonWriter.WriteStartObject();
switch (parameterSymbol.Type.Name)

// 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})
Copy link
Member

Choose a reason for hiding this comment

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

NewLine

What's the significance of TokenType.NewLine for this issue? (This might be the same question as Anthony's comment.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added more context to the comment

{
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();
}

Expand Down
Loading