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

Improve parser handling of single-line array/object #4956

Merged
merged 2 commits into from
Oct 23, 2021
Merged

Improve parser handling of single-line array/object #4956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
610ba43
Improve parser handling of single-line array/object
anthony-c-martin Oct 22, 2021
fbf0949
Update test baselines
Oct 22, 2021
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
156 changes: 156 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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