Skip to content

Commit

Permalink
Improve parser handling of single-line array/object (#4956)
Browse files Browse the repository at this point in the history 
* Improve parser handling of single-line array/object

* Update test baselines

Co-authored-by: Bicep Automation <bicep@noreply.github.com>
  • Loading branch information
@anthony-c-martin
anthony-c-martin and Bicep Automation authored Oct 23, 2021
1 parent 5b9c07f commit d7a7438
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 44 deletions.
156 changes: 156 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2794,5 +2794,161 @@ public void Test_Issue1228()
var evaluated = TemplateEvaluator.Evaluate(result.Template);
evaluated.Should().HaveValueAtPath("$.resources[?(@.name == 'Default initiative')].properties.policyDefinitions[0].policyDefinitionId", "/providers/Microsoft.Management/managementGroups/3fc9f36e-8699-43af-b038-1c103980942f/providers/Microsoft.Authorization/policyDefinitions/Allowed locations");
}

// https://github.com/Azure/bicep/issues/4955
[TestMethod]
public void Test_Issue4955()
{
// missing new line at the start and end of the array
var result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': ['hi']
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
// diagnostics for both the start and end of the array missing new lines
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});

// missing new line at the start of the array
result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': ['hi'
]
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});

// missing new line at the end of the array
result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': [
'hi']
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});
}

// https://github.com/Azure/bicep/issues/4955
[TestMethod]
public void Test_Issue4955_objects()
{
// missing new line at the start and end of the object
var result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': {foo: 'bar'}
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
// diagnostics for both the start and end of the object missing new lines
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});

// missing new line at the start of the object
result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': {foo: 'bar'
}
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});

