Fix incorrect open statement placement for modules with multiline attributes

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
 

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
 

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

vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs

@@ -30,13 +30,9 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (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 [<ImportingConstructor>] (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

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)
+
+[<Fact>]
+let ``Fixes FS0039 for missing opens - module has multiline attributes`` () =
+    let code =
+        """
+namespace X
+
+open System
+
+[<RequireQualifiedAccess;
+  CompiledName((nameof System.Collections.Immutable.ImmutableArray)
+               + "Module")>]
+module FlatList =
+
+    let a : KeyValuePair<string, int> = KeyValuePair<string, int>("key", 1)
+"""
+
+    let expected =
+        Some
+            {
+                Message = "open System.Collections.Generic"
+                FixedCode =
+                    """
+namespace X
+
+open System
+
+[<RequireQualifiedAccess;
+  CompiledName((nameof System.Collections.Immutable.ImmutableArray)
+               + "Module")>]
+module FlatList =
+
+    open System.Collections.Generic
+
+    let a : KeyValuePair<string, int> = KeyValuePair<string, int>("key", 1)
+"""
+            }
+
+    let actual = codeFix |> tryFix code mode
+
+    Assert.Equal(expected, actual)

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)
+
+[<Fact>]
+let ``Fixes FS0039 for missing opens - module has multiline attributes`` () =
+    let code =
+        """
+namespace X
+
+open System
+
+[<RequireQualifiedAccess;
+  CompiledName((nameof System.Collections.Immutable.ImmutableArray)
+               + "Module")>]
+module FlatList =
+
+    let a : KeyValuePair<string, int> = KeyValuePair<string, int>("key", 1)
+"""
+
+    let expected =
+        Some
+            {
+                Message = "open System.Collections.Generic"
+                FixedCode =
+                    """
+namespace X
+
+open System
+open System.Collections.Generic
+
+[<RequireQualifiedAccess;
+  CompiledName((nameof System.Collections.Immutable.ImmutableArray)
+               + "Module")>]
+module FlatList =
+
+    let a : KeyValuePair<string, int> = KeyValuePair<string, int>("key", 1)
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)