diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md index 233d62d27c3..7e59a14ef88 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.0.md @@ -11,6 +11,7 @@ * Fix units-of-measure changes not invalidating incremental builds. ([Issue #19049](https://github.com/dotnet/fsharp/issues/19049)) * Fix race in graph checking of type extensions. ([PR #19062](https://github.com/dotnet/fsharp/pull/19062)) * Type relations cache: handle unsolved type variables ([Issue #19037](https://github.com/dotnet/fsharp/issues/19037)) ([PR #19040](https://github.com/dotnet/fsharp/pull/19040)) +* Fix insertion context for modules with multiline attributes. ([Issue #18671](https://github.com/dotnet/fsharp/issues/18671)) ### Added diff --git a/docs/release-notes/.VisualStudio/18.0.md b/docs/release-notes/.VisualStudio/18.0.md index 6fb1f603a8e..6344731e3d5 100644 --- a/docs/release-notes/.VisualStudio/18.0.md +++ b/docs/release-notes/.VisualStudio/18.0.md @@ -1,5 +1,6 @@ ### Fixed * Split package init into foreground+background, fix background analysis setting ([Issue #18623](https://github.com/dotnet/fsharp/issues/18623), [Issue #18904](https://github.com/dotnet/fsharp/issues/18904), [PR #18646](https://github.com/dotnet/fsharp/pull/18646)) +* Fix incorrect `open` statement placement for modules with multiline attributes. ([Issue #18671](https://github.com/dotnet/fsharp/issues/18671)) ### Added diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index a209c500bbd..3065bc7f179 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/src/Compiler/Service/ServiceParsedInputOps.fs @@ -2428,7 +2428,7 @@ module ParsedInput = List.iter (walkSynModuleOrNamespace []) file.Contents and walkSynModuleOrNamespace (parent: LongIdent) modul = - let (SynModuleOrNamespace(longId = ident; kind = kind; decls = decls; range = range)) = + let (SynModuleOrNamespace(longId = ident; kind = kind; decls = decls; range = range; trivia = trivia)) = modul if range.EndLine >= currentLine then @@ -2444,7 +2444,14 @@ module ParsedInput = let fullIdent = parent @ ident - let startLine = if isModule then range.StartLine else range.StartLine - 1 + // Use trivia to get the actual module/namespace keyword line, which excludes attributes + let startLine = + match trivia.LeadingKeyword with + | SynModuleOrNamespaceLeadingKeyword.Module moduleRange -> moduleRange.StartLine + | SynModuleOrNamespaceLeadingKeyword.Namespace namespaceRange -> namespaceRange.StartLine - 1 + | SynModuleOrNamespaceLeadingKeyword.None -> + // No keyword (implicit module), use range.StartLine + if isModule then range.StartLine else range.StartLine - 1 let scopeKind = match isModule, parent with @@ -2459,15 +2466,22 @@ module ParsedInput = and walkSynModuleDecl (parent: LongIdent) (decl: SynModuleDecl) = match decl with | SynModuleDecl.NamespaceFragment fragment -> walkSynModuleOrNamespace parent fragment - | SynModuleDecl.NestedModule(moduleInfo = SynComponentInfo(longId = ident); decls = decls; range = range) -> + | SynModuleDecl.NestedModule(moduleInfo = SynComponentInfo(longId = ident); decls = decls; range = range; trivia = trivia) -> + let fullIdent = parent @ ident addModule (fullIdent, range) if range.EndLine >= currentLine then + // Use trivia to get the actual module keyword line, which excludes attributes + let moduleKeywordLine = + match trivia.ModuleKeyword with + | Some moduleKeywordRange -> moduleKeywordRange.StartLine + | None -> range.StartLine // Fallback if trivia unavailable + let moduleBodyIndentation = getMinColumn decls |> Option.defaultValue (range.StartColumn + 4) - doRange NestedModule fullIdent range.StartLine moduleBodyIndentation + doRange NestedModule fullIdent moduleKeywordLine moduleBodyIndentation List.iter (walkSynModuleDecl fullIdent) decls | SynModuleDecl.Open(_, range) -> doRange OpenDeclaration [] range.EndLine (range.StartColumn - 5) | SynModuleDecl.HashDirective(_, range) -> doRange HashDirective [] range.EndLine range.StartColumn diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index 6de1908be48..77727c18684 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -30,13 +30,9 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr Changes = [ TextChange(context.Span, qualifier) ] } - // Hey, I know what you're thinking: this is a horrible hack. - // Indeed it is, this is a better (but still bad) version of the OpenDeclarationHelper. - // The things should be actually fixed in the InsertionContext, it's bugged. - // But currently CompletionProvider also depends on InsertionContext and it's not tested enough. - // So fixing InsertionContext or OpenDeclarationHelper might break completion which would be bad. - // The hack below is at least heavily tested. - // And at least it shows what should be fixed down the line. + // With the fix in ServiceParsedInputOps, the InsertionContext now correctly + // points to the line after the module/namespace keyword (excluding attributes). + // However, we still need to handle implicit top-level modules and nested modules. let getOpenDeclaration (sourceText: SourceText) (ctx: InsertionContext) (ns: string) = // insertion context counts from 2, make the world sane let insertionLineNumber = ctx.Pos.Line - 2 @@ -53,35 +49,12 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr // nested module, shouldn't be here | line when line.StartsWith "module" -> insertionLineNumber, $"{margin}open {ns}{br}{br}" - // attribute, shouldn't be here - | line when line.StartsWith "[<" && line.EndsWith ">]" -> - let moduleDeclLineNumberOpt = - sourceText.Lines - |> Seq.skip insertionLineNumber - |> Seq.tryFindIndex (fun line -> line.ToString().Contains "module") - - match moduleDeclLineNumberOpt with - // implicit top level module - | None -> insertionLineNumber, $"{margin}open {ns}{br}{br}" - // explicit top level module - | Some number -> - // add back the skipped lines - let moduleDeclLineNumber = insertionLineNumber + number - let moduleDeclLineText = sourceText.Lines[moduleDeclLineNumber].ToString().Trim() - - if moduleDeclLineText.EndsWith "=" then - insertionLineNumber, $"{margin}open {ns}{br}{br}" - else - moduleDeclLineNumber + 2, $"{margin}open {ns}{br}{br}" - // implicit top level module | _ -> insertionLineNumber, $"{margin}open {ns}{br}{br}" | ScopeKind.Namespace -> insertionLineNumber + 3, $"{margin}open {ns}{br}{br}" | ScopeKind.NestedModule -> insertionLineNumber + 2, $"{margin}open {ns}{br}{br}" | ScopeKind.OpenDeclaration -> insertionLineNumber + 1, $"{margin}open {ns}{br}" - - // So far I don't know how to get here | ScopeKind.HashDirective -> insertionLineNumber + 1, $"open {ns}{br}{br}" let start = sourceText.Lines[startLineNumber].Start diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs index d3897828292..765b1aa6210 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -432,3 +432,44 @@ let x : RecordType = null let actual = codeFix |> tryFix code mode Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has multiline attributes`` () = + let code = + """ +namespace X + +open System + +[] +module FlatList = + + let a : KeyValuePair = KeyValuePair("key", 1) +""" + + let expected = + Some + { + Message = "open System.Collections.Generic" + FixedCode = + """ +namespace X + +open System + +[] +module FlatList = + + open System.Collections.Generic + + let a : KeyValuePair = KeyValuePair("key", 1) +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index 185fd8390d1..2614a54a493 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -425,3 +425,43 @@ let x : RecordType = null let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has multiline attributes`` () = + let code = + """ +namespace X + +open System + +[] +module FlatList = + + let a : KeyValuePair = KeyValuePair("key", 1) +""" + + let expected = + Some + { + Message = "open System.Collections.Generic" + FixedCode = + """ +namespace X + +open System +open System.Collections.Generic + +[] +module FlatList = + + let a : KeyValuePair = KeyValuePair("key", 1) +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual)