Handle operator precedence correctly in use-safe-access linter rule…

… codefix (#14710)

Handle operator precedence correctly in `use-safe-access` linter rule
codefix

Closes #14705

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

@@ -51,6 +51,24 @@ param foo object
 """, """
 param foo object
 var test = foo[?'oh-dear'] ?? 'baz'
+""");
+
+    [TestMethod]
+    public void Codefix_is_aware_of_operator_precedence() => AssertCodeFix("""
+param foo object
+var test = contai|ns(foo, 'bar') ? foo.bar : contains(foo, 'baz') ? foo.baz : 'qux'
+""", """
+param foo object
+var test = foo.?bar ?? (contains(foo, 'baz') ? foo.baz : 'qux')
+""");
+
+    [TestMethod]
+    public void Codefix_is_aware_of_operator_precedence_when_not_needed() => AssertCodeFix("""
+param foo object
+var test = contai|ns(foo, 'bar') ? foo.bar : 1 + 2
+""", """
+param foo object
+var test = foo.?bar ?? 1 + 2
 """);
 
     [TestMethod]
@@ -71,5 +89,63 @@ param notTarget string
     public void Rule_ignores_syntax_which_cannot_be_simplified_array_access() => AssertNoDiagnostics("""
 param foo object
 var test = contains(foo, 'bar') ? bar.bar : 'baz'
+""");
+
+    [TestMethod]
+    // https://github.com/Azure/bicep/issues/14705
+    public void Codefix_handles_issue14705_correctly() => AssertCodeFix("""
+@description('Optional. Array of role assignments to create.')
+param roleAssignments roleAssignmentType
+
+var builtInRoleNames = {
+  Contributor: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'b24988ac-6180-42a0-ab88-20f7382dd24c')
+  Owner: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '8e3af657-a8ff-443c-a75c-2fe8c4bcb635')
+  Reader: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'acdd72a7-3385-48ef-bd42-f606fba81ae7')
+}
+
+output formattedRoles array = [
+  for (roleAssignment, index) in (roleAssignments ?? []): union(roleAssignment, {
+    roleDefinitionId: conta|ins(builtInRoleNames, roleAssignment.roleDefinitionIdOrName)
+      ? builtInRoleNames[roleAssignment.roleDefinitionIdOrName]
+      : contains(roleAssignment.roleDefinitionIdOrName, '/providers/Microsoft.Authorization/roleDefinitions/')
+          ? roleAssignment.roleDefinitionIdOrName
+          : subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleAssignment.roleDefinitionIdOrName)
+  })
+]
+
+// ================ //
+// Definitions      //
+// ================ //
+
+type roleAssignmentType = {
+  @description('Required. The role to assign. You can provide either the display name of the role definition, the role definition GUID, or its fully qualified ID in the following format: \'/providers/Microsoft.Authorization/roleDefinitions/c2f4ef07-c644-48eb-af81-4b1b4947fb11\'.')
+  roleDefinitionIdOrName: string
+}[]?
+""", """
+@description('Optional. Array of role assignments to create.')
+param roleAssignments roleAssignmentType
+
+var builtInRoleNames = {
+  Contributor: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'b24988ac-6180-42a0-ab88-20f7382dd24c')
+  Owner: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', '8e3af657-a8ff-443c-a75c-2fe8c4bcb635')
+  Reader: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'acdd72a7-3385-48ef-bd42-f606fba81ae7')
+}
+
+output formattedRoles array = [
+  for (roleAssignment, index) in (roleAssignments ?? []): union(roleAssignment, {
+    roleDefinitionId: builtInRoleNames[?roleAssignment.roleDefinitionIdOrName] ?? (contains(roleAssignment.roleDefinitionIdOrName, '/providers/Microsoft.Authorization/roleDefinitions/')
+          ? roleAssignment.roleDefinitionIdOrName
+          : subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleAssignment.roleDefinitionIdOrName))
+  })
+]
+
+// ================ //
+// Definitions      //
+// ================ //
+
+type roleAssignmentType = {
+  @description('Required. The role to assign. You can provide either the display name of the role definition, the role definition GUID, or its fully qualified ID in the following format: \'/providers/Microsoft.Authorization/roleDefinitions/c2f4ef07-c644-48eb-af81-4b1b4947fb11\'.')
+  roleDefinitionIdOrName: string
+}[]?
 """);
 }

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

@@ -44,10 +44,19 @@ public override IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, Di
                 continue;
             }
 
+            // See https://github.com/Azure/bicep/issues/14705 for context on why this is necessary
+            var rhsNeedsParentheses = ternary.FalseExpression switch
+            {
+                TernaryOperationSyntax => true,
+                BinaryOperationSyntax binaryOperation => 
+                    TokenTypeHelper.GetOperatorPrecedence(binaryOperation.OperatorToken.Type) < TokenTypeHelper.GetOperatorPrecedence(TokenType.DoubleQuestion),
+                _ => false,
+            };
+
             var replacement = SyntaxFactory.CreateBinaryOperationSyntax(
                 SyntaxFactory.CreateSafeAccess(truePropertyAccess.BaseExpression, functionCall.Arguments[1].Expression),
                 TokenType.DoubleQuestion,
-                ternary.FalseExpression);
+                rhsNeedsParentheses ? SyntaxFactory.CreateParenthesized(ternary.FalseExpression) : ternary.FalseExpression);
 
             yield return CreateFixableDiagnosticForSpan(
                 diagnosticLevel,

src/Bicep.Core/Parsing/BaseParser.cs

@@ -63,34 +63,6 @@ TokenType.NewLine or
 
         private static bool CheckKeyword(Token? token, string keyword) => token?.Type == TokenType.Identifier && token.Text == keyword;
 
-        private static int GetOperatorPrecedence(TokenType tokenType) => tokenType switch
-        {
-            // the absolute values are not important here
-            TokenType.Modulo or
-            TokenType.Asterisk or
-            TokenType.Slash => 100,
-
-            TokenType.Plus or
-            TokenType.Minus => 90,
-
-            TokenType.RightChevron or
-            TokenType.GreaterThanOrEqual or
-            TokenType.LeftChevron or
-            TokenType.LessThanOrEqual => 80,
-
-            TokenType.Equals or
-            TokenType.NotEquals or
-            TokenType.EqualsInsensitive or
-            TokenType.NotEqualsInsensitive => 70,
-
-            // if we add bitwise operators in the future, they should go here
-            TokenType.LogicalAnd => 50,
-            TokenType.LogicalOr => 40,
-            TokenType.DoubleQuestion => 30,
-
-            _ => -1,
-        };
-
         protected static RecoveryFlags GetSuppressionFlag(SyntaxBase precedingNode)
         {
             // local function
@@ -246,7 +218,7 @@ private SyntaxBase BinaryExpression(ExpressionFlags expressionFlags, int precede
                 // it could also be the end of file or some other token that is actually valid in this place
                 Token candidateOperatorToken = this.reader.Peek();
 
-                int operatorPrecedence = GetOperatorPrecedence(candidateOperatorToken.Type);
+                int operatorPrecedence = TokenTypeHelper.GetOperatorPrecedence(candidateOperatorToken.Type);
 
                 if (operatorPrecedence <= precedence)
                 {

src/Bicep.Core/Parsing/TokenTypeHelper.cs

@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+namespace Bicep.Core.Parsing;
+
+public static class TokenTypeHelper
+{
+    public static int GetOperatorPrecedence(TokenType tokenType) => tokenType switch
+    {
+        // the absolute values are not important here
+        TokenType.Modulo or
+        TokenType.Asterisk or
+        TokenType.Slash => 100,
+
+        TokenType.Plus or
+        TokenType.Minus => 90,
+
+        TokenType.RightChevron or
+        TokenType.GreaterThanOrEqual or
+        TokenType.LeftChevron or
+        TokenType.LessThanOrEqual => 80,
+
+        TokenType.Equals or
+        TokenType.NotEquals or
+        TokenType.EqualsInsensitive or
+        TokenType.NotEqualsInsensitive => 70,
+
+        // if we add bitwise operators in the future, they should go here
+        TokenType.LogicalAnd => 50,
+        TokenType.LogicalOr => 40,
+        TokenType.DoubleQuestion => 30,
+
+        _ => -1,
+    };
+}

src/Bicep.Core/Syntax/SyntaxFactory.cs

@@ -386,5 +386,8 @@ public static BinaryOperationSyntax CreateBinaryOperationSyntax(SyntaxBase left,
                 left,
                 CreateToken(operatorType, SingleSpaceTrivia, SingleSpaceTrivia),
                 right);
+
+        public static ParenthesizedExpressionSyntax CreateParenthesized(SyntaxBase inner)
+            => new(LeftParenToken, inner, RightParenToken);
     }
 }