Skip to content

Commit

Permalink
Escape strings correctly in snippets (#14582)
Browse files Browse the repository at this point in the history 
Use the syntax factory + pretty printer to emit snippets rather than
ad-hoc string formatting. This avoids having to deal with edge cases
like escaping.

Closes #14578

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14582)
  • Loading branch information
@anthony-c-martin
anthony-c-martin authored Jul 18, 2024
1 parent d5c6d14 commit f1c7212
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 78 deletions.
4 changes: 2 additions & 2 deletions ...cep.Core.Samples/Files/baselines/InvalidModules_LF/Completions/moduleBodyCompletions.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"detail": "Required properties",
"documentation": {
"kind": "markdown",
"value": "```bicep\n{\n\tname: \n\tparams: {\n\t\tarrayParam: \n\t\tobjParam: {\n\t\t}\n\t\tstringParamB: \n\t}\n}\n``` \n"
"value": "```bicep\n{\n\tname: \n\tparams: {\n\t\tarrayParam: \n\t\tobjParam: {}\n\t\tstringParamB: \n\t}\n}\n``` \n"
},
"deprecated": false,
"preselect": true,
Expand All @@ -82,7 +82,7 @@
"insertTextMode": "adjustIndentation",
"textEdit": {
"range": {},
"newText": "{\n\tname: $1\n\tparams: {\n\t\tarrayParam: $2\n\t\tobjParam: {\n\t\t}\n\t\tstringParamB: $3\n\t}\n}$0"
"newText": "{\n\tname: $1\n\tparams: {\n\t\tarrayParam: $2\n\t\tobjParam: {}\n\t\tstringParamB: $3\n\t}\n}$0"
},
"command": {
"title": "module body completion snippet",
Expand Down
97 changes: 97 additions & 0 deletions src/Bicep.LangServer.IntegrationTests/CompletionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4899,5 +4899,102 @@ await RunCompletionScenarioTest(TestContext, ServerWithNamespaceProvider, fileWi
completionList.Should().SatisfyRespectively(i => i.Label.Should().Be("*"));
});
}

[TestMethod]
public async Task Strings_in_required_property_completions_are_correctly_escaped()
{
var fileWithCursors = """
@discriminator('odata.type')
type alertType = alertWebtestType | alertResourceType | alertMultiResourceType
type alertResourceType = {
'odata.type': 'Microsoft.Azure.Monitor.SingleResourceMultipleMetricCriteria'
allof: array
}
type alertMultiResourceType = {
'odata.type': 'Microsoft.Azure.Monitor.MultipleResourceMultipleMetricCriteria'
allof: array
}
type alertWebtestType = {
'odata.type': 'Microsoft.Azure.Monitor.WebtestLocationAvailabilityCriteria'
componentId: string
failedLocationCount: int
webTestId: string
}
param myAlert alertType = |>
""";

var (text, cursor) = ParserHelper.GetFileWithSingleCursor(fileWithCursors, "|>");
var file = await new ServerRequestHelper(TestContext, ServerWithExtensibilityEnabled).OpenFile(text);

var completions = await file.RequestCompletion(cursor);

var updatedFile = file.ApplyCompletion(completions, "required-properties-Microsoft.Azure.Monitor.WebtestLocationAvailabilityCriteria");
updatedFile.Should().HaveSourceText("""
@discriminator('odata.type')
type alertType = alertWebtestType | alertResourceType | alertMultiResourceType
type alertResourceType = {
'odata.type': 'Microsoft.Azure.Monitor.SingleResourceMultipleMetricCriteria'
allof: array
}
type alertMultiResourceType = {
'odata.type': 'Microsoft.Azure.Monitor.MultipleResourceMultipleMetricCriteria'
allof: array
}
type alertWebtestType = {
'odata.type': 'Microsoft.Azure.Monitor.WebtestLocationAvailabilityCriteria'
componentId: string
failedLocationCount: int
webTestId: string
}
param myAlert alertType = {
componentId: $1
failedLocationCount: $2
'odata.type': 'Microsoft.Azure.Monitor.WebtestLocationAvailabilityCriteria'
webTestId: $3
}|
""");
}

