Skip to content

Commit

Permalink
Fixes for various minor decompiler issues (#5887)
Browse files Browse the repository at this point in the history
* Fixes for issues 4896, 5881 & 5630

* Add repro for issue 2569

* Fix for issue 2162

* Add repro for issue 2639

* Fix for issue 2335

* Address PR comments
  • Loading branch information
anthony-c-martin authored Feb 7, 2022
1 parent 8542f75 commit 39bef95
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ param FlowLogName string = 'FlowLog1'
@description('Resource ID of the target NSG')
param existingNSG string

@description('Retention period in days. Default is zero which stands for permanent retention. Can be any Integer from 0 to 365')
@metadata({
description: 'Retention period in days. Default is zero which stands for permanent retention. Can be any Integer from 0 to 365'
range: 'From 0 to 365.'
})
@minValue(0)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
resource foo 'Microsoft.Storage/storageAccounts@2021-01-01' = {
name: 'foo'
kind: 'StorageV2'
location: ''
sku: {
name: 'Standard_RAGRS'
}
}
17 changes: 17 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue2162/main.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"functions": [],
"resources": [
{
"type": "Microsoft.Storage/storageAccounts/",
"apiVersion": "2021-01-01",
"name": "foo",
"kind": "StorageV2",
"location": "",
"sku": {
"name": "Standard_RAGRS"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var foo = '{"fooKey": "bar"}'
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"variables": {
"foo": "[format('{{\"fooKey\": \"{0}\"}}', 'bar')]"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
param siteName string

var apiSiteName = siteName
var apiAppInsightsName = '${siteName}Insights'
13 changes: 13 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue2569/main.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"parameters": {
"siteName": {
"type": "string"
}
},
"variables": {
"apiSiteName": "[concat(parameters('siteName'), '')]",
"apiAppInsightsName": "[concat(parameters('siteName'), 'Insights')]"
}
}
26 changes: 26 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue2639/main.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
param deployAutomationModules string
param automationAccountName string
param location string

var automationModules = {
modules: [
{
Name: 'foo'
url: 'https://foo.com'
}
]
}

resource automationAccountName_automationModules_modules_Name 'Microsoft.Automation/automationAccounts/modules@2015-10-31' = [for i in range(0, length(automationModules.modules)): if (deployAutomationModules == 'true') {
name: '${automationAccountName}/${automationModules.modules[i].Name}'
location: location
properties: {
contentLink: {
uri: automationModules.modules[i].url
}
}
dependsOn: [
resourceId('Microsoft.Automation/automationAccounts/', automationAccountName)
//@[4:81) [BCP034 (Error)] The enclosing array expected an item of type "module[] | (resource | module) | resource[]", but the provided item was of type "string". (CodeDescription: none) |resourceId('Microsoft.Automation/automationAccounts/', automationAccountName)|
]
}]
46 changes: 46 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue2639/main.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"parameters": {
"deployAutomationModules": {
"type": "string"
},
"automationAccountName": {
"type": "string"
},
"location": {
"type": "string"
}
},
"variables": {
"automationModules": {
"modules": [
{
"Name": "foo",
"url": "https://foo.com"
}
]
}
},
"resources": [
{
"condition": "[equals(parameters('deployAutomationModules'),'true')]",
"apiVersion": "2015-10-31",
"type": "Microsoft.Automation/automationAccounts/modules",
"name": "[concat(parameters('automationAccountName'), '/', variables('automationModules').modules[copyIndex()].Name)]",
"location": "[parameters('location')]",
"dependsOn": [
"[resourceId('Microsoft.Automation/automationAccounts/', parameters('automationAccountName'))]"
],
"copy": {
"name": "modulesLoop",
"count": "[length(variables('automationModules').modules)]"
},
"properties": {
"contentLink": {
"uri": "[variables('automationModules').modules[copyIndex()].url]"
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@description('The name of the Resource Group the VM(s) was/were created in.')
output virtualMachinesResourceGroup string = resourceGroup().name
13 changes: 13 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue4896/main.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"outputs": {
"virtualMachinesResourceGroup": {
"type": "string",
"value": "[resourceGroup().name]",
"metadata": {
"description": "The name of the Resource Group the VM(s) was/were created in."
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
var foo = 'abc'
var bar = guid(subscription().id, 'xxxx', foo)
var abc = guid('blah')
var def = {
'1234': '1234'
'${guid('blah')}': guid('blah')
}
13 changes: 13 additions & 0 deletions src/Bicep.Decompiler.IntegrationTests/Working/issue5881/main.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"variables": {
"foo": "abc",
"bar": "[format('{0}', guid(subscription().id, 'xxxx', variables('foo')))]",
"abc": "[format('{0}', guid('blah'))]",
"def": {
"[format('{0}', 1234)]": "[format('{0}', 1234)]",
"[format('{0}', guid('blah'))]": "[format('{0}', guid('blah'))]"
}
}
}
40 changes: 32 additions & 8 deletions src/Bicep.Decompiler/ArmHelpers/ExpressionHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -61,6 +62,32 @@ private static IEnumerable<LanguageExpression> CombineConcatArguments(IEnumerabl
}
}

private static ImmutableArray<(int index, int position, int length)> GetFormatStringHoles(string formatString)
{
var formatHoleMatches = Regex.Matches(formatString, "{([0-9]+)}");
var formatHoles = formatHoleMatches
.Select(match => (
index: int.Parse(match.Groups[1].Value),
position: match.Groups[1].Index,
length: match.Groups[1].Length
));

return formatHoles.ToImmutableArray();
}

private static string UnescapeFormatString(string formatString)
{
var formatHoles = GetFormatStringHoles(formatString);

var maxIndex = formatHoles.Select(x => x.index).Aggregate(Math.Max);
var replacements = Enumerable.Range(0, maxIndex + 1).Select(index => $"{{{index}}}");

// Use string.Format to handle unescaping for us - by passing "{index}" for each hole.
// This will replace '{{' and '}}' with '{' and '}' as appropriate.
// This works because the deployment engine format uses the same algorithm (string.Format) to handle replacement.
return String.Format(formatString, replacements.ToArray());
}

public static LanguageExpression FlattenStringOperations(LanguageExpression original)
{
if (original is not FunctionExpression functionExpression)
Expand All @@ -72,17 +99,14 @@ public static LanguageExpression FlattenStringOperations(LanguageExpression orig
if (functionExpression.NameEquals("format"))
{
var formatString = (functionExpression.Parameters[0] as JTokenExpression)?.Value.Value<string>() ?? throw new ArgumentException($"Unable to read format statement {ExpressionsEngine.SerializeExpression(functionExpression)} as string");
var formatHoleMatches = Regex.Matches(formatString, "{([0-9]+)}");
formatString = UnescapeFormatString(formatString);

var formatHoles = GetFormatStringHoles(formatString);

var concatExpressions = new List<LanguageExpression>();
var nextStart = 0;
for (var i = 0; i < formatHoleMatches.Count; i++)
foreach (var (index, position, length) in formatHoles)
{
var match = formatHoleMatches[i];
var position = match.Groups[1].Index;
var length = match.Groups[1].Length;
var intValue = int.Parse(match.Groups[1].Value);

// compensate for the {
if (nextStart < position - 1)
{
Expand All @@ -91,7 +115,7 @@ public static LanguageExpression FlattenStringOperations(LanguageExpression orig
}

// replace it with the appropriately-numbered expression
concatExpressions.Add(functionExpression.Parameters[intValue + 1]);
concatExpressions.Add(functionExpression.Parameters[index + 1]);

// compensate for the }
nextStart = position + length + 1;
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Decompiler/ArmHelpers/TemplateHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public static JToken AssertRequiredProperty(JObject parent, string propertyName,

public static (string type, string name, string apiVersion) ParseResource(JObject resource)
{
var type = AssertRequiredProperty(resource, "type", $"Unable to parse \"type\" for resource").ToString();
var type = AssertRequiredProperty(resource, "type", $"Unable to parse \"type\" for resource").ToString().Trim('/');
var name = AssertRequiredProperty(resource, "name", $"Unable to parse \"name\" for resource").ToString();
var apiVersion = AssertRequiredProperty(resource, "apiVersion", $"Unable to parse \"apiVersion\" for resource").ToString();

Expand Down
Loading

0 comments on commit 39bef95

Please sign in to comment.