Code formatter for Bicep (Azure#823)

* Implement pretty printer * Hook up document formatting LSP handler * Add a integration test for formatting handler * Remove unused code * Handle insertFinalNewline mismatch between client and server * No need to add client side middleware * Add default formatting configurations * Refactoring * Increase max indent size limit * Fix bugs * Add an integration test * Add formatter round trip test * Use DataSet.IsValid to filter valid data sets * Increase test coverage * Fix test errors
adamgil · Nov 5, 2020 · a89640a · a89640a
1 parent 56bdee6
commit a89640a
Show file tree

Hide file tree

Showing 34 changed files with 2,785 additions and 116 deletions.
diff --git a/src/Bicep.Core.IntegrationTests/Bicep.Core.IntegrationTests.csproj b/src/Bicep.Core.IntegrationTests/Bicep.Core.IntegrationTests.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>netcoreapp3.1</TargetFramework>

diff --git a/src/Bicep.Core.IntegrationTests/PrettyPrint/PrettyPrinterTests.cs b/src/Bicep.Core.IntegrationTests/PrettyPrint/PrettyPrinterTests.cs
@@ -0,0 +1,75 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.IO;
+using System.Text;
+using Bicep.Core.Parser;
+using Bicep.Core.PrettyPrint;
+using Bicep.Core.PrettyPrint.Options;
+using Bicep.Core.Samples;
+using Bicep.Core.Syntax;
+using Bicep.Core.UnitTests.Assertions;
+using Bicep.Core.UnitTests.Utils;
+using FluentAssertions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Bicep.Core.IntegrationTests.PrettyPrint
+{
+    [TestClass]
+    public class PrettyPrinterTests
+    {
+        [NotNull]
+        public TestContext? TestContext { get; set; }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetData), DynamicDataSourceType.Method, DynamicDataDisplayNameDeclaringType = typeof(DataSet), DynamicDataDisplayName = nameof(DataSet.GetDisplayName))]
+        public void PrintProgram_ProgramWithoutDiagnostics_ShouldProduceExpectedOutput(DataSet dataSet)
+        {
+            var program = ParserHelper.Parse(dataSet.Bicep);
+            var options = new PrettyPrintOptions(NewlineOption.Auto, IndentKindOption.Space, 2, true);
+
+            var formattedOutput = PrettyPrinter.PrintProgram(program, options);
+            formattedOutput.Should().NotBeNull();
+
+            var resultsFile = FileHelper.SaveResultFile(this.TestContext!, Path.Combine(dataSet.Name, DataSet.TestFileMainFormatted), formattedOutput!);
+
+            formattedOutput.Should().EqualWithLineByLineDiffOutput(
+                formattedOutput!,
+                expectedLocation: OutputHelper.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainFormatted),
+                actualLocation: resultsFile);
+        }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetData), DynamicDataSourceType.Method, DynamicDataDisplayNameDeclaringType = typeof(DataSet), DynamicDataDisplayName = nameof(DataSet.GetDisplayName))]
+        public void PrintProgram_ProgramWithoutDiagnostics_ShouldRoundTrip(DataSet dataSet)
+        {
+            var program = ParserHelper.Parse(dataSet.Bicep);
+            var options = new PrettyPrintOptions(NewlineOption.Auto, IndentKindOption.Space, 2, true);
+
+            var formattedOutput = PrettyPrinter.PrintProgram(program, options);
+            formattedOutput.Should().NotBeNull();
+
+            // The program should still be parsed without any errors after formatting.
+            var formattedProgram = ParserHelper.Parse(formattedOutput!);
+            formattedProgram.GetParseDiagnostics().Should().BeEmpty();
+
+            var buffer = new StringBuilder();
+            var printVisitor = new PrintVisitor(buffer,x =>
+                // Remove newlines and whitespaces.
+                (x is Token token && token.Type == TokenType.NewLine) ||
+                (x is SyntaxTrivia trivia && trivia.Type == SyntaxTriviaType.Whitespace));
+
+            printVisitor.Visit(program);
+            string programText = buffer.ToString();
+
+            buffer.Clear();
+            printVisitor.Visit(program);
+            string formattedProgramText = buffer.ToString();
+
+            formattedProgramText.Should().Be(programText);
+        }
+
+        private static IEnumerable<object[]> GetData() => DataSets.DataSetsWithNoDiagnostics.ToDynamicTestData();
+    }
+}
diff --git a/src/Bicep.Core.IntegrationTests/PrintVisitor.cs b/src/Bicep.Core.IntegrationTests/PrintVisitor.cs
@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
+using System;
 using System.Collections.Generic;
 using System.Text;
 using Bicep.Core.Parser;
