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 @@ provider kubernetes with {
             } 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;