[TestMethod]
public async Task Nested_tab_stops_are_correctly_ordered_in_required_properties_snippet()
{
var fileWithCursors = """
type nestedType = {
foo: string
bar: {
bar: string
}
baz: string
}
param test nestedType = |>
""";

var (text, cursor) = ParserHelper.GetFileWithSingleCursor(fileWithCursors, "|>");
var file = await new ServerRequestHelper(TestContext, ServerWithExtensibilityEnabled).OpenFile(text);

var completions = await file.RequestCompletion(cursor);

var updatedFile = file.ApplyCompletion(completions, "required-properties");
updatedFile.Should().HaveSourceText("""
type nestedType = {
foo: string
bar: {
bar: string
}
baz: string
}
param test nestedType = {
bar: {
bar: $1
}
baz: $2
foo: $3
}|
""");
}
}
}
128 changes: 52 additions & 76 deletions src/Bicep.LangServer/Snippets/SnippetsProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@

using System.Collections.Concurrent;
using System.Collections.Immutable;
using System.DirectoryServices.Protocols;
using System.Text;
using System.Text.RegularExpressions;
using Bicep.Core;
using Bicep.Core.Parsing;
using Bicep.Core.PrettyPrint;
using Bicep.Core.PrettyPrintV2;
using Bicep.Core.Resources;
using Bicep.Core.Syntax;
using Bicep.Core.TypeSystem;
Expand All @@ -25,7 +29,7 @@ public class SnippetsProvider : ISnippetsProvider
// The common properties should be authored consistently to provide for understandability and consumption of the code.
// See https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/best-practices.md#resources
// for more information
private readonly ImmutableArray<string> propertiesSortPreferenceList = ["scope", "parent", "name", "location", "zones", "sku", "kind", "scale", "plan", "identity", "tags", "properties", "dependsOn"];
private static readonly ImmutableArray<string> PropertiesSortPreferenceList = ["scope", "parent", "name", "location", "zones", "sku", "kind", "scale", "plan", "identity", "tags", "properties", "dependsOn"];

private static readonly SnippetCache snippetCache = SnippetCache.FromManifest();

Expand Down Expand Up @@ -119,96 +123,68 @@ private IEnumerable<Snippet> GetRequiredPropertiesSnippetsForDisciminatedObjectT
}
}

private Snippet? GetRequiredPropertiesSnippet(ObjectType objectType, string label, string? discriminatedObjectKey = null)
private static ObjectSyntax GetObjectSnippetSyntax(ObjectType objectType, ref int tabStopIndex, string? discriminatedObjectKey)
{
int index = 1;
StringBuilder sb = new();

var sortedProperties = objectType.Properties.OrderBy(x =>
var typeProperties = objectType.Properties.Values.OrderBy(x =>
PropertiesSortPreferenceList.IndexOf(x.Name) switch {
-1 => int.MaxValue,
int index => index,
})
.Where(TypeHelper.IsRequired);

var objectProperties = new List<ObjectPropertySyntax>();
foreach (var typeProperty in typeProperties)
{
var index = propertiesSortPreferenceList.IndexOf(x.Key);
// Here we deliberately want to iterate in the correct order, and use a DFS approach, to ensure that the tab stops are correctly ordered.
// For example, we want to ensure we output: {\n foo: $1\n nested: {\n bar: $2\n }\n baz: $3\n}
// Instead of: {\n foo: $1\n nested: {\n bar: $3\n }\n baz: $2\n}
objectProperties.Add(GetObjectPropertySnippetSyntax(typeProperty, ref tabStopIndex, discriminatedObjectKey));
}

return (index > -1) ? index : (propertiesSortPreferenceList.Length - 1);
});
return SyntaxFactory.CreateObject(objectProperties);
}