@@ -11,23 +12,39 @@ public class PrintVisitor : SyntaxVisitor
     {
         private readonly StringBuilder buffer;
 
+        private readonly Predicate<IPositionable>? shouldIgnore = null;
+
         public PrintVisitor(StringBuilder buffer)
         {
             this.buffer = buffer;
         }
 
+        public PrintVisitor(StringBuilder buffer, Predicate<IPositionable> shouldIgnore)
+            : this(buffer)
+        {
+            this.shouldIgnore = shouldIgnore;
+        }
+
         public override void VisitToken(Token token)
         {
             WriteTrivia(token.LeadingTrivia);
-            buffer.Append(token.Text);
+
+            if (shouldIgnore == null || !shouldIgnore(token))
+            {
+                buffer.Append(token.Text);
+            }
+
             WriteTrivia(token.TrailingTrivia);
         }
 
         private void WriteTrivia(IEnumerable<SyntaxTrivia> triviaList)
         {
             foreach (var trivia in triviaList)
             {
-                buffer.Append(trivia.Text);
+                if (shouldIgnore == null || !shouldIgnore(trivia))
+                {
+                    buffer.Append(trivia.Text);
+                }
             }
         }
     }

diff --git a/src/Bicep.Core.Samples/DataSet.cs b/src/Bicep.Core.Samples/DataSet.cs
@@ -21,6 +21,7 @@ public class DataSet
         public const string TestFileMainTokens = "main.tokens.bicep";
         public const string TestFileMainSymbols = "main.symbols.bicep";
         public const string TestFileMainSyntax = "main.syntax.bicep";
+        public const string TestFileMainFormatted = "main.formatted.bicep";
         public const string TestFileMainCompiled = "main.json";
         public const string TestCompletionsPrefix = TestCompletionsDirectory + "/";
         public const string TestCompletionsDirectory = "Completions";
@@ -39,6 +40,8 @@ public class DataSet
 
         private readonly Lazy<string> lazySymbols;
 
+        private readonly Lazy<string> lazyFormatted;
+
         private readonly Lazy<ImmutableDictionary<string, string>> lazyCompletions;
 
         public DataSet(string name)
@@ -51,6 +54,7 @@ public DataSet(string name)
             this.lazyCompiled = this.CreateIffValid(TestFileMainCompiled);
             this.lazySymbols = this.CreateRequired(TestFileMainSymbols);
             this.lazySyntax = this.CreateRequired(TestFileMainSyntax);
+            this.lazyFormatted = this.CreateRequired(TestFileMainFormatted);
             this.lazyCompletions = new Lazy<ImmutableDictionary<string, string>>(() => ReadDataSetDictionary(GetStreamName(TestCompletionsPrefix)), LazyThreadSafetyMode.PublicationOnly);
         }
 
@@ -70,6 +74,8 @@ public DataSet(string name)
 
         public string Syntax => this.lazySyntax.Value;
 
+        public string Formatted => this.lazyFormatted.Value;
+
         public ImmutableDictionary<string, string> Completions => this.lazyCompletions.Value;
 
         // validity is set by naming convention

diff --git a/src/Bicep.Core.Samples/DataSets.cs b/src/Bicep.Core.Samples/DataSets.cs
@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -54,6 +55,8 @@ public static class DataSets
                 .Select(property => property.GetValue(null))
                 .Cast<DataSet>();
 
+        public static IEnumerable<DataSet> DataSetsWithNoDiagnostics => AllDataSets.Where(dataSet => dataSet.IsValid);
+
         public static ImmutableDictionary<string, string> Completions => DataSet.ReadDataSetDictionary($"{DataSet.Prefix}{DataSet.TestCompletionsPrefix}");
 
         private static DataSet CreateDataSet([CallerMemberName] string? dataSetName = null) => new DataSet(dataSetName!);

