diff --git a/src/Bicep.Core/CodeAction/Fixes/DecoratorCodeFixProvider.cs b/src/Bicep.Core/CodeAction/Fixes/DecoratorCodeFixProvider.cs index 57a97096802..1f99c37e2b7 100644 --- a/src/Bicep.Core/CodeAction/Fixes/DecoratorCodeFixProvider.cs +++ b/src/Bicep.Core/CodeAction/Fixes/DecoratorCodeFixProvider.cs @@ -24,7 +24,7 @@ public DecoratorCodeFixProvider(string decoratorName, Decorator decorator) public IEnumerable GetFixes(SemanticModel semanticModel, IReadOnlyList matchingNodes) { - if (matchingNodes.OfType().FirstOrDefault() is not { } decorableSyntax || decorableSyntax.Decorators.Any(IsTargetDecorator)) + if (matchingNodes.OfType().LastOrDefault() is not { } decorableSyntax || decorableSyntax.Decorators.Any(IsTargetDecorator)) { yield break; } @@ -40,10 +40,7 @@ public IEnumerable GetFixes(SemanticModel semanticModel, IReadOnlyList< } var decoratorSyntax = SyntaxFactory.CreateDecorator(decoratorName, GetEmptyParams()); - var newline = semanticModel.Configuration.Formatting.Data.NewlineKind.ToEscapeSequence(); - var decoratorText = $"{decoratorSyntax}{newline}"; - var newSpan = new TextSpan(decorableSyntax.Span.Position, 0); - var codeReplacement = new CodeReplacement(newSpan, decoratorText); + var codeReplacement = CreateCodeReplacement(semanticModel, decorableSyntax, decoratorSyntax); yield return new CodeFix( $"Add @{decoratorName}", @@ -64,14 +61,16 @@ private bool IsTargetDecorator(DecoratorSyntax decoratorSyntax) OutputDeclarationSyntax => FunctionFlags.OutputDecorator, ExtensionDeclarationSyntax => FunctionFlags.ExtensionDecorator, MetadataDeclarationSyntax => FunctionFlags.MetadataDecorator, - TypeDeclarationSyntax or ObjectTypePropertySyntax => FunctionFlags.TypeDecorator, + TypeDeclarationSyntax or ObjectTypePropertySyntax or ObjectTypeAdditionalPropertiesSyntax or TupleTypeItemSyntax => FunctionFlags.TypeDecorator, _ => FunctionFlags.AnyDecorator, }; private TypeSymbol? GetPotentialTargetType(SemanticModel model, DecorableSyntax potentialTarget) => potentialTarget switch { - // The properties of explicitly declared object types will not be bound to a specific symbol, but the TypeManager will have cached the property's type - ObjectTypePropertySyntax objectTypeProperty when model.GetDeclaredType(objectTypeProperty) is { } typePropertyType => typePropertyType, + // Members of explicitly declared aggregate types will not be bound to a specific symbol, + // but the TypeManager will have cached the member's type. + ObjectTypePropertySyntax or ObjectTypeAdditionalPropertiesSyntax or TupleTypeItemSyntax + when model.GetDeclaredType(potentialTarget) is { } typeMemberType => typeMemberType, // Type declaration statements have a type of Type, but decorators evaluate T (e.g., string, not Type) to determine whether they can attach to a given type declaration TypeDeclarationSyntax typeDeclaration when model.GetDeclaredType(typeDeclaration) is { } declaredType => declaredType is TypeType typeType ? typeType.Unwrapped : declaredType, // All other statements should use their assigned type @@ -79,6 +78,28 @@ StatementSyntax declaration when model.GetSymbolInfo(declaration) is DeclaredSym _ => null, }; + private static CodeReplacement CreateCodeReplacement(SemanticModel model, DecorableSyntax syntax, DecoratorSyntax decorator) + { + var formatting = model.Configuration.Formatting.Data; + var newline = formatting.NewlineKind.ToEscapeSequence(); + var position = syntax.Span.Position; + var (_, character) = TextCoordinateConverter.GetPosition(model.SourceFile.LineStarts, position); + var linePrefix = model.SourceFile.Text[(position - character)..position]; + var indentation = linePrefix[..(linePrefix.Length - linePrefix.TrimStart(' ', '\t').Length)]; + + if (indentation.Length == linePrefix.Length) + { + return new(new TextSpan(position, 0), $"{decorator}{newline}{indentation}"); + } + + indentation += formatting.IndentKind == IndentKind.Tab ? "\t" : new string(' ', formatting.IndentSize); + + var whitespaceLength = linePrefix.Length - linePrefix.TrimEnd(' ', '\t').Length; + var replacementSpan = new TextSpan(position - whitespaceLength, whitespaceLength); + + return new(replacementSpan, $"{newline}{indentation}{decorator}{newline}{indentation}"); + } + private SyntaxBase[] GetEmptyParams() { if (decorator.Overload.MinimumArgumentCount == 1 && diff --git a/src/Bicep.LangServer.UnitTests/Handlers/BicepCodeActionHandlerTests.cs b/src/Bicep.LangServer.UnitTests/Handlers/BicepCodeActionHandlerTests.cs index 39b4aa55b5f..5eb3d3be609 100644 --- a/src/Bicep.LangServer.UnitTests/Handlers/BicepCodeActionHandlerTests.cs +++ b/src/Bicep.LangServer.UnitTests/Handlers/BicepCodeActionHandlerTests.cs @@ -128,6 +128,110 @@ public async Task Multiline_string_suggestion_skips_strings_without_newline_char codeActions.Should().NotContain(x => x.Title == MultilineStringCodeFixProvider.Title); } + [DataRow( + """ + @description('Type description.') + type BarConfig = { + ty|pe: 'bar' + value: bool + } + """, + """ + @description('Type description.') + type BarConfig = { + @description('') + type: 'bar' + value: bool + } + """)] + [DataRow( + """ + type config = { + nested: { + val|ue: string + } + } + """, + """ + type config = { + nested: { + @description('') + value: string + } + } + """)] + [DataRow( + """ + type config = { + |*: string + } + """, + """ + type config = { + @description('') + *: string + } + """)] + [DataRow( + """ + type config = [ + str|ing + ] + """, + """ + type config = [ + @description('') + string + ] + """)] + [DataRow( + "type config = { val|ue: string }", + """ + type config = { + @description('') + value: string } + """)] + [DataTestMethod] + public async Task Type_member_decorator_actions_target_the_innermost_decorable_syntax(string fileWithCursor, string expectedText) + { + var (contents, cursor) = ParserHelper.GetFileWithSingleCursor(fileWithCursor); + var bicepFile = GetBicepFile(contents); + var codeAction = await GetSingleCodeAction(bicepFile, cursor, "Add @description"); + + codeAction.Kind.Should().Be(CodeActionKind.Refactor); + LspRefactoringHelper.ApplyCodeAction(bicepFile, codeAction).Text.Should().Be(expectedText); + } + + [TestMethod] + public async Task Type_property_description_action_is_not_suggested_when_property_already_has_description() + { + var (contents, cursor) = ParserHelper.GetFileWithSingleCursor( + """ + type config = { + @description('Property description.') + val|ue: string + } + """); + + var codeActions = await GetCodeActions(GetBicepFile(contents), cursor); + + codeActions.Should().NotContain(x => x.Title == "Add @description"); + } + + [TestMethod] + public async Task Type_declaration_decorator_actions_still_target_the_type_declaration() + { + var (contents, cursor) = ParserHelper.GetFileWithSingleCursor("type con|fig = string"); + var bicepFile = GetBicepFile(contents); + var codeAction = await GetSingleCodeAction(bicepFile, cursor, "Add @description"); + + LspRefactoringHelper.ApplyCodeAction(bicepFile, codeAction).Text.Should().Be( + """ + @description('') + type config = string + """); + } + private static LanguageClientFile GetBicepFile(string contents) => new(new Uri("file:///main.bicep"), contents);