Provide adding decorators as quickfix. Fixes #1406 (#5544)

* Codefix example

* Made SecureParameterCodeFixProvider more robust

* Trying to fix the tests for CodeActions (after adding the new QuickFix'es)

* Added DescriptionParameterCodeFixProvider

* Refactored and added support for @Allowed, @minlength, @maxlength, @minValue, @MaxValue

* Made ParameterCodeFixProvider more robust

Co-authored-by: Anthony Martin <antmarti@microsoft.com>
Co-authored-by: Hedström, Nils <nils.hedstrom@if.se>

src/Bicep.Core/Semantics/SymbolExtensions.cs

@@ -29,13 +29,18 @@ public static SyntaxBase UnTryGetBodyPropertyValue(this ResourceSymbol resourceS
             => TryGetBodyProperty(moduleSymbol, propertyName)?.Value;
 
         public static bool IsSecure(this ParameterSymbol parameterSymbol)
+        {
+            return HasDecorator(parameterSymbol, "secure");
+        }
+
+        public static bool HasDecorator(this ParameterSymbol parameterSymbol, string decoratorName)
         {
             // local function
-            bool isSecure(DecoratorSyntax? value) => value?.Expression is FunctionCallSyntax functionCallSyntax && functionCallSyntax.NameEquals("secure");
+            bool hasDecorator(DecoratorSyntax? value, string decoratorName) => value?.Expression is FunctionCallSyntax functionCallSyntax && functionCallSyntax.NameEquals(decoratorName);
 
             if (parameterSymbol?.DeclaringSyntax is ParameterDeclarationSyntax paramDeclaration)
             {
-                return paramDeclaration.Decorators.Any(d => isSecure(d));
+                return paramDeclaration.Decorators.Any(d => hasDecorator(d, decoratorName));
             }
             return false;
         }

src/Bicep.LangServer.IntegrationTests/CodeActionTests.cs

@@ -61,7 +61,7 @@ public async Task RequestingCodeActionWithFixableDiagnosticsShouldProduceQuickFi
                     // Assert.
                     quickFixes.Should().NotBeNull();
 
-                    var quickFixList = quickFixes.Where(x => x.CodeAction?.Kind == CodeActionKind.QuickFix).ToList();
+                    var quickFixList = quickFixes.Where(x => x.CodeAction?.Kind == CodeActionKind.QuickFix && x.CodeAction?.IsPreferred == true).ToList();
                     var bicepFixList = fixable.Fixes.ToList();
 
                     quickFixList.Should().HaveSameCount(bicepFixList);
@@ -188,8 +188,7 @@ private async Task VerifyCodeActionIsAvailableToSuppressLinterDiagnostics(string
                 TextDocument = new TextDocumentIdentifier(documentUri),
                 Range = diagnostics.First().ToRange(lineStarts)
             });
-
-            codeActions.Should().SatisfyRespectively(
+            codeActions.Where(c => c.CodeAction?.Kind != CodeActionKind.QuickFix).Should().SatisfyRespectively(
                 x =>
                 {
                     x.CodeAction!.Title.Should().Be("Disable no-unused-params for this line");

src/Bicep.LangServer.IntegrationTests/CodeFixTests.cs

@@ -0,0 +1,183 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using System.Threading.Tasks;
+using Bicep.Core.Extensions;
+using Bicep.Core.Navigation;
+using Bicep.Core.Text;
+using Bicep.Core.UnitTests.Assertions;
+using Bicep.Core.UnitTests.Utils;
+using Bicep.Core.Workspaces;
+using Bicep.LanguageServer.Utils;
+using FluentAssertions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using OmniSharp.Extensions.LanguageServer.Protocol.Client;
+using OmniSharp.Extensions.LanguageServer.Protocol.Document;
+using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
+
+namespace Bicep.LangServer.IntegrationTests
+{
+    [TestClass]
+    public class CodeFixTests
+    {
+        private const string SecureTitle = "Add @secure";
+        private const string DescriptionTitle = "Add @description";
+        private const string AllowedTitle = "Add @allowed";
+        private const string MinLengthTitle = "Add @minLength";
+        private const string MaxLengthTitle = "Add @maxLength";
+        private const string MinValueTitle = "Add @minValue";
+        private const string MaxValueTitle = "Add @maxValue";
+
+        [NotNull]
+        public TestContext? TestContext { get; set; }
+
+        [DataRow("string", "@secure()", SecureTitle)]
+        [DataRow("object", "@secure()", SecureTitle)]
+        [DataRow("string", "@description('')", DescriptionTitle)]
+        [DataRow("object", "@description('')", DescriptionTitle)]
+        [DataRow("array", "@description('')", DescriptionTitle)]
+        [DataRow("bool", "@description('')", DescriptionTitle)]
+        [DataRow("int", "@description('')", DescriptionTitle)]
+        [DataRow("string", "@allowed([])", AllowedTitle)]
+        [DataRow("object", "@allowed([])", AllowedTitle)]
+        [DataRow("array", "@allowed([])", AllowedTitle)]
+        [DataRow("bool", "@allowed([])", AllowedTitle)]
+        [DataRow("int", "@allowed([])", AllowedTitle)]
+        [DataRow("string", "@minLength()", MinLengthTitle)]
+        [DataRow("array", "@minLength()", MinLengthTitle)]
+        [DataRow("string", "@maxLength()", MaxLengthTitle)]
+        [DataRow("array", "@maxLength()", MaxLengthTitle)]
+        [DataRow("int", "@minValue()", MinValueTitle)]
+        [DataRow("int", "@maxValue()", MaxValueTitle)]
+        [DataTestMethod]
+        public async Task Parameter_basic_test(string type, string decorator, string title)
+        {
+            await AssertBasicParameterTest(type, title, decorator);
+        }
+
+        [DataRow("string", "@secure()", SecureTitle)]
+        [DataRow("object", "@secure()", SecureTitle)]
+        [DataRow("string", "@description()", DescriptionTitle)]
+        [DataRow("object", "@description()", DescriptionTitle)]
+        [DataRow("array", "@description()", DescriptionTitle)]
+        [DataRow("bool", "@description()", DescriptionTitle)]
+        [DataRow("int", "@description()", DescriptionTitle)]
+        [DataRow("string", "@allowed([])", AllowedTitle)]
+        [DataRow("object", "@allowed([])", AllowedTitle)]
+        [DataRow("array", "@allowed([])", AllowedTitle)]
+        [DataRow("bool", "@allowed([])", AllowedTitle)]
+        [DataRow("int", "@allowed([])", AllowedTitle)]
+        [DataRow("string", "@minLength()", MinLengthTitle)]
+        [DataRow("object", "@minLength()", MinLengthTitle)]
+        [DataRow("string", "@maxLength()", MaxLengthTitle)]
+        [DataRow("object", "@maxLength()", MaxLengthTitle)]
+        [DataRow("int", "@minValue()", MinValueTitle)]
+        [DataRow("int", "@maxValue()", MaxValueTitle)]
+        [DataTestMethod]
+        public async Task Parameter_do_not_add_duplicate(string type, string decorator, string title)
+        {
+            (var codeActions, var bicepFile) = await TestCodeAction(type, decorator);
+            codeActions.Should().NotContain(x => x.Title == title);
+        }
+
+        [DataRow("array", SecureTitle)]
+        [DataRow("bool", SecureTitle)]
+        [DataRow("int", SecureTitle)]
+        [DataRow("object", MinLengthTitle)]
+        [DataRow("bool", MinLengthTitle)]
+        [DataRow("int", MinLengthTitle)]
+        [DataRow("object", MaxLengthTitle)]
+        [DataRow("bool", MaxLengthTitle)]
+        [DataRow("int", MaxLengthTitle)]
+        [DataRow("object", MinValueTitle)]
+        [DataRow("bool", MinValueTitle)]
+        [DataRow("string", MinValueTitle)]
+        [DataRow("array", MinValueTitle)]
+        [DataRow("object", MaxValueTitle)]
+        [DataRow("bool", MaxValueTitle)]
+        [DataRow("string", MaxValueTitle)]
+        [DataRow("array", MaxValueTitle)]
+        [DataTestMethod]
+        public async Task Parameter_do_not_add_for_unsupported_type(string type, string title)
+        {
+            (var codeActions, var bicepFile) = await TestCodeAction(type);
+            codeActions.Should().NotContain(x => x.Title == title);
+        }
+
+        private async Task AssertBasicParameterTest(string type, string title, string expectedDecorator)
+        {
+            (var codeActions, var bicepFile) = await TestCodeAction(type);
+            codeActions.Should().Contain(x => x.Title == title);
+
+            var updatedFile = ApplyCodeAction(bicepFile, codeActions.Single(x => x.Title == title));
+            updatedFile.Should().HaveSourceText($@"
+{expectedDecorator}
+param foo {type}
+");
+        }
+
+        private static async Task<IEnumerable<CodeAction>> RequestCodeActions(ILanguageClient client, BicepFile bicepFile, int cursor)
+        {
+            var startPosition = TextCoordinateConverter.GetPosition(bicepFile.LineStarts, cursor);
+            var endPosition = TextCoordinateConverter.GetPosition(bicepFile.LineStarts, cursor);
+
+            var result = await client.RequestCodeAction(new CodeActionParams
+            {
+                TextDocument = new TextDocumentIdentifier(bicepFile.FileUri),
+                Range = new Range(startPosition, endPosition),
+            });
+
+            return result.Select(x => x.CodeAction).WhereNotNull();
+        }
+
+        private async Task<(IEnumerable<CodeAction> codeActions, BicepFile bicepFile)> TestCodeAction(string type, string decorator = "")
+        {
+            string fileWithCursors = @$"
+param fo|o {type}
+";
+            if (!String.IsNullOrWhiteSpace(decorator))
+            {
+                fileWithCursors = @$"
+{decorator}
+param fo|o {type}
+";
+            }
+            var (file, cursors) = ParserHelper.GetFileWithCursors(fileWithCursors);
+            var bicepFile = SourceFileFactory.CreateBicepFile(new Uri("file:///main.bicep"), file);
+            using var helper = await LanguageServerHelper.StartServerWithTextAsync(TestContext, file, bicepFile.FileUri);
+            var client = helper.Client;
+
+            var codeActions = await RequestCodeActions(client, bicepFile, cursors.Single());
+            return (codeActions, bicepFile);
+        }
+
+        private static BicepFile ApplyCodeAction(BicepFile bicepFile, CodeAction codeAction, params string[] tabStops)
+        {
+            // only support a small subset of possible edits for now - can always expand this later on
+            codeAction.Edit!.Changes.Should().NotBeNull();
+            codeAction.Edit.Changes.Should().HaveCount(1);
+            codeAction.Edit.Changes.Should().ContainKey(bicepFile.FileUri);
+
+            var changes = codeAction.Edit.Changes![bicepFile.FileUri];
+            changes.Should().HaveCount(1);
+
+            var replacement = changes.Single();
+
+            var start = PositionHelper.GetOffset(bicepFile.LineStarts, replacement.Range.Start);
+            var end = PositionHelper.GetOffset(bicepFile.LineStarts, replacement.Range.End);
+            var textToInsert = replacement.NewText;
+
+            // the handler can contain tabs. convert to double space to simplify printing.
+            textToInsert = textToInsert.Replace("\t", "  ");
+
+            var originalFile = bicepFile.ProgramSyntax.ToTextPreserveFormatting();
+            var replaced = originalFile.Substring(0, start) + textToInsert + originalFile.Substring(end);
+
+            return SourceFileFactory.CreateBicepFile(bicepFile.FileUri, replaced);
+        }
+    }
+}

src/Bicep.LangServer/CodeFixes/ICodeFixProvider.cs

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+using Bicep.Core.CodeAction;
+using Bicep.Core.Semantics;
+using Bicep.Core.Syntax;
+
+namespace Bicep.LanguageServer.CodeFixes
+{
+    public interface ICodeFixProvider
+    {
+        IEnumerable<CodeFix> GetFixes(SemanticModel semanticModel, IReadOnlyList<SyntaxBase> matchingNodes);
+    }
+}

src/Bicep.LangServer/CodeFixes/ParameterCodeFixProvider.cs

@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using Bicep.Core.CodeAction;
+using Bicep.Core.Navigation;
+using Bicep.Core.Parsing;
+using Bicep.Core.PrettyPrint;
+using Bicep.Core.Semantics;
+using Bicep.Core.Syntax;
+
+namespace Bicep.LanguageServer.CodeFixes
+{
+    public class ParameterCodeFixProvider : ICodeFixProvider
+    {
+        private readonly string decoratorName;
+        private readonly string[] supportedTypes;
+        private readonly SyntaxBase[] decoratorParams;
+
+        public ParameterCodeFixProvider(string decoratorName, string[] supportedTypes, SyntaxBase[] decoratorParams)
+        {
+            this.decoratorName = decoratorName;
+            this.supportedTypes = supportedTypes;
+            this.decoratorParams = decoratorParams;
+        }
+
+        public IEnumerable<CodeFix> GetFixes(SemanticModel semanticModel, IReadOnlyList<SyntaxBase> matchingNodes)
+        {
+            if (matchingNodes.OfType<ParameterDeclarationSyntax>().FirstOrDefault() is not {} parameterSyntax ||
+                semanticModel.GetSymbolInfo(parameterSyntax) is not ParameterSymbol parameterSymbol ||
+                parameterSymbol.HasDecorator(decoratorName)) 
+            {
+                yield break;
+            }
+            if(!supportedTypes.Any(t => parameterSyntax.ParameterType?.TypeName == t))
+            {
+                yield break;
+            }
+
+            var decorator = SyntaxFactory.CreateDecorator(decoratorName, decoratorParams);
+            var decoratorText = $"{decorator.ToText()}{Environment.NewLine}";
+            var newSpan = new TextSpan(parameterSyntax.Span.Position, 0);
+            var codeReplacement = new CodeReplacement(newSpan, decoratorText);
+
+            yield return new CodeFix(
+                $"Add @{decoratorName}",
+                false,
+                codeReplacement);
+        }
+    }
+}

src/Bicep.LangServer/Completions/SyntaxMatcher.cs

@@ -3,7 +3,9 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
+using Bicep.Core.Extensions;
 using Bicep.Core.Navigation;
 using Bicep.Core.Parsing;
 using Bicep.Core.Syntax;
@@ -154,6 +156,18 @@ public static List<SyntaxBase> FindNodesMatchingOffset(ProgramSyntax syntax, int
             return nodes;
         }
 
+        public static ImmutableArray<SyntaxBase> FindNodesInRange(ProgramSyntax syntax, int startOffset, int endOffset)
+        {
+            var startNodes = FindNodesMatchingOffset(syntax, startOffset);
+            var endNodes = FindNodesMatchingOffset(syntax, endOffset);
+
+            return startNodes
+                .Zip(endNodes, (x, y) => object.ReferenceEquals(x, y) ? x : null)
+                .TakeWhile(x => x is not null)
+                .WhereNotNull()
+                .ToImmutableArray();
+        }
+
         public static List<SyntaxBase> FindNodesMatchingOffsetExclusive(ProgramSyntax syntax, int offset)
         {
             var nodes = new List<SyntaxBase>();

src/Bicep.LangServer/Handlers/BicepCodeActionHandler.cs

@@ -1,5 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
+
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -10,23 +12,38 @@
 using Bicep.Core.Diagnostics;
 using Bicep.Core.Extensions;
 using Bicep.Core.Parsing;
+using Bicep.Core.Syntax;
 using Bicep.Core.Text;
 using Bicep.Core.Workspaces;
+using Bicep.LanguageServer.CodeFixes;
 using Bicep.LanguageServer.CompilationManager;
+using Bicep.LanguageServer.Completions;
 using Bicep.LanguageServer.Extensions;
 using Bicep.LanguageServer.Telemetry;
 using Bicep.LanguageServer.Utils;
 using OmniSharp.Extensions.LanguageServer.Protocol;
 using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
 using OmniSharp.Extensions.LanguageServer.Protocol.Document;
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
 
 namespace Bicep.LanguageServer.Handlers
 {
     public class BicepCodeActionHandler : CodeActionHandlerBase
     {
         private readonly ICompilationManager compilationManager;
 
+        private static readonly ImmutableArray<ICodeFixProvider> codeFixProviders = new ICodeFixProvider[]
+        {
+            new ParameterCodeFixProvider("secure", new []{"string", "object"}, Array.Empty<SyntaxBase>()),
+            new ParameterCodeFixProvider("description", new []{"string", "object", "array", "bool", "int"}, new []{SyntaxFactory.CreateStringLiteral(String.Empty)}),
+            new ParameterCodeFixProvider("allowed", new []{"string", "object", "array", "bool", "int"}, new []{SyntaxFactory.CreateArray(Array.Empty<SyntaxBase>()) }),
+            new ParameterCodeFixProvider("minLength", new []{"string", "array"}, Array.Empty<SyntaxBase>()),
+            new ParameterCodeFixProvider("maxLength", new []{"string", "array"}, Array.Empty<SyntaxBase>()),
+            new ParameterCodeFixProvider("minValue", new []{"int"}, Array.Empty<SyntaxBase>()),
+            new ParameterCodeFixProvider("maxValue", new []{"int"}, Array.Empty<SyntaxBase>()),
+        }.ToImmutableArray<ICodeFixProvider>();
+
         public BicepCodeActionHandler(ICompilationManager compilationManager)
         {
             this.compilationManager = compilationManager;
@@ -89,6 +106,12 @@ public override Task<CommandOrCodeActionContainer> Handle(CodeActionParams reque
                 }
             }
 
+            var matchingNodes = SyntaxMatcher.FindNodesInRange(compilationContext.ProgramSyntax, requestStartOffset, requestEndOffset);
+            var codeFixes = codeFixProviders
+                .SelectMany(provider => provider.GetFixes(semanticModel, matchingNodes))
+                .Select(fix => CreateQuickFix(request.TextDocument.Uri, compilationContext, fix));
+            commandOrCodeActions.AddRange(codeFixes);
+
             return Task.FromResult(new CommandOrCodeActionContainer(commandOrCodeActions));
         }