From 00c36e2421c18ce0fa68002a8071fbc61b180d58 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 9 Sep 2026 19:09:33 +0200 Subject: [PATCH 1/3] Place the added open from the syntax tree, not from a line of text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "open namespace" code fix and the completion that adds an open both decided where the declaration goes by recognizing a module header in the text of a line — `line.StartsWith "module" && not (line.EndsWith "=")` — followed by arithmetic that assumed a blank line under that header. Every shape the spelling missed put the open where it does not compile: `[] module Ns.Name` written on one line read as an implicit module and took the open above the module declaration, and a header with no blank line under it took the open below the first declaration, leaving the identifier as unresolved as before. InsertionContext.Pos now means one thing for every ScopeKind — the first line inside the scope — taken from the leading keyword the trivia records, the name it introduces and, for a nested module, the `=`. Attributes and doc comments sit above that line by construction, so no spelling has to be recognized. AdjustInsertionPoint keeps only the cosmetic step of stepping over the blank line under a header, and the editor's two insertion paths now share one implementation, which also follows the file's own line endings instead of the host's. Co-Authored-By: Claude Opus 5 (1M context) --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + docs/release-notes/.VisualStudio/18.vNext.md | 1 + src/Compiler/Service/ServiceParsedInputOps.fs | 87 ++++++------------ .../Service/ServiceParsedInputOps.fsi | 7 +- .../OpenDeclarationInsertionTests.fs | 28 +++++- .../CodeFixes/AddOpenCodeFixProvider.fs | 37 +------- .../src/FSharp.Editor/Common/RoslynHelpers.fs | 91 +++++++++---------- .../CodeFixes/AddOpenOnTopOffTests.fs | 25 +++++ .../CodeFixes/AddOpenOnTopOnTests.fs | 75 +++++++++++++++ 9 files changed, 211 insertions(+), 141 deletions(-) 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 27f2c0070d7..c3f1caca057 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,7 @@ ### Fixed * Fix internal error FS0192 "Iterate2D" when a `[]` parameter auto-quotes an argument that captures a not-yet-generalized use of an inferred generically-recursive function. The auto-quoted (`Expr.WithValue`) copy now keeps a fresh link to the recursive-value use so it receives the same inferred type arguments as the executable expression at the letrec point. ([Issue #20379](https://github.com/dotnet/fsharp/issues/20379)) +* `ParsedInput.FindNearestPointToInsertOpenDeclaration` now reports where an `open` belongs from the syntax tree instead of leaving consumers to recognize a declaration header by its text. `InsertionContext.Pos` is the first line inside the scope — below the module or namespace header, or below the opens and directives above it — for every `ScopeKind`, so an `open` is no longer placed above a header the text sniffing failed to recognize (`[] module Ns.Name` on one line) or below the first declaration when no blank line separates it from the header. `ParsedInput.AdjustInsertionPoint` keeps only the cosmetic step of skipping the blank line under a header. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500)) * Fix `NativePtr.stackalloc` nested in a larger expression (e.g. a call argument or the right of an assignment) producing an assembly that throws `InvalidProgramException` at load. ([Issue #8083](https://github.com/dotnet/fsharp/issues/8083), [PR #20302](https://github.com/dotnet/fsharp/pull/20302)) * Fix internal error "Unexpected generalized type variables when compiling an active pattern" when an active pattern is used in a `let` binding whose right-hand side is a generic value, e.g. `let (T) = id`. Such a binding is now checked like the equivalent `match` and is not generalized. ([Issue #16856](https://github.com/dotnet/fsharp/issues/16856), [PR #20383](https://github.com/dotnet/fsharp/pull/20383)) * Fix Release-only (`--optimize+`) `System.InvalidProgramException` from `Seq.collect` / `yield!` over a value-type (struct) collection implementing `seq<'T>` (e.g. `ImmutableArray<_>`) when materialised with `List.ofSeq` / `Seq.toList` / `Seq.toArray` or a list/array comprehension. The collector lowering now boxes a struct sub-collection to `seq<'T>` before calling `AddMany`/`AddManyAndClose` (matching the coercion the type checker already inserts for `yield!`), and uses `unit` as the try/finally result type instead of the body type (removing a spurious `ldnull` store). ([Issue #20203](https://github.com/dotnet/fsharp/issues/20203)) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index ba03f663967..adf65dab12b 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,6 +5,7 @@ ### Fixed +* The "open namespace" code fix and the completion that adds an `open` now place it from the syntax tree rather than by recognizing a `module` line by its text, so it no longer lands above the file's own module declaration (`[] module Ns.Name` written on one line) or below the first declaration when no blank line follows the header. Both paths now share one implementation and follow the file's existing line endings. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * 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)) diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index 7ee76f4652e..aeea6420af6 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/src/Compiler/Service/ServiceParsedInputOps.fs @@ -2476,6 +2476,18 @@ module ParsedInput = | _ -> None |> Option.map (fun r -> r.StartColumn) + // The line a declaration's header ends on: its leading keyword, the name it introduces and, + // for a nested module, the `=`. Attributes and doc comments sit above it, the body below. + let headerEndLine (keyword: range) (ident: LongIdent) (equals: range option) = + let nameEnd = + match List.tryLast ident with + | Some lastIdent -> max keyword.EndLine lastIdent.idRange.EndLine + | None -> keyword.EndLine + + match equals with + | Some equalsRange -> max nameEnd equalsRange.EndLine + | None -> nameEnd + let rec walkImplFileInput (file: ParsedImplFileInput) = List.iter (walkSynModuleOrNamespace []) file.Contents @@ -2496,14 +2508,12 @@ module ParsedInput = let fullIdent = parent @ ident - // Use trivia to get the actual module/namespace keyword line, which excludes attributes - let startLine = + let headerLine = 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 + // An implicit module has no header, so its first declaration opens the scope. + | SynModuleOrNamespaceLeadingKeyword.None -> range.StartLine - 1 let scopeKind = match isModule, parent with @@ -2511,7 +2521,7 @@ module ParsedInput = | true, _ -> NestedModule | _ -> Namespace - doRange scopeKind fullIdent startLine range.StartColumn + doRange scopeKind fullIdent headerLine range.StartColumn addModule (fullIdent, range) List.iter (walkSynModuleDecl fullIdent) decls @@ -2524,16 +2534,15 @@ module ParsedInput = 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 moduleBodyIndentation = getMinColumn decls |> Option.defaultValue (range.StartColumn + 4) - doRange NestedModule fullIdent moduleKeywordLine moduleBodyIndentation + doRange NestedModule fullIdent headerLine 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 +2618,14 @@ 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. + /// Nudges the insertion point past the blank line that conventionally follows a declaration + /// header, so that the `open` joins the code below it instead of the gap above it. 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 +2668,8 @@ 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. + // The open belongs directly under the directives, so report the scope that keeps it + // there rather than the module scope, which would push it past the blank line below. let scopeKind = if ctx.ScopeKind = ScopeKind.TopModule then ScopeKind.HashDirective diff --git a/src/Compiler/Service/ServiceParsedInputOps.fsi b/src/Compiler/Service/ServiceParsedInputOps.fsi index 1b28bfb18d3..71aecf25a28 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fsi +++ b/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, that + /// is, below the declaration header or below the open declarations and directives above it. Pos: pos } @@ -207,7 +208,9 @@ 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. + /// Nudges the insertion point past the blank line that conventionally follows a declaration header, + /// so that the `open` joins the code below it instead of the gap above it. `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 diff --git a/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs b/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs index ef1f09f466b..21c4a98cec0 100644 --- a/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs @@ -116,7 +116,33 @@ let ``Open not forced above named module in fsx`` () = 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. + // Inside the module, next to the code rather than in the gap under the header. + Assert.Equal(3, line) + +[] // Regression: an attribute sharing the header's line must not read as an implicit module +let ``Open not forced above module with attribute on same line`` () = + let source = """[] module Foo + +let x = System.IO.File.ReadAllText "a" +""" + let line = findOpenInsertionLine "test.fs" source "System.IO" + Assert.Equal(3, line) + +[] // Regression: the header line is known from the tree, not from a blank line below it +let ``Open placed inside module without a blank line under the header`` () = + let source = """module Foo +let x = System.IO.File.ReadAllText "a" +""" + let line = findOpenInsertionLine "test.fs" source "System.IO" + Assert.Equal(2, line) + +[] +let ``Open placed under namespace without a blank line under the header`` () = + let source = """namespace Ns +type T() = + member _.M() = System.IO.File.ReadAllText "a" +""" + let line = findOpenInsertionLine "test.fs" source "System.IO" Assert.Equal(2, line) [] // Only #r/#load drive placement; other directives (#time/#help/#I/...) must not diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index 57bca908da6..c7299ae13b4 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -2,7 +2,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor -open System open System.Composition open System.Collections.Immutable @@ -19,8 +18,6 @@ open CancellableTasks type internal AddOpenCodeFixProvider [] (assemblyContentProvider: AssemblyContentProvider) = inherit CodeFixProvider() - static let br = Environment.NewLine - let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__") let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) = @@ -30,39 +27,11 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr Changes = [ TextChange(context.Span, qualifier) ] } - // 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 - let margin = String(' ', ctx.Pos.Column) - - let startLineNumber, openDeclaration = - match ctx.ScopeKind with - | ScopeKind.TopModule -> - match sourceText.Lines[insertionLineNumber].ToString().Trim() with - - // explicit top level module - | line when line.StartsWith "module" && not (line.EndsWith "=") -> insertionLineNumber + 2, $"{margin}open {ns}{br}{br}" - - // nested module, shouldn't be here - | line when line.StartsWith "module" -> insertionLineNumber, $"{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}" - | ScopeKind.HashDirective -> insertionLineNumber + 1, $"open {ns}{br}{br}" - - let start = sourceText.Lines[startLineNumber].Start - TextChange(TextSpan(start, 0), openDeclaration) - let openNamespaceFix ctx name ns multipleNames sourceText = let displayText = $"open {ns}" + (if multipleNames then " (" + name + ")" else "") - let change = getOpenDeclaration sourceText ctx ns + + let change = + OpenDeclarationHelper.getOpenDeclarationChange sourceText ctx ns { Name = CodeFix.AddOpen diff --git a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs index 2679740fbb3..d157875ab10 100644 --- a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs @@ -230,63 +230,60 @@ module internal RoslynHelpers = module internal OpenDeclarationHelper = /// - /// Inserts open declaration into `SourceText`. + /// The change that adds an open declaration at the point the insertion context names. /// /// SourceText. /// Insertion context. Typically returned from tryGetInsertionContext /// Namespace to open. - let insertOpenDeclaration (sourceText: SourceText) (ctx: InsertionContext) (ns: string) : SourceText * int = - let mutable minPos = None - - let insert line lineStr (sourceText: SourceText) : SourceText = - let ln = sourceText.Lines.[line] - let pos = ln.Start - - minPos <- - match minPos with - | None -> Some pos - | Some oldPos -> Some(min oldPos pos) - - // find the line break characters on the previous line to use, Environment.NewLine should not be used - // as it makes assumptions on the line endings in the source. - let lineBreak = - ln.Text.ToString(TextSpan(ln.End, ln.EndIncludingLineBreak - ln.End)) - - sourceText.WithChanges(TextChange(TextSpan(pos, 0), lineStr + lineBreak)) - + let getOpenDeclarationChange (sourceText: SourceText) (ctx: InsertionContext) (ns: string) : TextChange = let getLineStr line = - sourceText.Lines.[line].ToString().Trim() + if line >= 0 && line < sourceText.Lines.Count then + sourceText.Lines[line].ToString().Trim() + else + "" let pos = ParsedInput.AdjustInsertionPoint getLineStr ctx - let docLine = Line.toZ pos.Line - let lineStr = (String.replicate pos.Column " ") + "open " + ns - - // If we're at the top of a file (e.g., F# script) then add a newline before adding the open declaration - let sourceText = - if docLine = 0 then - sourceText |> insert docLine Environment.NewLine |> insert docLine lineStr + let line = sourceText.Lines[min (Line.toZ pos.Line) (sourceText.Lines.Count - 1)] + + // Follow the line endings the file itself uses rather than assuming the host's. + let lineBreak = + match line.Text.ToString(TextSpan(line.End, line.EndIncludingLineBreak - line.End)) with + | "" -> Environment.NewLine + | breakChars -> breakChars + + let isHeaderScope = + match ctx.ScopeKind with + | ScopeKind.TopModule + | ScopeKind.Namespace + | ScopeKind.NestedModule -> true + | _ -> false + + // A declaration header sitting directly above becomes its own paragraph, so the open block + // below it reads as a block rather than as part of the header. + let separatorAbove = + if isHeaderScope && getLineStr (line.LineNumber - 1) <> "" then + lineBreak else - sourceText |> insert docLine lineStr + "" - // if there's no a blank line between open declaration block and the rest of the code, we add one - let sourceText = - if sourceText.Lines.[docLine + 1].ToString().Trim() <> "" then - sourceText |> insert (docLine + 1) "" - else - sourceText - - let sourceText = - // for top level module we add a blank line between the module declaration and first open statement - if - (pos.Column = 0 || ctx.ScopeKind = ScopeKind.Namespace) - && docLine > 0 - && not (sourceText.Lines.[docLine - 1].ToString().Trim().StartsWith "open") - then - sourceText |> insert docLine "" - else - sourceText + let separatorBelow = + match ctx.ScopeKind with + // The open joins the block of opens right above it. + | ScopeKind.OpenDeclaration -> lineBreak + | _ when getLineStr line.LineNumber = "" -> lineBreak + | _ -> lineBreak + lineBreak + + TextChange(TextSpan(line.Start, 0), separatorAbove + String(' ', pos.Column) + "open " + ns + separatorBelow) - sourceText, minPos |> Option.defaultValue 0 + /// + /// Inserts open declaration into `SourceText`. + /// + /// SourceText. + /// Insertion context. Typically returned from tryGetInsertionContext + /// Namespace to open. + let insertOpenDeclaration (sourceText: SourceText) (ctx: InsertionContext) (ns: string) : SourceText * int = + let change = getOpenDeclarationChange sourceText ctx ns + sourceText.WithChanges change, change.Span.Start // http://www.fssnip.net/7S3/title/Intersperse-a-list module List = diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs index 765b1aa6210..4518c5c4dd1 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -125,6 +125,31 @@ Console.WriteLine 42 open System +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has an attribute on the same line`` () = + let code = + """[] module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """[] module Module1 + +open System + Console.WriteLine 42 """ } diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index 2614a54a493..2b10e82113d 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -127,6 +127,81 @@ Console.WriteLine 42 Assert.Equal(expected, actual) +[] +let ``Fixes FS0039 for missing opens - module has an attribute on the same line`` () = + let code = + """[] module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """[] module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - explicit top level module without a blank line`` () = + let code = + """module Module1 +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - namespace without a blank line`` () = + let code = + """namespace N1 +module M1 = + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """namespace N1 + +open System + +module M1 = + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + [] let ``Fixes FS0039 for missing opens - nested module`` () = let code = From f8bd9728b4b994011a1ceecc8730492c92f2feec Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 01:33:30 +0200 Subject: [PATCH 2/3] Run dotnet fantomas The formatting-check CI job flagged AddOpenCodeFixProvider.fs. Co-Authored-By: Claude Sonnet 5 --- .../src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index c7299ae13b4..9de03c976f1 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -30,8 +30,7 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr let openNamespaceFix ctx name ns multipleNames sourceText = let displayText = $"open {ns}" + (if multipleNames then " (" + name + ")" else "") - let change = - OpenDeclarationHelper.getOpenDeclarationChange sourceText ctx ns + let change = OpenDeclarationHelper.getOpenDeclarationChange sourceText ctx ns { Name = CodeFix.AddOpen From f5dd4ff5ad263d69bccbb1e807a0c81a4c6fcec8 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 15:02:44 +0200 Subject: [PATCH 3/3] Break the line when the declaration shares it with what precedes it The insertion point is a position, and the change was applied at the start of its line. That is the same thing whenever what precedes the declaration on that line is whitespace, and something else entirely when it is not: (* header *) let x = StringBuilder() Here the scope's first declaration is on the line that closes the comment, so the open went between `(* header` and `*)`, inside the comment, leaving the name as unresolved as it was. Whether the old code got this right was luck of the same kind: it snapped an implicit module to line 1 and so wrote above the comment here, but wrote inside it when the comment closed on a line of its own. What precedes the declaration is trivia that has to stay where it is, so the line is broken at the declaration instead, and the declaration keeps its column - moving it to the start of the line would put the open the implicit module now begins with offside of it. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/FSharp.Editor/Common/RoslynHelpers.fs | 15 +++++++++++- .../CodeFixes/AddOpenOnTopOnTests.fs | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs index d157875ab10..8f47a6545f8 100644 --- a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs @@ -273,7 +273,20 @@ module internal OpenDeclarationHelper = | _ when getLineStr line.LineNumber = "" -> lineBreak | _ -> lineBreak + lineBreak - TextChange(TextSpan(line.Start, 0), separatorAbove + String(' ', pos.Column) + "open " + ns + separatorBelow) + let margin = String(' ', pos.Column) + let column = min pos.Column (line.End - line.Start) + let trivia = sourceText.ToString(TextSpan(line.Start, column)).TrimEnd() + + // Anything but whitespace before the insertion point is trivia the scope's first declaration + // follows on its line - a block comment closing there, say. Break the line at the declaration + // rather than write the open into the middle of what precedes it. + if trivia.Length > 0 then + TextChange( + TextSpan(line.Start + trivia.Length, column - trivia.Length), + lineBreak + margin + "open " + ns + lineBreak + lineBreak + margin + ) + else + TextChange(TextSpan(line.Start, 0), separatorAbove + margin + "open " + ns + separatorBelow) /// /// Inserts open declaration into `SourceText`. diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index 2b10e82113d..23e1ae1de06 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -78,6 +78,30 @@ Console.WriteLine 42 Assert.Equal(expected, actual) +[] // The first declaration follows a block comment closing on its line +let ``Fixes FS0039 for missing opens - declaration shares its line with the end of a comment`` () = + let code = + """(* header +*) Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """(* header +*) + open System + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + [] let ``Fixes FS0039 for missing opens - there is already an open directive`` () = let code =