diff --git a/src/Bicep.Core.Samples/Files/AKS_LF/main.formatted.bicep b/src/Bicep.Core.Samples/Files/AKS_LF/main.formatted.bicep
@@ -0,0 +1,60 @@
+// mandatory params
+param dnsPrefix string
+param linuxAdminUsername string
+param sshRSAPublicKey string
+param servcePrincipalClientId string {
+  secure: true
+}
+param servicePrincipalClientSecret string {
+  secure: true
+}
+
+// optional params
+param clusterName string = 'aks101cluster'
+param location string = resourceGroup().location
+param osDiskSizeGB int {
+  default: 0
+  minValue: 0
+  maxValue: 1023
+}
+param agentCount int {
+  default: 3
+  minValue: 1
+  maxValue: 50
+}
+param agentVMSize string = 'Standard_DS2_v2'
+// osType was a defaultValue with only one allowedValue, which seems strange?, could be a good TTK test
+
+resource aks 'Microsoft.ContainerService/managedClusters@2020-03-01' = {
+  name: clusterName
+  location: location
+  properties: {
+    dnsPrefix: dnsPrefix
+    agentPoolProfiles: [
+      {
+        name: 'agentpool'
+        osDiskSizeGB: osDiskSizeGB
+        vmSize: agentVMSize
+        osType: 'Linux'
+        storageProfile: 'ManagedDisks'
+      }
+    ]
+    linuxProfile: {
+      adminUsername: linuxAdminUsername
+      ssh: {
+        publicKeys: [
+          {
+            keyData: sshRSAPublicKey
+          }
+        ]
+      }
+    }
+    servicePrincipalProfile: {
+      clientId: servcePrincipalClientId
+      secret: servicePrincipalClientSecret
+    }
+  }
+}
+
+// fyi - dot property access (aks.fqdn) has not been spec'd
+//output controlPlaneFQDN string = aks.properties.fqdn 
diff --git a/src/Bicep.Core.Samples/Files/Dependencies_LF/main.formatted.bicep b/src/Bicep.Core.Samples/Files/Dependencies_LF/main.formatted.bicep
@@ -0,0 +1,61 @@
+param deployTimeParam string = 'steve'
+var deployTimeVar = 'nigel'
+var dependentVar = {
+  dependencies: [
+    deployTimeVar
+    deployTimeParam
+  ]
+}
+
+var resourceDependency = {
+  dependenciesA: [
+    resA.id
+    resA.name
+    resA.type
+    resA.properties.deployTime
+    resA.eTag
+  ]
+}
+
+output resourceAType string = resA.type
+resource resA 'My.Rp/myResourceType@2020-01-01' = {
+  name: 'resA'
+  properties: {
+    deployTime: dependentVar
+  }
+  eTag: '1234'
+}
+
+output resourceBId string = resB.id
+resource resB 'My.Rp/myResourceType@2020-01-01' = {
+  name: 'resB'
+  properties: {
+    dependencies: resourceDependency
+  }
+}
+
+var resourceIds = {
+  a: resA.id
+  b: resB.id
+}
+
+resource resC 'My.Rp/myResourceType@2020-01-01' = {
+  name: 'resC'
+  properties: {
+    resourceIds: resourceIds
+  }
+}
+
+resource resD 'My.Rp/myResourceType/childType@2020-01-01' = {
+  name: '${resC.name}/resD'
+  properties: {}
+}
+
+resource resE 'My.Rp/myResourceType/childType@2020-01-01' = {
+  name: 'resC/resD'
+  properties: {
+    resDRef: resD.id
+  }
+}
+
+output resourceCProperties object = resC.properties
diff --git a/src/Bicep.Core.Samples/Files/Empty/main.formatted.bicep b/src/Bicep.Core.Samples/Files/Empty/main.formatted.bicep
@@ -0,0 +1 @@
+
diff --git a/src/Bicep.Core.Samples/Files/ModulesWithScopes_LF/main.formatted.bicep b/src/Bicep.Core.Samples/Files/ModulesWithScopes_LF/main.formatted.bicep
@@ -0,0 +1,11 @@
+targetScope='tenant'
+
+module myManagementGroupMod 'modules/managementgroup.bicep' = {
+  name: 'myManagementGroupMod'
+  scope: managementGroup('myManagementGroup')
+}
+
+module mySubscriptionMod 'modules/subscription.bicep' = {
+  name: 'mySubscriptionMod'
+  scope: subscription('ee44cd78-68c6-43d9-874e-e684ec8d1191')
+}
diff --git a/src/Bicep.Core.Samples/Files/Modules_CRLF/main.formatted.bicep b/src/Bicep.Core.Samples/Files/Modules_CRLF/main.formatted.bicep
@@ -0,0 +1,78 @@
+module modATest './modulea.bicep' = {
+  name: 'modATest'
+  params: {
+    stringParamB: 'hello!'
+    objParam: {
+      a: 'b'
+    }
+    arrayParam: [
+      {
+        a: 'b'
+      }
+      'abc'
+    ]
+  }
+}
+
+module modB './child/moduleb.bicep' = {
+  name: 'modB'
+  params: {
+    location: 'West US'
+  }
+}
+
+module optionalWithNoParams1 './child/optionalParams.bicep' = {
+  name: 'optionalWithNoParams1'
+}
+
+module optionalWithNoParams2 './child/optionalParams.bicep' = {
+  name: 'optionalWithNoParams2'
+  params: {}
+}
+
+module optionalWithAllParams './child/optionalParams.bicep' = {
+  name: 'optionalWithNoParams2'
+  params: {
+    optionalString: 'abc'
+    optionalInt: 42
+    optionalObj: {}
+    optionalArray: []
+  }
+}
+
+resource resWithDependencies 'Mock.Rp/mockResource@2020-01-01' = {
+  name: 'harry'
+  properties: {
+    modADep: modATest.outputs.stringOutputA
+    modBDep: modB.outputs.myResourceId
+  }
+}
+
+module optionalWithAllParamsAndManualDependency './child/optionalParams.bicep' = {
+  name: 'optionalWithAllParamsAndManualDependency'
+  params: {
+    optionalString: 'abc'
+    optionalInt: 42
+    optionalObj: {}
+    optionalArray: []
+  }
+  dependsOn: [
+    resWithDependencies
+    optionalWithAllParams
+  ]
+}
+
+module optionalWithImplicitDependency './child/optionalParams.bicep' = {
+  name: 'optionalWithImplicitDependency'
+  params: {
+    optionalString: concat(resWithDependencies.id, optionalWithAllParamsAndManualDependency.name)
+    optionalInt: 42
+    optionalObj: {}
+    optionalArray: []
+  }
+}
+
+output stringOutputA string = modATest.outputs.stringOutputA
+output stringOutputB string = modATest.outputs.stringOutputB
+output objOutput object = modATest.outputs.objOutput
+output arrayOutput array = modATest.outputs.arrayOutput