Fix open declaration insertion in .fsx scripts after #r/#load directives (#16271)

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

@@ -81,6 +81,7 @@
 * Reference assembly MVIDs are now deterministic across compiler invocations. Previously, `--refout` / `<ProduceReferenceAssembly>true</ProduceReferenceAssembly>` produced a different MVID every build because the implied signature hash used .NET's randomized `String.GetHashCode()`. ([Issue #19751](https://github.com/dotnet/fsharp/issues/19751), [PR #19801](https://github.com/dotnet/fsharp/pull/19801))
 * Parser: recover on unfinished if and binary expressions
 ([PR #19724](https://github.com/dotnet/fsharp/pull/19724))
+* Fix `open` declaration insertion in `.fsx`/`.fsscript` scripts being placed before `#r`/`#load` directives, producing invalid script files. ([Issue #16271](https://github.com/dotnet/fsharp/issues/16271), [PR #19879](https://github.com/dotnet/fsharp/pull/19879))
 * Warn FS3888 when a compiler-semantic attribute on a value/member or type/module is present in the `.fs` but missing from the `.fsi`. Such attributes were previously ignored at the consumer side. Under the `ErrorOnMissingSignatureAttribute` preview language feature, FS3888 is an error. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880))
 * Emit debug points at a stack-empty position ([PR #19877](https://github.com/dotnet/fsharp/pull/19877))
 * Fix spurious XmlDoc warnings (unknown parameter / no documentation for parameter) under `--warnon:3390` when a get/set property documents the full parameter set across both accessors. ([Issue #13684](https://github.com/dotnet/fsharp/issues/13684), [PR #19884](https://github.com/dotnet/fsharp/pull/19884))

src/Compiler/Service/ServiceParsedInputOps.fs

@@ -2608,11 +2608,53 @@ module ParsedInput =
         (entity: ShortIdents)
         (insertionPoint: OpenStatementInsertionPoint)
         =
-        match tryFindNearestPointAndModules currentLine parsedInput insertionPoint with
-        | Some(scope, _, point), modules -> findBestPositionToInsertOpenDeclaration modules scope point entity
-        | _ ->
-            // we failed to find insertion point because ast is empty for some reason, return top left point in this case
-            {
-                ScopeKind = ScopeKind.TopModule
-                Pos = mkPos 1 0
-            }
+        let ctx =
+            match tryFindNearestPointAndModules currentLine parsedInput insertionPoint with
+            | Some(scope, _, point), modules -> findBestPositionToInsertOpenDeclaration modules scope point entity
+            | _ ->
+                // we failed to find insertion point because ast is empty for some reason, return top left point in this case
+                {
+                    ScopeKind = ScopeKind.TopModule
+                    Pos = mkPos 1 0
+                }
+
+        // In scripts, leading `#r`/`#reference`/`#load` directives must precede any `open`.
+        match parsedInput with
+        | ParsedInput.ImplFile impl when impl.IsScript ->
+            let isReferenceOrLoad =
+                function
+                | "r"
+                | "reference"
+                | "load" -> true
+                | _ -> false
+
+            let lastReferenceLine =
+                match impl.Contents with
+                | SynModuleOrNamespace(decls = decls) :: _ ->
+                    decls
+                    |> List.takeWhile (function
+                        | SynModuleDecl.HashDirective _ -> true
+                        | _ -> false)
+                    |> List.choose (function
+                        | SynModuleDecl.HashDirective(ParsedHashDirective(ident, _, _), range) when isReferenceOrLoad ident ->
+                            Some range.EndLine
+                        | _ -> None)
+                    |> List.fold max 0
+                | _ -> 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.
+                let scopeKind =
+                    if ctx.ScopeKind = ScopeKind.TopModule then
+                        ScopeKind.HashDirective
+                    else
+                        ctx.ScopeKind
+
+                {
+                    ScopeKind = scopeKind
+                    Pos = mkPos (max (lastReferenceLine + 1) ctx.Pos.Line) ctx.Pos.Column
+                }
+            else
+                ctx
+        | _ -> ctx

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj

@@ -74,6 +74,7 @@
     <Compile Include="AssemblyContentProviderTests.fs" />
     <Compile Include="TreeVisitorTests.fs" />
     <Compile Include="ParsedInputModuleTests.fs" />
+    <Compile Include="OpenDeclarationInsertionTests.fs" />
     <Compile Include="ParsedInputModuleTests.VeryBigArrayExprTest.fs" />
     <Compile Include="FSharpExprPatternsTests.fs" />
     <Compile Include="SourceTextTests.fs" />

tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs

@@ -0,0 +1,188 @@
+module FSharp.Compiler.Service.Tests.OpenDeclarationInsertionTests
+
+open Xunit
+open FSharp.Compiler.EditorServices
+open FSharp.Compiler.Service.Tests.Common
+
+/// Computes the position an `open` would be inserted at, mirroring the real editor consumer
+/// (RoslynHelpers.insertContext): `FindNearestPointToInsertOpenDeclaration` followed by
+/// `AdjustInsertionPoint`. Asserting on the adjusted position is what the user actually sees.
+let private findOpenInsertion (insertionPoint: OpenStatementInsertionPoint) (fileName: string) (source: string) (entity: string) =
+    let parseTree = parseSourceCode (fileName, source)
+
+    let lines = source.Replace("\r\n", "\n").Split('\n')
+
+    let currentLine =
+        let mutable i = lines.Length - 1
+        while i > 0 && System.String.IsNullOrWhiteSpace(lines.[i]) do
+            i <- i - 1
+        i + 1
+
+    let idents = entity.Split('.')
+
+    let ctx =
+        ParsedInput.FindNearestPointToInsertOpenDeclaration currentLine parseTree idents insertionPoint
+
+    let getLineStr line =
+        if line >= 0 && line < lines.Length then lines.[line].Trim() else ""
+
+    let pos = ParsedInput.AdjustInsertionPoint getLineStr ctx
+    pos.Line, pos.Column
+
+let private findOpenInsertionLine fileName source entity =
+    findOpenInsertion OpenStatementInsertionPoint.TopLevel fileName source entity |> fst
+
+[<Fact>]
+let ``Open placed after r directives in fsx`` () =
+    let source = """#r "nuget: Newtonsoft.Json"
+#r "nuget: FSharp.Data"
+
+let x : Newtonsoft.Json.JsonConvert = failwith ""
+"""
+    let line = findOpenInsertionLine "test.fsx" source "Newtonsoft.Json"
+    // Right after the last #r directive (line 2), never before it.
+    Assert.Equal(3, line)
+
+[<Fact>]  // NEGATIVE: .fs files unaffected
+let ``Open in fs file placed at top`` () =
+    let source = """module Test
+let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fs" source "System.IO"
+    Assert.Equal(2, line)  // right after module decl
+
+[<Fact>]
+let ``Open after r and load directives`` () =
+    let source = """#r "nuget: FSharp.Data"
+#load "helper.fsx"
+
+let x = 1
+"""
+    let line = findOpenInsertionLine "test.fsx" source "SomeNs"
+    Assert.Equal(3, line)
+
+[<Fact>]
+let ``No r directives in fsx inserts at top`` () =
+    let source = """let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    Assert.Equal(1, line)
+
+[<Fact>]
+let ``No r directives in multiline fsx inserts at top`` () =
+    let source = """let y = 1
+
+let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    Assert.Equal(1, line)
+
+[<Fact>]
+let ``Open after r with existing opens`` () =
+    let source = """#r "nuget: FSharp.Data"
+open System
+
+let x : System.IO.File = failwith ""
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    // Grouped with the existing `open System` (line 2), still after the #r directive.
+    Assert.Equal(2, line)
+
+[<Fact>]
+let ``Open grouped with existing opens below header comment`` () =
+    let source = """// My script header
+open System
+
+let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    // Grouped after existing `open System` (line 2), not forced to line 1.
+    Assert.Equal(3, line)
+
+[<Fact>]
+let ``Open placed after r directives in fsscript`` () =
+    let source = """#r "nuget: Newtonsoft.Json"
+#r "nuget: FSharp.Data"
+
+let x : Newtonsoft.Json.JsonConvert = failwith ""
+"""
+    let line = findOpenInsertionLine "test.fsscript" source "Newtonsoft.Json"
+    Assert.Equal(3, line)
+
+[<Fact>]  // Regression: must not force the open above a named module (would not compile)
+let ``Open not forced above named module in fsx`` () =
+    let source = """module Foo
+
+let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    // Below the `module Foo` header (line 1), inside the module.
+    Assert.Equal(2, line)
+
+[<Fact>]  // Only #r/#load drive placement; other directives (#time/#help/#I/...) must not
+let ``Non reference directive does not drive placement in fsx`` () =
+    let source = """#time "on"
+
+let x = System.IO.File.ReadAllText "a"
+"""
+    let line = findOpenInsertionLine "test.fsx" source "System.IO"
+    // #time is not a reference directive, so the open is placed by the normal top-of-file logic.
+    Assert.Equal(1, line)
+
+[<Fact>]  // #reference is an alias of #r; the open must still land after it
+let ``Open placed after reference directive in fsx`` () =
+    let source = """#reference "nuget: Newtonsoft.Json"
+
+let x : Newtonsoft.Json.JsonConvert = failwith ""
+"""
+    let line = findOpenInsertionLine "test.fsx" source "Newtonsoft.Json"
+    Assert.Equal(2, line)
+
+[<Fact>]  // #I precedes #r; the open still lands after the last #r, not merely after #I
+let ``Open after r preceded by I directive in fsx`` () =
+    let source = """#I "/tmp"
+#r "nuget: FSharp.Data"
+
+let x : Newtonsoft.Json.JsonConvert = failwith ""
+"""
+    let line = findOpenInsertionLine "test.fsx" source "Newtonsoft.Json"
+    Assert.Equal(3, line)
+[<Fact>]  // comments / blank lines between directives are trivia, not decls: scan still spans both
+let ``Open after r and load separated by comment and blank line in fsx`` () =
+    let source = """#r "nuget: FSharp.Data"
+
+// pull in a helper
+#load "helper.fsx"
+
+let x = SomeNs.foo
+"""
+    let line = findOpenInsertionLine "test.fsx" source "SomeNs"
+    Assert.Equal(5, line)  // after #load on line 4
+
+[<Fact>]  // a non-reference directive between two references must not stop the scan
+let ``Open after last reference with interleaved I directive in fsx`` () =
+    let source = """#r "nuget: A"
+#I "/tmp"
+#load "b.fsx"
+
+let x = SomeNs.foo
+"""
+    let line = findOpenInsertionLine "test.fsx" source "SomeNs"
+    Assert.Equal(4, line)  // after #load on line 3, not after #I on line 2
+
+[<Fact>]  // CRLF line endings must not shift the computed line
+let ``Open placed after r directive with CRLF line endings`` () =
+    let source = "#r \"nuget: Newtonsoft.Json\"\r\n\r\nlet x : Newtonsoft.Json.JsonConvert = failwith \"\"\r\n"
+    let line = findOpenInsertionLine "test.fsx" source "Newtonsoft.Json"
+    Assert.Equal(2, line)
+
+[<Fact>]  // Nearest insertion inside a nested module keeps the open's indentation (not forced to col 0,
+          // which would break the module's offside) while still landing after the leading #r.
+let ``Open under Nearest keeps indentation inside nested module after r`` () =
+    let source = """#r "nuget: FSharp.Data"
+module Outer =
+    module Inner =
+        let _z = FSharp.Data.JsonValue.Parse "1"
+"""
+    let line, col = findOpenInsertion OpenStatementInsertionPoint.Nearest "test.fsx" source "FSharp.Data"
+    Assert.Equal((4, 8), (line, col))