diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index b89cf11dc7e..4cc6514c33f 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/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` / `true` 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)) diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index ed1a4da5bf5..8724712440c 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/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 diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj index 1cbea95b31e..d29c8693d18 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj @@ -74,6 +74,7 @@ + diff --git a/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs b/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs new file mode 100644 index 00000000000..ef1f09f466b --- /dev/null +++ b/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 + +[] +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) + +[] // 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 + +[] +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) + +[] +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) + +[] +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) + +[] +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) + +[] +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) + +[] +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) + +[] // 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) + +[] // 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) + +[] // #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) + +[] // #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) +[] // 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 + +[] // 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 + +[] // 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) + +[] // 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))