Convert with + as keywords into identifiers (#14360)

Noticed when helping @polatengin on modular params that we have also implemented `with` and `as` as dedicated tokens, rather than identifiers. This means that the following cannot be parsed as legitimate Bicep code: ```bicep var with = 'with' ``` Also closes #13347 ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14360)
Azure · Jun 17, 2024 · 338f769 · 338f769
1 parent fece467
commit 338f769
Show file tree

Hide file tree

Showing 21 changed files with 190 additions and 167 deletions.
diff --git a/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs b/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs
@@ -111,7 +111,7 @@ public async Task Provider_Statement_Without_As_Keyword_Should_Raise_Error()
  } something
  """);
  result.Should().HaveDiagnostics(new[] {
- ("BCP012", DiagnosticLevel.Error, "Expected the \"as\" keyword at this location."),
+ ("BCP305", DiagnosticLevel.Error, """Expected the "with" keyword, "as" keyword, or a new line character at this location."""),
  });
  }
 

diff --git a/src/Bicep.Core.Samples/Files/baselines/Imports_LF/main.syntax.bicep b/src/Bicep.Core.Samples/Files/baselines/Imports_LF/main.syntax.bicep
@@ -31,7 +31,7 @@ import * as mod2 from 'modules/mod2.bicep'
 //@[07:016) | ├─WildcardImportSyntax
 //@[07:008) | | ├─Token(Asterisk) |*|
 //@[09:016) | | └─AliasAsClauseSyntax
-//@[09:011) | | ├─Token(AsKeyword) |as|
+//@[09:011) | | ├─Token(Identifier) |as|
 //@[12:016) | | └─IdentifierSyntax
 //@[12:016) | | └─Token(Identifier) |mod2|
 //@[17:042) | └─CompileTimeImportFromClauseSyntax
@@ -50,7 +50,7 @@ import {
 //@[02:032) | | | ├─StringSyntax
 //@[02:032) | | | | └─Token(StringComplete) |'not-a-valid-bicep-identifier'|
 //@[33:057) | | | └─AliasAsClauseSyntax
-//@[33:035) | | | ├─Token(AsKeyword) |as|
+//@[33:035) | | | ├─Token(Identifier) |as|
 //@[36:057) | | | └─IdentifierSyntax
 //@[36:057) | | | └─Token(Identifier) |withInvalidIdentifier|
 //@[57:058) | | ├─Token(NewLine) |\n|

diff --git a/src/Bicep.Core.Samples/Files/baselines/Imports_LF/main.tokens.bicep b/src/Bicep.Core.Samples/Files/baselines/Imports_LF/main.tokens.bicep
@@ -15,7 +15,7 @@ import {foo, fizz, pop, greet} from 'modules/mod.bicep'
 import * as mod2 from 'modules/mod2.bicep'
 //@[00:06) Identifier |import|
 //@[07:08) Asterisk |*|
-//@[09:11) AsKeyword |as|
+//@[09:11) Identifier |as|
 //@[12:16) Identifier |mod2|
 //@[17:21) Identifier |from|
 //@[22:42) StringComplete |'modules/mod2.bicep'|
@@ -26,7 +26,7 @@ import {
 //@[08:09) NewLine |\n|
  'not-a-valid-bicep-identifier' as withInvalidIdentifier
 //@[02:32) StringComplete |'not-a-valid-bicep-identifier'|
-//@[33:35) AsKeyword |as|
+//@[33:35) Identifier |as|
 //@[36:57) Identifier |withInvalidIdentifier|
 //@[57:58) NewLine |\n|
  refersToCopyVariable

diff --git a/src/Bicep.Core.Samples/Files/baselines_bicepparam/Imports/parameters.syntax.bicepparam b/src/Bicep.Core.Samples/Files/baselines_bicepparam/Imports/parameters.syntax.bicepparam
@@ -11,7 +11,7 @@ import * as bicepconfig from 'bicepconfig.bicep'
 //@[007:023) | ├─WildcardImportSyntax
 //@[007:008) | | ├─Token(Asterisk) |*|
 //@[009:023) | | └─AliasAsClauseSyntax
-//@[009:011) | | ├─Token(AsKeyword) |as|
+//@[009:011) | | ├─Token(Identifier) |as|
 //@[012:023) | | └─IdentifierSyntax
 //@[012:023) | | └─Token(Identifier) |bicepconfig|
 //@[024:048) | └─CompileTimeImportFromClauseSyntax

diff --git a/src/Bicep.Core.Samples/Files/baselines_bicepparam/Imports/parameters.tokens.bicepparam b/src/Bicep.Core.Samples/Files/baselines_bicepparam/Imports/parameters.tokens.bicepparam
@@ -5,7 +5,7 @@ using 'main.bicep'
 import * as bicepconfig from 'bicepconfig.bicep'
 //@[000:006) Identifier |import|
 //@[007:008) Asterisk |*|
-//@[009:011) AsKeyword |as|
+//@[009:011) Identifier |as|
 //@[012:023) Identifier |bicepconfig|
 //@[024:028) Identifier |from|
 //@[029:048) StringComplete |'bicepconfig.bicep'|

diff --git a/...icep.Core.Samples/Files/baselines_bicepparam/Invalid_Imports/parameters.syntax.bicepparam b/...icep.Core.Samples/Files/baselines_bicepparam/Invalid_Imports/parameters.syntax.bicepparam
@@ -12,7 +12,7 @@ import * as foo from 'foo.bicep'
 //@[07:15) | ├─WildcardImportSyntax
 //@[07:08) | | ├─Token(Asterisk) |*|
 //@[09:15) | | └─AliasAsClauseSyntax
-//@[09:11) | | ├─Token(AsKeyword) |as|
+//@[09:11) | | ├─Token(Identifier) |as|
 //@[12:15) | | └─IdentifierSyntax
 //@[12:15) | | └─Token(Identifier) |foo|
 //@[16:32) | └─CompileTimeImportFromClauseSyntax

diff --git a/...icep.Core.Samples/Files/baselines_bicepparam/Invalid_Imports/parameters.tokens.bicepparam b/...icep.Core.Samples/Files/baselines_bicepparam/Invalid_Imports/parameters.tokens.bicepparam
@@ -6,7 +6,7 @@ using 'main.bicep'
 import * as foo from 'foo.bicep'
 //@[00:06) Identifier |import|
 //@[07:08) Asterisk |*|
-//@[09:11) AsKeyword |as|
+//@[09:11) Identifier |as|
 //@[12:15) Identifier |foo|
 //@[16:20) Identifier |from|
 //@[21:32) StringComplete |'foo.bicep'|

diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/PreferUnquotedPropertyNamesRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/PreferUnquotedPropertyNamesRuleTests.cs
@@ -7,144 +7,165 @@
 using FluentAssertions;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
-namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests
+namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests;
+
+[TestClass]
+public class PreferUnquotedPropertyNamesRuleTests : LinterRuleTestsBase
 {
- [TestClass]
- public class PreferUnquotedPropertyNamesRuleTests : LinterRuleTestsBase
+ private void AssertNoDiagnostics(string inputFile)
+ => AssertLinterRuleDiagnostics(PreferUnquotedPropertyNamesRule.Code, inputFile, [], new(OnCompileErrors.Ignore, IncludePosition.None));
+
+ private void ExpectPass(string text)
  {
- private void ExpectPass(string text)
- {
- AssertLinterRuleDiagnostics(PreferUnquotedPropertyNamesRule.Code, text, []);
- }
+ AssertLinterRuleDiagnostics(PreferUnquotedPropertyNamesRule.Code, text, []);
+ }
 
- private void ExpectDiagnosticWithFix(string text, string expectedFix)
+ private void ExpectDiagnosticWithFix(string text, string expectedFix)
+ {
+ AssertLinterRuleDiagnostics(PreferUnquotedPropertyNamesRule.Code, text, diags =>
  {
- AssertLinterRuleDiagnostics(PreferUnquotedPropertyNamesRule.Code, text, diags =>
- {
- diags.Should().HaveCount(1, $"expected one fix per testcase");
+ diags.Should().HaveCount(1, $"expected one fix per testcase");
 
-  diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
-  diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
-  diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.First().Text.Should().Be(expectedFix);
-  });
-  }
+ diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
+ diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
+ diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.First().Text.Should().Be(expectedFix);
+ });
+ }
 
-  [DataRow(
-  @"
-  var v1 = {
-  'myProp': {}
-  }",
-  "myProp"
-  )]
-  [DataRow(
-  @"
-  var v1 = {
-  'my_property': {}
-  }",
-  "my_property"
-  )]
-  [DataRow(
-  @"
-  var v1 = {
-  'myProp1': {}
-  }",
-  "myProp1"
-  )]
-  [DataTestMethod]
-  public void ObjectPropertyDeclaration(string text, string expectedFix)
-  {
-  ExpectDiagnosticWithFix(text, expectedFix);
-  }
+ [DataRow(
+ @"
+ var v1 = {
+ 'myProp': {}
+ }",
+ "myProp"
+ )]
+ [DataRow(
+ @"
+ var v1 = {
+ 'my_property': {}
+ }",
+ "my_property"
+ )]
+ [DataRow(
+ @"
+ var v1 = {
+ 'myProp1': {}
+ }",
+ "myProp1"
+ )]
+ [DataTestMethod]
+ public void ObjectPropertyDeclaration(string text, string expectedFix)
+ {
+ ExpectDiagnosticWithFix(text, expectedFix);
+ }
 
-  [DataRow(
-  @"
-  var v1 = {
-  '1': {}
-  }"
-  )]
-  [DataRow(
-  @"
-  var v1 = {
-  'my-property': {}
-  }"
-  )]
-  [DataTestMethod]
-  public void ObjectPropertyDeclaration_NotValidIdentifier(string text)
-  {
-  ExpectPass(text);
-  }
+ [DataRow(
+ @"
+ var v1 = {
+ '1': {}
+ }"
+ )]
+ [DataRow(
+ @"
+ var v1 = {
+ 'my-property': {}
+ }"
+ )]
+ [DataTestMethod]
+ public void ObjectPropertyDeclaration_NotValidIdentifier(string text)
+ {
+ ExpectPass(text);
+ }
 
-  [DataRow(
-  @"
-  var v1 = {
-  v1: {}
-  }"
-  )]
-  [DataRow(
-  @"
-  var v1 = {
-  myProperty: {}
-  }"
-  )]
-  [DataTestMethod]
-  public void ObjectPropertyDeclaration_AlreadyBare(string text)
-  {
-  ExpectPass(text);
-  }
+ [DataRow(
+ @"
+ var v1 = {
+ v1: {}
+ }"
+ )]
+ [DataRow(
+ @"
+ var v1 = {
+ myProperty: {}
+ }"
+ )]
+ [DataTestMethod]
+ public void ObjectPropertyDeclaration_AlreadyBare(string text)
+ {
+ ExpectPass(text);
+ }
 
-  [DataRow(
-  @"
-  param AnObject object
-  var v1 = AnObject['myProp']",
-  ".myProp"
-  )]
-  [DataRow(
-  @"
-  param AnObject object
-  var v1 = AnObject['my_property']",
-  ".my_property"
-  )]
-  [DataRow(
-  @"
-  param AnObject object
-  var v1 = AnObject['myProp1']",
-  ".myProp1"
-  )]
-  [DataTestMethod]
-  public void ObjectPropertyDereference(string text, string expectedFix)
-  {
-  ExpectDiagnosticWithFix(text, expectedFix);
-  }
+ [DataRow(
+ @"
+ param AnObject object
+ var v1 = AnObject['myProp']",
+ ".myProp"
+ )]
+ [DataRow(
+ @"
+ param AnObject object
+ var v1 = AnObject['my_property']",
+ ".my_property"
+ )]
+ [DataRow(
+ @"
+ param AnObject object
+ var v1 = AnObject['myProp1']",
+ ".myProp1"
+ )]
+ [DataTestMethod]
+ public void ObjectPropertyDereference(string text, string expectedFix)
+ {
+ ExpectDiagnosticWithFix(text, expectedFix);
+ }
 
-  [DataRow(
-  @"
-  param AnObject object
-  var v1 = AnObject['1']"
-  )]
-  [DataRow(
-  @"
-  param AnObject object
-  var v1 = AnObject['my-property']"
-  )]
-  [DataTestMethod]
-  public void ObjectPropertyDereference_NotValidIdentifier(string text)
-  {
-  ExpectPass(text);
-  }
+ [DataRow(
+ @"
+ param AnObject object
+ var v1 = AnObject['1']"
+ )]
+ [DataRow(
+ @"
+ param AnObject object
+ var v1 = AnObject['my-property']"
+ )]
+ [DataTestMethod]
+ public void ObjectPropertyDereference_NotValidIdentifier(string text)
+ {
+ ExpectPass(text);
+ }
 
- [DataRow(
- @"
- param AutomationAccountName string
- param AutomationAccountLocation string
- resource AutomationAccount 'Microsoft.Automation/automationAccounts@2020-01-13-preview' = {
- name: AutomationAccountName
- location: AutomationAccountLocation
- }"
- )]
- [DataTestMethod]
- public void NoPropertyDeclarationOrDereference_Passes(string text)
- {
- ExpectPass(text);
- }
+ [DataRow(
+ @"
+ param AutomationAccountName string
+ param AutomationAccountLocation string
+ resource AutomationAccount 'Microsoft.Automation/automationAccounts@2020-01-13-preview' = {
+ name: AutomationAccountName
+ location: AutomationAccountLocation
+ }"
+ )]
+ [DataTestMethod]
+ public void NoPropertyDeclarationOrDereference_Passes(string text)
+ {
+ ExpectPass(text);
  }
+
+ [TestMethod]
+ public void Rule_ignores_non_contextual_keywords_in_object_keys() => AssertNoDiagnostics("""
+var test = {
+ 'null': 'asdf'
+ 'true': 'asdf'
+ 'false': 'asdf'
 }
+""");
+
+ [TestMethod]
+ public void Rule_ignores_non_contextual_keywords_in_object_property_access() => AssertNoDiagnostics("""
+param test object
+var foo = [
+ test['null']
+ test['true']
+ test['false']
+]
+""");
+}
diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/PreferUnquotedPropertyNamesRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/PreferUnquotedPropertyNamesRule.cs
@@ -70,10 +70,12 @@ private void AddCodeFix(TextSpan span, string replacement, string description)
 
  private static bool TryGetValidIdentifierToken(SyntaxBase syntax, [NotNullWhen(true)] out string? validToken)
  {
- if (syntax is StringSyntax @string)
+ if (syntax is StringSyntax stringSyntax &&
+ stringSyntax.TryGetLiteralValue() is {} literalValue)
  {
- string? literalValue = @string.TryGetLiteralValue();
- if (literalValue is not null && Lexer.IsValidIdentifier(literalValue))
+ if (Lexer.IsValidIdentifier(literalValue) &&
+ // exclude non-contextual keywords like 'nul and 'true' - see https://github.com/Azure/bicep/issues/13347.
+ !LanguageConstants.NonContextualKeywords.ContainsKey(literalValue))
  {
  validToken = literalValue;
  return true;