foreach (var (key, value) in sortedProperties)
private static ObjectPropertySyntax GetObjectPropertySnippetSyntax(TypeProperty typeProperty, ref int tabStopIndex, string? discriminatedObjectKey)
{
var valueType = typeProperty.TypeReference.Type;
if (valueType is ObjectType objectType)
{
string? snippetText = GetSnippetText(value, indentLevel: 1, ref index, discriminatedObjectKey);

if (snippetText is not null)
{
sb.Append(snippetText);
}
return SyntaxFactory.CreateObjectProperty(
typeProperty.Name,
GetObjectSnippetSyntax(objectType, ref tabStopIndex, null));
}

if (sb.Length > 0)
else if (discriminatedObjectKey is {} &&
valueType is StringLiteralType stringLiteralType &&
stringLiteralType.Name == discriminatedObjectKey)
{
// Insert open curly at the beginning
sb.Insert(0, "{\n");

// Insert final tab stop outside the top level object
sb.Append("}$0");

return new Snippet(sb.ToString(), CompletionPriority.Medium, label, RequiredPropertiesDescription);
return SyntaxFactory.CreateObjectProperty(
typeProperty.Name,
SyntaxFactory.CreateStringLiteral(stringLiteralType.RawStringValue));
}
else
{
var newTabStopIndex = tabStopIndex++;
return SyntaxFactory.CreateObjectProperty(
typeProperty.Name,
SyntaxFactory.CreateFreeformToken(TokenType.Unrecognized, GetTabStop(newTabStopIndex)));
}

return null;
}

private string? GetSnippetText(TypeProperty typeProperty, int indentLevel, ref int index, string? discrimatedObjectKey = null)
private static string GetTabStop(int index)
=> $"${index}";

private Snippet? GetRequiredPropertiesSnippet(ObjectType objectType, string label, string? discriminatedObjectKey = null)
{
if (TypeHelper.IsRequired(typeProperty))
if (!objectType.Properties.Values.Any(TypeHelper.IsRequired))
{
StringBuilder sb = new();

if (typeProperty.TypeReference.Type is ObjectType objectType)
{
sb.AppendLine(GetIndentString(indentLevel) + typeProperty.Name + ": {");

indentLevel++;

foreach (KeyValuePair<string, TypeProperty> kvp in objectType.Properties.OrderBy(x => x.Key))
{
string? snippetText = GetSnippetText(kvp.Value, indentLevel, ref index);
if (snippetText is not null)
{
sb.Append(snippetText);
}
}

indentLevel--;
sb.AppendLine(GetIndentString(indentLevel) + "}");
}
else
{
string value = ": $" + (index).ToString();
bool shouldIncrementIndent = true;

if (discrimatedObjectKey is not null &&
typeProperty.TypeReference.Type is TypeSymbol typeSymbol &&
typeSymbol.Name == discrimatedObjectKey)
{
value = ": " + discrimatedObjectKey;
shouldIncrementIndent = false;
}

sb.AppendLine(GetIndentString(indentLevel) + typeProperty.Name + value);

if (shouldIncrementIndent)
{
index++;
}
}

return sb.ToString();
return null;
}

return null;
}
var tabStopIndex = 1;
var syntax = GetObjectSnippetSyntax(objectType, ref tabStopIndex, discriminatedObjectKey);

private string GetIndentString(int indentLevel)
{
return new string('\t', indentLevel);
var output = PrettyPrinterV2.PrintValid(syntax, PrettyPrinterV2Options.Default with { IndentKind = IndentKind.Tab }) + GetTabStop(0);
return new Snippet(output, CompletionPriority.Medium, label, RequiredPropertiesDescription);
}

private Snippet GetEmptySnippet()
Expand Down

0 comments on commit f1c7212

Please sign in to comment.