Offer every open that resolves the name, and open type where a plain open cannot reach

docs/release-notes/.FSharp.Compiler.Service/11.0.200.md

@@ -8,6 +8,7 @@
 * Capture diagnostics emitted when transparent project checking finishes a project, including FS0247 for namespace-module name collisions. ([Issue #6036](https://github.com/dotnet/fsharp/issues/6036), [PR #20706](https://github.com/dotnet/fsharp/pull/20706))
 * A project reusing cached framework imports is checked with its own `--pathmap` instead of the map of the project that filled the cache, so the ranges of its in-memory reference data name its real files. ([Issue #20474](https://github.com/dotnet/fsharp/issues/20474), [PR #20476](https://github.com/dotnet/fsharp/pull/20476))
 * Split Linux CoreCLR CI into two parallel test batches to reduce test turnaround time. ([PR #20587](https://github.com/dotnet/fsharp/pull/20587))
+* `ParsedInput.FindNearestPointToInsertOpenDeclaration` reports where an `open` belongs from the syntax tree: `InsertionContext.Pos` is the first line inside the scope for every `ScopeKind`, so an `open` no longer lands above a header written as `[<AutoOpen>] module Ns.Name` or below the first declaration when no blank line follows the header. `ParsedInput.AdjustInsertionPoint` only skips the blank line under a header. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500))
 * Fix `NativePtr.stackalloc` used inside the `with` handler, filter or `finally` block of a `try` expression producing an assembly that throws `InvalidProgramException` at method load. Direct or mandatory-inline allocations are now reported at compile time as error FS3924; calls to non-inline allocation helpers, including handler-local helpers, remain legal and retain their method boundary during inlining and let-binding elimination. ([Issue #20295](https://github.com/dotnet/fsharp/issues/20295), [PR #20491](https://github.com/dotnet/fsharp/pull/20491))
 * Preserve SRTP code generation when extension constraint solutions are disabled. ([PR #20773](https://github.com/dotnet/fsharp/pull/20773))
 * `FsiEvaluationSession.EvalInteraction` and `EvalInteractionNonThrowing` evaluate every `;;`-separated interaction in the text they are given, as their documentation has always said, instead of silently discarding everything after the first. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396))
@@ -22,6 +23,7 @@
 
 ### Changed
 
+* `InsertionContextEntity` gains `NamespaceIdentCount`, the number of idents of the entity's full name that its `Namespace` covers, which tells a consumer whether `Namespace` is a namespace or F# module (a plain `open` reaches it) or a type (only `open type` does). Code that constructs the record has to pass it. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501))
 * Remove the unused internal `--test:NoErrorText` switch and its argument-only diagnostic formatting. ([PR #20742](https://github.com/dotnet/fsharp/pull/20742))
 * Assign seven previously preview-only features to [F# 11.2](https://fsharp.github.io/fsharp-compiler-docs/release-notes/Language.html#11.2), now the default language version. `default`, `latest`, and `latestmajor` select F# 11.2; explicit `11` and `11.0` retain F# 11.0 behavior. Preview SDK projects use the same compiler default unless `LangVersion` is explicitly set. `FromEndSlicing` remains preview-only. ([PR #20617](https://github.com/dotnet/fsharp/pull/20617))
 * Add `SynExceptionDefnReprTrivia` with the range of the `exception` keyword to `SynExceptionDefnRepr`. ([Fantomas Issue #3483](https://github.com/fsprojects/fantomas/issues/3483), [PR #20612](https://github.com/dotnet/fsharp/pull/20612))

docs/release-notes/.VisualStudio/18.vNext.md

@@ -10,6 +10,9 @@
 * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482))
 * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503))
 * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128), [PR #20695](https://github.com/dotnet/fsharp/pull/20695))
+* The "open namespace" code fix and the completion that adds an `open` place it from the syntax tree, so it no longer lands above the file's own module declaration (`[<AutoOpen>] module Ns.Name` on one line) or below the first declaration when no blank line follows the header, and both follow the file's line endings. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500))
+* The "open namespace" code fix offers `open type System.Console` where it used to offer `open System.Console`, which does not compile: a plain `open` only reaches namespaces and F# modules. A type inside a generic type is not offered, since `open type` needs type arguments the name does not carry. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501))
+* The "open namespace" code fix lists every namespace and type a name can be resolved from instead of one arbitrary suggestion: an unresolved `File` was answered with `System.Net.WebRequestMethods` and `System.IO` was never offered. As in Roslyn's add-import fix, the list is capped at five opens and three qualifications and ordered with `System` first. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501))
 * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
 * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
 * Avoid using `cancellableTask` in `DocumentCache`; the editor cache now uses direct `CancellationToken`-aware `task` wrappers, avoiding the background `Task.Run` offload and a larger wrapper closure from the `cancellableTask` builder. ([Issue #20268](https://github.com/dotnet/fsharp/issues/20268))

src/Compiler/Service/ServiceParsedInputOps.fs

@@ -158,6 +158,7 @@ type InsertionContextEntity =
         FullRelativeName: string
         Qualifier: string
         Namespace: string option
+        NamespaceIdentCount: int
         FullDisplayName: string
         LastIdent: ShortIdent
     }
@@ -254,12 +255,16 @@ module Entity =
                     | _ ->
                         let fullRelativeName = Array.append (getRelativeNs fullOpenableNs) restIdents
 
+                        let withoutQualifierWritten (idents: ShortIdents) =
+                            if identCount > 1 && relativeNs.Length >= identCount then
+                                idents[0 .. idents.Length - identCount]
+                            else
+                                idents
+
                         let ns =
                             match relativeNs with
                             | [||] -> None
-                            | _ when identCount > 1 && relativeNs.Length >= identCount ->
-                                Some(relativeNs[0 .. relativeNs.Length - identCount] |> String.concat ".")
-                            | _ -> Some(relativeNs |> String.concat ".")
+                            | _ -> Some(withoutQualifierWritten relativeNs |> String.concat ".")
 
                         let qualifier =
                             if fullRelativeName.Length > 1 && fullRelativeName.Length >= identCount then
@@ -272,6 +277,7 @@ module Entity =
                                 FullRelativeName = String.concat "." fullRelativeName //.[0..fullRelativeName.Length - identCount - 1]
                                 Qualifier = String.concat "." qualifier
                                 Namespace = ns
+                                NamespaceIdentCount = (withoutQualifierWritten openableNs).Length
                                 FullDisplayName =
                                     match restIdents with
                                     | [| _ |] -> ""
@@ -2461,9 +2467,9 @@ module ParsedInput =
                             )
                     | _ -> ()
 
-        let getMinColumn decls =
+        let tryGetFirstDeclStart decls =
             match decls with
-            | [] -> None
+            | [] -> ValueNone
             | firstDecl :: _ ->
                 match firstDecl with
                 | SynModuleDecl.NestedModule(range = r)
@@ -2472,9 +2478,16 @@ module ParsedInput =
                 | SynModuleDecl.Types(range = r)
                 | SynModuleDecl.Exception(range = r)
                 | SynModuleDecl.Open(range = r)
-                | SynModuleDecl.HashDirective(range = r) -> Some r
-                | _ -> None
-                |> Option.map (fun r -> r.StartColumn)
+                | SynModuleDecl.HashDirective(range = r) -> ValueSome r.Start
+                | _ -> ValueNone
+
+        let headerEndLine (keyword: range) (ident: LongIdent) (equals: range option) =
+            let nameEndLine =
+                ident
+                |> List.fold (fun line part -> max line part.idRange.EndLine) keyword.EndLine
+
+            equals
+            |> Option.fold (fun line equalsSign -> max line equalsSign.EndLine) nameEndLine
 
         let rec walkImplFileInput (file: ParsedImplFileInput) =
             List.iter (walkSynModuleOrNamespace []) file.Contents
@@ -2496,45 +2509,62 @@ module ParsedInput =
 
                 let fullIdent = parent @ ident
 
-                // Use trivia to get the actual module/namespace keyword line, which excludes attributes
-                let startLine =
+                let lineAboveBody =
                     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
+                    | SynModuleOrNamespaceLeadingKeyword.Module keyword
+                    | SynModuleOrNamespaceLeadingKeyword.Namespace keyword -> headerEndLine keyword ident None
+                    | SynModuleOrNamespaceLeadingKeyword.None -> range.StartLine - 1
 
                 let scopeKind =
                     match isModule, parent with
                     | true, [] -> TopModule
                     | true, _ -> NestedModule
                     | _ -> Namespace
 
-                doRange scopeKind fullIdent startLine range.StartColumn
+                doRange scopeKind fullIdent lineAboveBody range.StartColumn
                 addModule (fullIdent, range)
                 List.iter (walkSynModuleDecl fullIdent) decls
 
         and walkSynModuleDecl (parent: LongIdent) (decl: SynModuleDecl) =
             match decl with
             | SynModuleDecl.NamespaceFragment fragment -> walkSynModuleOrNamespace parent fragment
             | SynModuleDecl.NestedModule(moduleInfo = compInfo; decls = decls; range = range; trivia = trivia) ->
+                let (SynComponentInfo(attributes = attributes)) = compInfo
                 let ident = compInfo.LongIdent
                 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 =
+                    let headerLine =
                         match trivia.ModuleKeyword with
-                        | Some moduleKeywordRange -> moduleKeywordRange.StartLine
+                        | Some moduleKeyword -> headerEndLine moduleKeyword ident trivia.EqualsRange
                         | None -> range.StartLine // Fallback if trivia unavailable
 
+                    let firstDeclStart = tryGetFirstDeclStart decls
+
                     let moduleBodyIndentation =
-                        getMinColumn decls |> Option.defaultValue (range.StartColumn + 4)
+                        firstDeclStart
+                        |> ValueOption.map _.Column
+                        |> ValueOption.defaultValue (range.StartColumn + 4)
+
+                    let bodyStartsOnHeaderLine =
+                        firstDeclStart |> ValueOption.exists (fun start -> start.Line = headerLine)
+
+                    let lineAboveBody =
+                        if bodyStartsOnHeaderLine then
+                            headerLine - 1
+                        else
+                            headerLine
+
+                    let useMayBeInModuleAttribute =
+                        bodyStartsOnHeaderLine
+                        && currentLine = headerLine
+                        && attributes
+                           |> List.exists (fun attributeList -> attributeList.Range.EndLine = headerLine)
 
-                    doRange NestedModule fullIdent moduleKeywordLine moduleBodyIndentation
-                    List.iter (walkSynModuleDecl fullIdent) decls
+                    if headerLine <= currentLine && not useMayBeInModuleAttribute then
+                        doRange NestedModule fullIdent lineAboveBody 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
             | _ -> ()
@@ -2609,50 +2639,12 @@ module ParsedInput =
                 entities
                 |> Array.map (fun e -> e, findBestPositionToInsertOpenDeclaration modules scope pos entity)
 
-    /// Corrects insertion line number based on kind of scope and text surrounding the insertion point.
     let AdjustInsertionPoint (getLineStr: int -> string) ctx =
-        let line =
-            match ctx.ScopeKind with
-            | ScopeKind.TopModule ->
-                if ctx.Pos.Line > 1 then
-                    // it's an implicit module without any open declarations
-                    let line = getLineStr (ctx.Pos.Line - 2)
-
-                    let isImplicitTopLevelModule =
-                        not (line.StartsWithOrdinal("module") && not (line.EndsWithOrdinal("=")))
-
-                    if isImplicitTopLevelModule then 1 else ctx.Pos.Line
-                else
-                    1
-
-            | ScopeKind.Namespace ->
-                // For namespaces the start line is start line of the first nested entity
-                // If we are not on the first line, try to find opening namespace, and return line after it (in F# format)
-                if ctx.Pos.Line > 1 then
-                    [ 0 .. ctx.Pos.Line - 1 ]
-                    |> List.mapi (fun i line -> i, getLineStr line)
-                    |> List.tryPick (fun (i, lineStr) ->
-                        if lineStr.StartsWithOrdinal("namespace") then
-                            Some i
-                        else
-                            None)
-                    |> function
-                        // move to the next line below "namespace" and convert it to F# 1-based line number
-                        | Some line -> line + 2
-                        | None -> ctx.Pos.Line
-                // If we are on 1st line in the namespace ctx, this line _should_ be the namespace declaration, check it and return next line.
-                // Otherwise, return first line (which theoretically should not happen).
-                else
-                    let lineStr = getLineStr (ctx.Pos.Line - 1)
-
-                    if lineStr.StartsWithOrdinal("namespace") then
-                        ctx.Pos.Line + 1
-                    else
-                        ctx.Pos.Line
-
-            | _ -> ctx.Pos.Line
-
-        mkPos line ctx.Pos.Column
+        match ctx.ScopeKind with
+        | ScopeKind.TopModule
+        | ScopeKind.Namespace
+        | ScopeKind.NestedModule when getLineStr (Line.toZ ctx.Pos.Line) = "" -> mkPos (ctx.Pos.Line + 1) ctx.Pos.Column
+        | _ -> ctx.Pos
 
     let FindNearestPointToInsertOpenDeclaration
         (currentLine: int)
@@ -2695,8 +2687,7 @@ module ParsedInput =
                 | _ -> 0
 
             if lastReferenceLine > 0 then
-                // `AdjustInsertionPoint` snaps a `TopModule` position up to line 1, above the directives;
-                // remap it to `HashDirective`, which (like the other scopes) it passes through unchanged.
+                // AdjustInsertionPoint would move a TopModule position below the blank line under the directives.
                 let scopeKind =
                     if ctx.ScopeKind = ScopeKind.TopModule then
                         ScopeKind.HashDirective

src/Compiler/Service/ServiceParsedInputOps.fsi

@@ -131,7 +131,8 @@ type public InsertionContext =
         /// Current scope kind.
         ScopeKind: ScopeKind
 
-        /// Current position (F# compiler line number).
+        /// Where the `open` belongs (F# compiler line number): the first line inside the scope, below its header,
+        /// opens and directives. For a nested module whose body starts on the header's line, that line.
         Pos: pos
     }
 
@@ -162,6 +163,10 @@ type public InsertionContextEntity =
         /// Namespace that is needed to open to make the entity resolvable in the current scope.
         Namespace: string option
 
+        /// How many leading idents of the entity's full name `Namespace` covers: what tells whether a plain `open`
+        /// reaches it (a namespace or F# module) or only `open type` does (a type).
+        NamespaceIdentCount: int
+
         /// Full display name (i.e. last ident plus modules with `RequireQualifiedAccess` attribute prefixed).
         FullDisplayName: string
 
@@ -207,7 +212,8 @@ module public ParsedInput =
     /// Returns long identifier at position.
     val GetLongIdentAt: parsedInput: ParsedInput -> pos: pos -> LongIdent option
 
-    /// Corrects insertion line number based on kind of scope and text surrounding the insertion point.
+    /// Moves the insertion point below the blank line that follows a declaration header. `getLineStr` returns
+    /// the trimmed text of a zero-based line, or an empty string past the end of the file.
     val AdjustInsertionPoint: getLineStr: (int -> string) -> ctx: InsertionContext -> pos
 
 // implementation details used by other code in the compiler