Add use-safe-access linter rule (#14322)

Recommends against the following:
```bicep
var test = contains(foo, 'bar') ? foo.bar : 'baz'
```
And instead suggests:

```bicep
var test = foo.?bar ?? 'baz'
```

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14322)

src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/UseSafeAccessRuleTests.cs

@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Bicep.Core.Analyzers.Linter.Rules;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests;
+
+[TestClass]
+public class UseSafeAccessRuleTests : LinterRuleTestsBase
+{
+    private void AssertCodeFix(string inputFile, string resultFile)
+        => AssertCodeFix(UseSafeAccessRule.Code, "Use the safe access (.?) operator", inputFile, resultFile);
+
+    private void AssertNoDiagnostics(string inputFile)
+        => AssertLinterRuleDiagnostics(UseSafeAccessRule.Code, inputFile, [], new(OnCompileErrors.Ignore, IncludePosition.None));
+
+    [TestMethod]
+    public void Codefix_fixes_syntax_which_can_be_simplified() => AssertCodeFix("""
+param foo object
+var test = contai|ns(foo, 'bar') ? foo.bar : 'baz'
+""", """
+param foo object
+var test = foo.?bar ?? 'baz'
+""");
+
+    [TestMethod]
+    public void Codefix_fixes_syntax_which_can_be_simplified_array_access() => AssertCodeFix("""
+param foo object
+param target string
+var test = contai|ns(foo, target) ? foo[target] : 'baz'
+""", """
+param foo object
+param target string
+var test = foo[?target] ?? 'baz'
+""");
+
+    [TestMethod]
+    public void Rule_ignores_syntax_which_cannot_be_simplified() => AssertNoDiagnostics("""
+param foo object
+var test = contains(foo, 'bar') ? foo.baz : 'baz'
+""");
+
+    [TestMethod]
+    public void Rule_ignores_syntax_which_cannot_be_simplified_2() => AssertNoDiagnostics("""
+param foo object
+param target string
+param notTarget string
+var test = contains(foo, target) ? bar[notTarget] : 'baz'
+""");
+
+    [TestMethod]
+    public void Rule_ignores_syntax_which_cannot_be_simplified_array_access() => AssertNoDiagnostics("""
+param foo object
+var test = contains(foo, 'bar') ? bar.bar : 'baz'
+""");
+}

src/Bicep.Core/Analyzers/Linter/Rules/UseSafeAccessRule.cs

@@ -0,0 +1,63 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Bicep.Core.CodeAction;
+using Bicep.Core.Diagnostics;
+using Bicep.Core.Parsing;
+using Bicep.Core.Semantics;
+using Bicep.Core.Semantics.Namespaces;
+using Bicep.Core.Syntax;
+using Bicep.Core.Syntax.Comparers;
+using Bicep.Core.Syntax.Visitors;
+
+namespace Bicep.Core.Analyzers.Linter.Rules;
+
+public sealed class UseSafeAccessRule : LinterRuleBase
+{
+    public new const string Code = "use-safe-access";
+
+    public UseSafeAccessRule() : base(
+        code: Code,
+        description: CoreResources.UseSafeAccessRule_Description,
+        LinterRuleCategory.BestPractice,
+        docUri: new Uri($"https://aka.ms/bicep/linter/{Code}"))
+    { }
+
+    public override IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, DiagnosticLevel diagnosticLevel)
+    {
+        foreach (var ternary in SyntaxAggregator.AggregateByType<TernaryOperationSyntax>(model.Root.Syntax))
+        {
+            if (SemanticModelHelper.TryGetNamedFunction(model, SystemNamespaceType.BuiltInName, "contains", ternary.ConditionExpression) is not {} functionCall ||
+                functionCall.Arguments.Length != 2)
+            {
+                continue;
+            }
+
+            if (ternary.TrueExpression is not AccessExpressionSyntax truePropertyAccess ||
+                !SyntaxIgnoringTriviaComparer.Instance.Equals(functionCall.Arguments[0].Expression, truePropertyAccess.BaseExpression))
+            {
+                continue;
+            }
+
+            if (!truePropertyAccess.AccessExpressionMatches(functionCall.Arguments[1].Expression))
+            {
+                continue;
+            }
+
+            var replacement = SyntaxFactory.CreateBinaryOperationSyntax(
+                SyntaxFactory.CreateSafeAccess(truePropertyAccess.BaseExpression, functionCall.Arguments[1].Expression),
+                TokenType.DoubleQuestion,
+                ternary.FalseExpression);
+
+            yield return CreateFixableDiagnosticForSpan(
+                diagnosticLevel,
+                ternary.Span,
+                new CodeFix(
+                    CoreResources.UseSafeAccessRule_CodeFix,
+                    isPreferred: true,
+                    CodeFixKind.QuickFix,
+                    new CodeReplacement(ternary.Span, replacement.ToString())),
+                CoreResources.UseSafeAccessRule_MessageFormat);
+        }
+    }
+}