// missing new line at the end of the object
result = CompilationHelper.Compile(@"
targetScope = 'subscription'
resource myRg 'Microsoft.Resources/resourceGroups@2021-04-01' = {
name: 'asdf'
location: 'asdf'
tags: {
'abc': {
foo: 'bar'}
}
}
var otherExp = noValidation
");

result.Template.Should().NotHaveValue();
result.Should().HaveDiagnostics(new[] {
("BCP019", DiagnosticLevel.Error, "Expected a new line character at this location."),
// Ensure we see these two diagnostics as it means that parsing the rest of the document has succeeded
("no-unused-vars", DiagnosticLevel.Warning, "Variable \"otherExp\" is declared but never used."),
("BCP057", DiagnosticLevel.Error, "The name \"noValidation\" does not exist in the current context."),
});
}
}
}
4 changes: 2 additions & 2 deletions src/Bicep.Core.Samples/Files/InvalidModules_LF/main.diagnostics.bicep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ module modWithListKeysInCondition './main.bicep' = if (listKeys('foo', '2020-05-

module modANoName './modulea.bicep' = if ({ 'a': b }.a == true) {
//@[7:17) [BCP028 (Error)] Identifier "modANoName" is declared multiple times. Remove or rename the duplicates. (CodeDescription: none) |modANoName|
//@[51:52) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) |}|
//@[44:44) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) ||
//@[51:51) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) ||

}
//@[1:1) [BCP018 (Error)] Expected the ")" character at this location. (CodeDescription: none) ||

module modANoInputs './modulea.bicep' = {
//@[7:19) [BCP035 (Error)] The specified "module" declaration is missing the following required properties: "params". (CodeDescription: none) |modANoInputs|
Expand Down
44 changes: 24 additions & 20 deletions src/Bicep.Core.Samples/Files/InvalidModules_LF/main.syntax.bicep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,31 +518,35 @@ module modANoName './modulea.bicep' = if ({ 'a': b }.a == true) {
//@[36:37) Assignment |=|
//@[38:68) IfConditionSyntax
//@[38:40) Identifier |if|
//@[41:68) ParenthesizedExpressionSyntax
//@[41:63) ParenthesizedExpressionSyntax
//@[41:42) LeftParen |(|
//@[42:68) ObjectSyntax
//@[42:43) LeftBrace |{|
//@[44:50) ObjectPropertySyntax
//@[44:47) StringSyntax
//@[44:47) StringComplete |'a'|
//@[47:48) Colon |:|
//@[49:50) VariableAccessSyntax
//@[49:50) IdentifierSyntax
//@[49:50) Identifier |b|
//@[51:67) SkippedTriviaSyntax
//@[51:52) RightBrace |}|
//@[42:62) BinaryOperationSyntax
//@[42:54) PropertyAccessSyntax
//@[42:52) ObjectSyntax
//@[42:43) LeftBrace |{|
//@[44:44) SkippedTriviaSyntax
//@[44:50) ObjectPropertySyntax
//@[44:47) StringSyntax
//@[44:47) StringComplete |'a'|
//@[47:48) Colon |:|
//@[49:50) VariableAccessSyntax
//@[49:50) IdentifierSyntax
//@[49:50) Identifier |b|
//@[51:51) SkippedTriviaSyntax
//@[51:52) RightBrace |}|
//@[52:53) Dot |.|
//@[53:54) Identifier |a|
//@[55:57) Equals |==|
//@[53:54) IdentifierSyntax
//@[53:54) Identifier |a|
//@[55:57) Equals |==|
//@[58:62) BooleanLiteralSyntax
//@[58:62) TrueKeyword |true|
//@[62:63) RightParen |)|
//@[64:65) LeftBrace |{|
//@[65:67) NewLine |\n\n|
//@[62:63) RightParen |)|
//@[64:68) ObjectSyntax
//@[64:65) LeftBrace |{|
//@[65:67) NewLine |\n\n|

}
//@[0:1) RightBrace |}|
//@[1:1) SkippedTriviaSyntax
//@[1:1) SkippedTriviaSyntax
//@[0:1) RightBrace |}|
//@[1:3) NewLine |\n\n|

module modANoInputs './modulea.bicep' = {
Expand Down
5 changes: 3 additions & 2 deletions src/Bicep.Core.Samples/Files/InvalidResources_CRLF/main.diagnostics.bicep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ resource foo 'Microsoft.Foo/foos@2020-02-02-alpha'= if (name == 'value') {

resource foo 'Microsoft.Foo/foos@2020-02-02-alpha'= if ({ 'a': b }.a == 'foo') {
//@[9:12) [BCP028 (Error)] Identifier "foo" is declared multiple times. Remove or rename the duplicates. (CodeDescription: none) |foo|
//@[9:12) [BCP035 (Error)] The specified "resource" declaration is missing the following required properties: "name". (CodeDescription: none) |foo|
//@[13:50) [BCP081 (Warning)] Resource type "Microsoft.Foo/foos@2020-02-02-alpha" does not have types available. (CodeDescription: none) |'Microsoft.Foo/foos@2020-02-02-alpha'|
//@[58:58) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) ||
//@[63:64) [BCP057 (Error)] The name "b" does not exist in the current context. (CodeDescription: none) |b|
//@[65:66) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) |}|
//@[65:65) [BCP019 (Error)] Expected a new line character at this location. (CodeDescription: none) ||
}
//@[1:1) [BCP018 (Error)] Expected the ")" character at this location. (CodeDescription: none) ||

// simulate typing if condition
resource foo 'Microsoft.Foo/foos@2020-02-02-alpha'= if
Expand Down
44 changes: 24 additions & 20 deletions src/Bicep.Core.Samples/Files/InvalidResources_CRLF/main.syntax.bicep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,30 +222,34 @@ resource foo 'Microsoft.Foo/foos@2020-02-02-alpha'= if ({ 'a': b }.a == 'foo') {
//@[50:51) Assignment |=|
//@[52:83) IfConditionSyntax
//@[52:54) Identifier |if|
//@[55:83) ParenthesizedExpressionSyntax
//@[55:78) ParenthesizedExpressionSyntax
//@[55:56) LeftParen |(|
//@[56:83) ObjectSyntax
//@[56:57) LeftBrace |{|
//@[58:64) ObjectPropertySyntax
//@[58:61) StringSyntax
//@[58:61) StringComplete |'a'|
//@[61:62) Colon |:|
//@[63:64) VariableAccessSyntax
//@[63:64) IdentifierSyntax
//@[63:64) Identifier |b|
//@[65:82) SkippedTriviaSyntax
//@[65:66) RightBrace |}|
//@[56:77) BinaryOperationSyntax
//@[56:68) PropertyAccessSyntax
//@[56:66) ObjectSyntax
//@[56:57) LeftBrace |{|
//@[58:58) SkippedTriviaSyntax
//@[58:64) ObjectPropertySyntax
//@[58:61) StringSyntax
//@[58:61) StringComplete |'a'|
//@[61:62) Colon |:|
//@[63:64) VariableAccessSyntax
//@[63:64) IdentifierSyntax
//@[63:64) Identifier |b|
//@[65:65) SkippedTriviaSyntax
//@[65:66) RightBrace |}|
//@[66:67) Dot |.|
//@[67:68) Identifier |a|
//@[69:71) Equals |==|
//@[67:68) IdentifierSyntax
//@[67:68) Identifier |a|
//@[69:71) Equals |==|
//@[72:77) StringSyntax
//@[72:77) StringComplete |'foo'|
//@[77:78) RightParen |)|
//@[79:80) LeftBrace |{|
//@[80:82) NewLine |\r\n|
//@[77:78) RightParen |)|
//@[79:83) ObjectSyntax
//@[79:80) LeftBrace |{|
//@[80:82) NewLine |\r\n|
}
//@[0:1) RightBrace |}|
//@[1:1) SkippedTriviaSyntax
//@[1:1) SkippedTriviaSyntax
//@[0:1) RightBrace |}|
//@[1:5) NewLine |\r\n\r\n|

// simulate typing if condition
Expand Down
31 changes: 31 additions & 0 deletions src/Bicep.Core/Parsing/Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,11 @@ private SyntaxBase Array()

var itemsOrTokens = new List<SyntaxBase>();

if (!Check(TokenType.NewLine))
{
itemsOrTokens.Add(SkipEmpty(x => x.ExpectedNewLine()));
}

while (!this.IsAtEnd() && this.reader.Peek().Type != TokenType.RightSquare)
{
// this produces an item node, skipped tokens node, or just a newline token
Expand All @@ -1065,6 +1070,16 @@ private SyntaxBase Array()
itemsOrTokens.Add(skippedSyntax);
}

// we've got a ']' immediately after a property.
// consume it and exit early with an error to avoid assuming we're still inside the object
if (Check(TokenType.RightSquare))
{
itemsOrTokens.Add(SkipEmpty(x => x.ExpectedNewLine()));
var earlyCloseBracket = this.reader.Read();

return new ArraySyntax(openBracket, itemsOrTokens, earlyCloseBracket);
}

// items must be followed by newlines
var newLine = this.WithRecoveryNullable(this.NewLineOrEof, RecoveryFlags.ConsumeTerminator, TokenType.NewLine);
if (newLine != null)
Expand Down Expand Up @@ -1107,6 +1122,12 @@ private ObjectSyntax Object(ExpressionFlags expressionFlags)
}

var propertiesOrResourcesTokens = new List<SyntaxBase>();

if (!Check(TokenType.NewLine))
{
propertiesOrResourcesTokens.Add(SkipEmpty(x => x.ExpectedNewLine()));
}

while (!this.IsAtEnd() && this.reader.Peek().Type != TokenType.RightBrace)
{
// this produces a property node, skipped tokens node, or just a newline token
Expand All @@ -1128,6 +1149,16 @@ private ObjectSyntax Object(ExpressionFlags expressionFlags)
propertiesOrResourcesTokens.Add(skippedSyntax);
}

// we've got a '}' immediately after a property.
// consume it and exit early with an error to avoid assuming we're still inside the object
if (Check(TokenType.RightBrace))
{
propertiesOrResourcesTokens.Add(SkipEmpty(x => x.ExpectedNewLine()));
var earlyCloseBrace = this.reader.Read();

return new ObjectSyntax(openBrace, propertiesOrResourcesTokens, earlyCloseBrace);
}

// properties must be followed by newlines
var newLine = this.WithRecoveryNullable(this.NewLineOrEof, RecoveryFlags.ConsumeTerminator, TokenType.NewLine);
if (newLine != null)
Expand Down

0 comments on commit d7a7438

Please sign in to comment.