src/Bicep.Core/CoreResources.resx

@@ -495,4 +495,13 @@
   <data name="ExperimentalFeatureNames_ResourceDerivedTypes" xml:space="preserve">
     <value>Resource-derived types</value>
   </data>
+  <data name="UseSafeAccessRule_Description" xml:space="preserve">
+    <value>Use the safe access (.?) operator instead of checking object contents with the 'contains' function.</value>
+  </data>
+  <data name="UseSafeAccessRule_CodeFix" xml:space="preserve">
+    <value>Use the safe access (.?) operator</value>
+  </data>
+  <data name="UseSafeAccessRule_MessageFormat" xml:space="preserve">
+    <value>The syntax can be simplified by using the safe access (.?) operator.</value>
+  </data>
 </root>

src/Bicep.Core/Semantics/SemanticModelHelper.cs

@@ -21,6 +21,18 @@ public static IEnumerable<FunctionCallSyntaxBase> GetFunctionsByName(SemanticMod
                 .Where(s => SemanticModelHelper.TryGetFunctionInNamespace(model, @namespace, s) is { });
         }
 
+        public static FunctionCallSyntaxBase? TryGetNamedFunction(SemanticModel model, string @namespace, string functionName, SyntaxBase syntax)
+        {
+            if (syntax is FunctionCallSyntaxBase functionCall &&
+                functionCall.NameEquals(functionName) &&
+                SemanticModelHelper.TryGetFunctionInNamespace(model, @namespace, functionCall) is { })
+            {
+                return functionCall;
+            }
+
+            return null;
+        }
+
         public static FunctionCallSyntaxBase? TryGetFunctionInNamespace(SemanticModel semanticModel, string @namespace, SyntaxBase syntax)
         {
             if (semanticModel.GetSymbolInfo(syntax) is FunctionSymbol function &&

src/Bicep.Core/Syntax/SyntaxExtensions.cs

@@ -2,55 +2,79 @@
 // Licensed under the MIT License.
 using Bicep.Core.Parsing;
 using Bicep.Core.Semantics;
+using Bicep.Core.Syntax.Comparers;
 using Bicep.Core.TypeSystem.Types;
 
-namespace Bicep.Core.Syntax
+namespace Bicep.Core.Syntax;
+
+public static class SyntaxExtensions
 {
-    public static class SyntaxExtensions
+    private static TypeProperty? TryGetTypeProperty(SemanticModel model, SyntaxBase objectSyntax, string propertyName)
     {
-        private static TypeProperty? TryGetTypeProperty(SemanticModel model, SyntaxBase objectSyntax, string propertyName)
+        // Cannot use assigned type here because it won't handle the case where the property value
+        // is an array accesss or a string interpolation.
+        return model.TypeManager.GetDeclaredType(objectSyntax) switch
         {
-            // Cannot use assigned type here because it won't handle the case where the property value
-            // is an array accesss or a string interpolation.
-            return model.TypeManager.GetDeclaredType(objectSyntax) switch
-            {
-                ObjectType { Properties: var properties }
-                    when properties.TryGetValue(propertyName, out var typeProperty) => typeProperty,
-                DiscriminatedObjectType { DiscriminatorKey: var discriminatorKey, DiscriminatorProperty: var typeProperty }
-                    when LanguageConstants.IdentifierComparer.Equals(propertyName, discriminatorKey) => typeProperty,
-                _ => null,
-            };
-        }
+            ObjectType { Properties: var properties }
+                when properties.TryGetValue(propertyName, out var typeProperty) => typeProperty,
+            DiscriminatedObjectType { DiscriminatorKey: var discriminatorKey, DiscriminatorProperty: var typeProperty }
+                when LanguageConstants.IdentifierComparer.Equals(propertyName, discriminatorKey) => typeProperty,
+            _ => null,
+        };
+    }
 
-        public static TypeProperty? TryGetTypeProperty(this ObjectPropertySyntax syntax, SemanticModel model)
+    public static TypeProperty? TryGetTypeProperty(this ObjectPropertySyntax syntax, SemanticModel model)
+    {
+        if (syntax.TryGetKeyText() is not string propertyName || model.Binder.GetParent(syntax) is not ObjectSyntax objectSyntax)
         {
-            if (syntax.TryGetKeyText() is not string propertyName || model.Binder.GetParent(syntax) is not ObjectSyntax objectSyntax)
-            {
-                return null;
-            }
-
-            return TryGetTypeProperty(model, objectSyntax, propertyName);
+            return null;
         }
 
-        /// <remarks>
-        /// If a chain of accesses starts with a "safe" access (e.g., <code><i>base</i>[?0].property</code> or <code><i>base</i>.?some.deeply.nested.property</code>),
-        /// it may short-circuit at runtime, meaning that <code>.deeply.nested.property</code> will only be evaluated if <code><i>base</i>.?some</code> returns a non-null value.
-        /// The upshot of this is that we will need to mark <code><i>base</i>.?some</code> as non-nullable when evaluating any chained property accesses, then
-        /// mark the resultant type as nullable iff the original "safe" access might return null.
-        /// Because of this requirement, it's necessary to evaluate the full access chain and determine if it is kicked off by a .? or [?] operator rather than
-        /// just evaluating <code>syntax.BaseExpression</code> recursively
-        /// </remarks>
-        public static Stack<AccessExpressionSyntax> ToAccessExpressionStack(this AccessExpressionSyntax syntax)
+        return TryGetTypeProperty(model, objectSyntax, propertyName);
+    }
+
+    /// <remarks>
+    /// If a chain of accesses starts with a "safe" access (e.g., <code><i>base</i>[?0].property</code> or <code><i>base</i>.?some.deeply.nested.property</code>),
+    /// it may short-circuit at runtime, meaning that <code>.deeply.nested.property</code> will only be evaluated if <code><i>base</i>.?some</code> returns a non-null value.
+    /// The upshot of this is that we will need to mark <code><i>base</i>.?some</code> as non-nullable when evaluating any chained property accesses, then
+    /// mark the resultant type as nullable iff the original "safe" access might return null.
+    /// Because of this requirement, it's necessary to evaluate the full access chain and determine if it is kicked off by a .? or [?] operator rather than
+    /// just evaluating <code>syntax.BaseExpression</code> recursively
+    /// </remarks>
+    public static Stack<AccessExpressionSyntax> ToAccessExpressionStack(this AccessExpressionSyntax syntax)
+    {
+        Stack<AccessExpressionSyntax> chainedAccesses = new();
+        chainedAccesses.Push(syntax);
+
+        while (chainedAccesses.TryPeek(out var current) && !current.IsSafeAccess && current.BaseExpression is AccessExpressionSyntax baseAccessExpression)
         {
-            Stack<AccessExpressionSyntax> chainedAccesses = new();
-            chainedAccesses.Push(syntax);
+            chainedAccesses.Push(baseAccessExpression);
+        }
+
+        return chainedAccesses;
+    }
 
-            while (chainedAccesses.TryPeek(out var current) && !current.IsSafeAccess && current.BaseExpression is AccessExpressionSyntax baseAccessExpression)
-            {
-                chainedAccesses.Push(baseAccessExpression);
-            }
+    /// <summary>
+    /// Checks whether the a particular access expression is accessing an expression matching the target expression.
+    /// For example, if the target expression is `'bar'`, this would return true for either `foo['bar']` or `foo.bar`.
+    /// If the target expression is `myExpr`. this would return true for `foo[myExpr]`.
+    /// </summary>
+    public static bool AccessExpressionMatches(this AccessExpressionSyntax access, SyntaxBase targetExpression)
+    {
+        if (access is ArrayAccessSyntax arrayAccess &&
+            SyntaxIgnoringTriviaComparer.Instance.Equals(arrayAccess.IndexExpression, targetExpression))
+        {
+            return true;
+        }
 
-            return chainedAccesses;
+        if (access is PropertyAccessSyntax propertyAccess &&
+            targetExpression is StringSyntax stringSyntax &&
+            stringSyntax.TryGetLiteralValue() is { } literalValue &&
+            propertyAccess.PropertyName.NameEquals(literalValue))
+        {
+            return true;
         }
+
+        return false;
     }
 }

src/Bicep.Core/Syntax/SyntaxFactory.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 using System.Collections.Immutable;
+using System.ComponentModel;
 using Bicep.Core.Extensions;
 using Bicep.Core.Parsing;
 
@@ -64,6 +65,7 @@ public static Token GetCommaToken(IEnumerable<SyntaxTrivia>? leadingTrivia = nul
             => CreateToken(TokenType.Comma, leadingTrivia, trailingTrivia);
         public static Token DotToken => CreateToken(TokenType.Dot);
         public static Token QuestionToken => CreateToken(TokenType.Question);
+        public static Token DoubleQuestionToken => CreateToken(TokenType.DoubleQuestion);
         public static Token ColonToken => CreateToken(TokenType.Colon);
         public static Token SemicolonToken => CreateToken(TokenType.Semicolon);
         public static Token AssignmentToken => CreateToken(TokenType.Assignment, EmptyTrivia, SingleSpaceTrivia);
@@ -356,13 +358,30 @@ public static LambdaSyntax CreateLambdaSyntax(IReadOnlyList<string> parameterNam
         };
 
         public static PropertyAccessSyntax CreatePropertyAccess(SyntaxBase @base, string propertyName)
-            => new(@base, DotToken, null, CreateIdentifier(propertyName));
+            => CreatePropertyAccess(@base, false, propertyName);
+
+        public static PropertyAccessSyntax CreatePropertyAccess(SyntaxBase @base, bool safe, string propertyName)
+            => new(@base, DotToken, safe ? QuestionToken : null, CreateIdentifier(propertyName));
+
+        public static ArrayAccessSyntax CreateArrayAccess(SyntaxBase @base, bool safe, SyntaxBase accessExpression)
+            => new(@base, LeftSquareToken, safe ? QuestionToken : null, accessExpression, RightSquareToken);
+
+        public static SyntaxBase CreateSafeAccess(SyntaxBase @base, SyntaxBase accessExpression)
+            => (accessExpression is StringSyntax stringAccess && stringAccess.TryGetLiteralValue() is {} stringValue) ?
+                CreatePropertyAccess(@base, true, stringValue) :
+                CreateArrayAccess(@base, true, accessExpression);
 
         public static ParameterAssignmentSyntax CreateParameterAssignmentSyntax(string name, SyntaxBase value)
             => new(
                 ParameterKeywordToken,
                 CreateIdentifierWithTrailingSpace(name),
                 AssignmentToken,
                 value);
+
+        public static BinaryOperationSyntax CreateBinaryOperationSyntax(SyntaxBase left, TokenType operatorType, SyntaxBase right)
+            => new(
+                left,
+                CreateToken(operatorType, SingleSpaceTrivia, SingleSpaceTrivia),
+                right);
     }
 }

src/vscode-bicep/schemas/bicepconfig.schema.json

@@ -700,6 +700,16 @@
                     }
                   ]
                 },
+                "use-safe-access": {
+                  "allOf": [
+                    {
+                      "description": "Use the safe access (.?) operator instead of checking object contents with the 'contains' function. Defaults to 'Warning'. See https://aka.ms/bicep/linter/use-safe-access"
+                    },
+                    {
+                      "$ref": "#/definitions/rule-def-level-warning"
+                    }
+                  ]
+                },
                 "use-recent-api-versions": {
                   "allOf": [
                     {