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..cfed5a8a782 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,8 @@ ### 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)) +* `AssemblySymbol` and `InsertionContextEntity` now carry how far a plain `open` reaches (`OpenableIdentCount`) and how much of the entity's name the suggested one covers (`NamespaceIdentCount`), so a consumer can tell the namespaces and F# modules a plain `open` takes from the types only `open type` opens. Until now nothing distinguished them, and tooling suggested opening a type as if it were a namespace. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501)) * 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..84eaeec0e1a 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,6 +5,9 @@ ### 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)) +* The "open namespace" code fix offers `open type System.Console` where it used to offer `open System.Console`. A plain `open` only reaches namespaces and F# modules, so for a type nested in a type (`System.Net.WebRequestMethods.File`) or a static member of one (`System.Console.WriteLine`) every suggestion it made was code that does not compile — `FS0039: The namespace 'Console' is not defined`. ([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. It computed them all along and then kept whichever one the assembly crawler happened to reach first, so an unresolved `File` was answered with `System.Net.WebRequestMethods` and `System.IO` was never offered at all. As in Roslyn's add-import fix, the list is capped — five opens, three qualifications — and ordered with `System` first. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501)) * 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/ServiceAssemblyContent.fs b/src/Compiler/Service/ServiceAssemblyContent.fs index f08faa996a5..9fa739b2b5a 100644 --- a/src/Compiler/Service/ServiceAssemblyContent.fs +++ b/src/Compiler/Service/ServiceAssemblyContent.fs @@ -39,6 +39,7 @@ type AssemblySymbol = { FullName: string CleanedIdents: ShortIdents Namespace: ShortIdents option + OpenableIdentCount: int NearestRequireQualifiedAccessParent: ShortIdents option TopRequireQualifiedAccessParent: ShortIdents option AutoOpenParent: ShortIdents option @@ -57,7 +58,10 @@ type Parent = TopRequiresQualifiedAccess: (* isForMemberOrValue *) bool -> ShortIdents option AutoOpen: ShortIdents option WithModuleSuffix: ShortIdents option - IsModule: bool } + IsModule: bool + /// How many leading idents a plain `open` reaches. A type never extends it: everything it + /// contains is reached by naming the type, or by opening the type itself. + OpenableIdentCount: int } static member Empty = { Namespace = None @@ -65,7 +69,13 @@ type Parent = TopRequiresQualifiedAccess = fun _ -> None AutoOpen = None WithModuleSuffix = None - IsModule = true } + IsModule = true + OpenableIdentCount = 0 } + + /// The reach of a plain `open` over an entity declared in this parent: the enclosing namespace + /// at the very least, and whatever modules the parent chain has added to it. + member x.OpenableIdentCountFor (ns: ShortIdents option) = + max x.OpenableIdentCount (ns |> Option.map Array.length |> Option.defaultValue 0) static member RewriteParentIdents (parentIdents: ShortIdents option) (idents: ShortIdents) = match parentIdents with @@ -149,6 +159,7 @@ module AssemblyContent = { FullName = fullName CleanedIdents = cleanIdents Namespace = ns + OpenableIdentCount = parent.OpenableIdentCountFor ns NearestRequireQualifiedAccessParent = parent.ThisRequiresQualifiedAccess false |> Option.map parent.FixParentModuleSuffix TopRequireQualifiedAccessParent = topRequireQualifiedAccessParent AutoOpenParent = parent.AutoOpen |> Option.map parent.FixParentModuleSuffix @@ -179,6 +190,7 @@ module AssemblyContent = { FullName = fullName CleanedIdents = cleanedIdents Namespace = ns + OpenableIdentCount = parent.OpenableIdentCountFor ns NearestRequireQualifiedAccessParent = parent.ThisRequiresQualifiedAccess true |> Option.map parent.FixParentModuleSuffix TopRequireQualifiedAccessParent = topRequireQualifiedAccessParent AutoOpenParent = autoOpenParent @@ -250,7 +262,12 @@ module AssemblyContent = else parent.WithModuleSuffix Namespace = ns - IsModule = entity.IsFSharpModule } + IsModule = entity.IsFSharpModule + + OpenableIdentCount = + match entity.IsNamespace || entity.IsFSharpModule, currentEntity with + | true, Some e -> e.CleanedIdents.Length + | _ -> parent.OpenableIdentCountFor ns } match entity.TryGetMembersFunctionsAndValues() with | xs when xs.Count > 0 -> diff --git a/src/Compiler/Service/ServiceAssemblyContent.fsi b/src/Compiler/Service/ServiceAssemblyContent.fsi index 09756eee2e5..7fc789aa85e 100644 --- a/src/Compiler/Service/ServiceAssemblyContent.fsi +++ b/src/Compiler/Service/ServiceAssemblyContent.fsi @@ -37,6 +37,10 @@ type public AssemblySymbol = /// `FSharpEntity.Namespace`. Namespace: ShortIdents option + /// How many leading `CleanedIdents` a plain `open` reaches: the namespace plus the enclosing + /// F# modules. Idents past it name types, whose contents `open type` brings into scope instead. + OpenableIdentCount: int + /// The most narrative parent module that has `RequireQualifiedAccess` attribute. NearestRequireQualifiedAccessParent: ShortIdents option diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index 7ee76f4652e..096c698b730 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/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,18 @@ module Entity = | _ -> let fullRelativeName = Array.append (getRelativeNs fullOpenableNs) restIdents + // What the suggested `open` covers, named relatively to the current scope and, + // for the count, absolutely: the two differ by the prefix already in scope. + let shortenByQualifiedIdents (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(shortenByQualifiedIdents relativeNs |> String.concat ".") let qualifier = if fullRelativeName.Length > 1 && fullRelativeName.Length >= identCount then @@ -272,6 +279,7 @@ module Entity = FullRelativeName = String.concat "." fullRelativeName //.[0..fullRelativeName.Length - identCount - 1] Qualifier = String.concat "." qualifier Namespace = ns + NamespaceIdentCount = (shortenByQualifiedIdents openableNs).Length FullDisplayName = match restIdents with | [| _ |] -> "" @@ -2476,6 +2484,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 +2516,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 +2529,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 +2542,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 +2626,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 +2676,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..87fc37d575d 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 } @@ -162,6 +163,11 @@ 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 that namespace covers. Compare it with + /// `AssemblySymbol.OpenableIdentCount` to tell a namespace a plain `open` reaches from a type, + /// which only `open type` opens. + NamespaceIdentCount: int + /// Full display name (i.e. last ident plus modules with `RequireQualifiedAccess` attribute prefixed). FullDisplayName: string @@ -207,7 +213,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/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl index 5c9c346b613..73749e8ce39 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl @@ -3108,6 +3108,8 @@ FSharp.Compiler.EditorServices.AssemblySymbol: FSharp.Compiler.EditorServices.Un FSharp.Compiler.EditorServices.AssemblySymbol: FSharp.Compiler.EditorServices.UnresolvedSymbol get_UnresolvedSymbol() FSharp.Compiler.EditorServices.AssemblySymbol: FSharp.Compiler.Symbols.FSharpSymbol Symbol FSharp.Compiler.EditorServices.AssemblySymbol: FSharp.Compiler.Symbols.FSharpSymbol get_Symbol() +FSharp.Compiler.EditorServices.AssemblySymbol: Int32 OpenableIdentCount +FSharp.Compiler.EditorServices.AssemblySymbol: Int32 get_OpenableIdentCount() FSharp.Compiler.EditorServices.AssemblySymbol: Microsoft.FSharp.Core.FSharpFunc`2[FSharp.Compiler.EditorServices.LookupType,FSharp.Compiler.EditorServices.EntityKind] Kind FSharp.Compiler.EditorServices.AssemblySymbol: Microsoft.FSharp.Core.FSharpFunc`2[FSharp.Compiler.EditorServices.LookupType,FSharp.Compiler.EditorServices.EntityKind] get_Kind() FSharp.Compiler.EditorServices.AssemblySymbol: Microsoft.FSharp.Core.FSharpOption`1[System.String[]] AutoOpenParent @@ -3123,7 +3125,7 @@ FSharp.Compiler.EditorServices.AssemblySymbol: System.String ToString() FSharp.Compiler.EditorServices.AssemblySymbol: System.String get_FullName() FSharp.Compiler.EditorServices.AssemblySymbol: System.String[] CleanedIdents FSharp.Compiler.EditorServices.AssemblySymbol: System.String[] get_CleanedIdents() -FSharp.Compiler.EditorServices.AssemblySymbol: Void .ctor(System.String, System.String[], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], FSharp.Compiler.Symbols.FSharpSymbol, Microsoft.FSharp.Core.FSharpFunc`2[FSharp.Compiler.EditorServices.LookupType,FSharp.Compiler.EditorServices.EntityKind], FSharp.Compiler.EditorServices.UnresolvedSymbol) +FSharp.Compiler.EditorServices.AssemblySymbol: Void .ctor(System.String, System.String[], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Int32, Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], Microsoft.FSharp.Core.FSharpOption`1[System.String[]], FSharp.Compiler.Symbols.FSharpSymbol, Microsoft.FSharp.Core.FSharpFunc`2[FSharp.Compiler.EditorServices.LookupType,FSharp.Compiler.EditorServices.EntityKind], FSharp.Compiler.EditorServices.UnresolvedSymbol) FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.EditorServices.InheritanceContext context FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.EditorServices.InheritanceContext get_context() FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] get_path() @@ -3726,6 +3728,8 @@ FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 CompareTo(System.Ob FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 CompareTo(System.Object, System.Collections.IComparer) FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 GetHashCode() FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 GetHashCode(System.Collections.IEqualityComparer) +FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 NamespaceIdentCount +FSharp.Compiler.EditorServices.InsertionContextEntity: Int32 get_NamespaceIdentCount() FSharp.Compiler.EditorServices.InsertionContextEntity: Microsoft.FSharp.Core.FSharpOption`1[System.String] Namespace FSharp.Compiler.EditorServices.InsertionContextEntity: Microsoft.FSharp.Core.FSharpOption`1[System.String] get_Namespace() FSharp.Compiler.EditorServices.InsertionContextEntity: System.String FullDisplayName @@ -3737,7 +3741,7 @@ FSharp.Compiler.EditorServices.InsertionContextEntity: System.String get_FullDis FSharp.Compiler.EditorServices.InsertionContextEntity: System.String get_FullRelativeName() FSharp.Compiler.EditorServices.InsertionContextEntity: System.String get_LastIdent() FSharp.Compiler.EditorServices.InsertionContextEntity: System.String get_Qualifier() -FSharp.Compiler.EditorServices.InsertionContextEntity: Void .ctor(System.String, System.String, Microsoft.FSharp.Core.FSharpOption`1[System.String], System.String, System.String) +FSharp.Compiler.EditorServices.InsertionContextEntity: Void .ctor(System.String, System.String, Microsoft.FSharp.Core.FSharpOption`1[System.String], Int32, System.String, System.String) FSharp.Compiler.EditorServices.InterfaceData+Interface: FSharp.Compiler.Syntax.SynType get_interfaceType() FSharp.Compiler.EditorServices.InterfaceData+Interface: FSharp.Compiler.Syntax.SynType interfaceType FSharp.Compiler.EditorServices.InterfaceData+Interface: Microsoft.FSharp.Core.FSharpOption`1[Microsoft.FSharp.Collections.FSharpList`1[FSharp.Compiler.Syntax.SynMemberDefn]] get_memberDefns() 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..5bbda140350 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -19,7 +19,21 @@ open CancellableTasks type internal AddOpenCodeFixProvider [] (assemblyContentProvider: AssemblyContentProvider) = inherit CodeFixProvider() - static let br = Environment.NewLine + // A name can be reachable from a great many places, and the lightbulb is a menu a person reads. + // The same reason Roslyn's add-import fix stops at five suggestions and its fully-qualify at three. + let maxOpenSuggestions = 5 + let maxQualifySuggestions = 3 + + // Which assembly the entity crawler reached first is no order to offer suggestions in. Sort them + // the way Roslyn's add-import fix does: what `System` holds first, the rest alphabetically after. + let suggestionOrder (declaration: string) = + let opened = declaration.Substring(declaration.LastIndexOf ' ' + 1) + + let isSystem = + opened.Equals("System", StringComparison.Ordinal) + || opened.StartsWith("System.", StringComparison.Ordinal) + + (if isSystem then 0 else 1), declaration let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__") @@ -30,39 +44,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}" + let openNamespaceFix ctx name declaration multipleNames sourceText = + let displayText = declaration + (if multipleNames then " (" + name + ")" else "") - // 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 declaration { Name = CodeFix.AddOpen @@ -70,33 +56,46 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr Changes = [ change ] } + // A plain `open` reaches namespaces and F# modules. When the entity sits deeper than that - a type + // nested in a type, or a static member of one - the type itself has to be opened. + let openDeclaration (entity: InsertionContextEntity) openableIdentCount ns = + if entity.NamespaceIdentCount > openableIdentCount then + $"open type {ns}" + else + $"open {ns}" + let getSuggestionsAsCodeFixes (context: CodeFixContext) (sourceText: SourceText) - (candidates: (InsertionContextEntity * InsertionContext) list) + (candidates: (InsertionContextEntity * InsertionContext * int) list) = seq { candidates - |> Seq.choose (fun (entity, ctx) -> entity.Namespace |> Option.map (fun ns -> ns, entity.FullDisplayName, ctx)) - |> Seq.groupBy (fun (ns, _, _) -> ns) - |> Seq.map (fun (ns, xs) -> - ns, + |> Seq.choose (fun (entity, ctx, openableIdentCount) -> + entity.Namespace + |> Option.map (fun ns -> openDeclaration entity openableIdentCount ns, entity.FullDisplayName, ctx)) + |> Seq.groupBy (fun (declaration, _, _) -> declaration) + |> Seq.map (fun (declaration, xs) -> + declaration, xs |> Seq.map (fun (_, name, ctx) -> name, ctx) |> Seq.distinctBy (fun (name, _) -> name) |> Seq.sortBy fst |> Seq.toArray) - |> Seq.map (fun (ns, names) -> + |> Seq.sortBy (fst >> suggestionOrder) + |> Seq.map (fun (declaration, names) -> let multipleNames = names |> Array.length > 1 - names |> Seq.map (fun (name, ctx) -> ns, name, ctx, multipleNames)) + names |> Seq.map (fun (name, ctx) -> declaration, name, ctx, multipleNames)) |> Seq.concat - |> Seq.map (fun (ns, name, ctx, multipleNames) -> openNamespaceFix ctx name ns multipleNames sourceText) + |> Seq.truncate maxOpenSuggestions + |> Seq.map (fun (declaration, name, ctx, multipleNames) -> openNamespaceFix ctx name declaration multipleNames sourceText) candidates - |> Seq.filter (fun (entity, _) -> not (entity.LastIdent.StartsWith "op_")) // Don't include qualified operator names. The resultant codefix won't compile because it won't be an infix operator anymore. - |> Seq.map (fun (entity, _) -> entity.FullRelativeName, entity.Qualifier) + |> Seq.filter (fun (entity, _, _) -> not (entity.LastIdent.StartsWith "op_")) // Don't include qualified operator names. The resultant codefix won't compile because it won't be an infix operator anymore. + |> Seq.map (fun (entity, _, _) -> entity.FullRelativeName, entity.Qualifier) |> Seq.distinct |> Seq.sort + |> Seq.truncate maxQualifySuggestions |> Seq.map (qualifySymbolFix context) } @@ -104,10 +103,10 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039", "FS0043") - override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFixes this - interface IFSharpCodeFixProvider with - member _.GetCodeFixIfAppliesAsync context = + interface IFSharpMultiCodeFixProvider with + member _.GetCodeFixesAsync context = cancellableTask { let document = context.Document @@ -162,7 +161,9 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies checkResults |> Array.collect (fun s -> [| - yield s.TopRequireQualifiedAccessParent, s.AutoOpenParent, s.Namespace, s.CleanedIdents + yield + s.OpenableIdentCount, + (s.TopRequireQualifiedAccessParent, s.AutoOpenParent, s.Namespace, s.CleanedIdents) if isAttribute then let lastIdent = s.CleanedIdents.[s.CleanedIdents.Length - 1] @@ -171,17 +172,18 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr && s.Kind LookupType.Precise = EntityKind.Attribute then yield - s.TopRequireQualifiedAccessParent, - s.AutoOpenParent, - s.Namespace, - s.CleanedIdents - |> Array.replace - (s.CleanedIdents.Length - 1) - (lastIdent.Substring(0, lastIdent.Length - 9)) + s.OpenableIdentCount, + (s.TopRequireQualifiedAccessParent, + s.AutoOpenParent, + s.Namespace, + s.CleanedIdents + |> Array.replace + (s.CleanedIdents.Length - 1) + (lastIdent.Substring(0, lastIdent.Length - 9))) |]) ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End - |> Option.bind (fun longIdent -> + |> Option.map (fun longIdent -> let maybeUnresolvedIdents = longIdent |> List.map (fun ident -> @@ -205,11 +207,11 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr insertionPoint entities - |> Seq.map createEntity - |> Seq.concat + |> Seq.collect (fun (openableIdentCount, symbol) -> + createEntity symbol + |> Seq.map (fun (entity, ctx) -> entity, ctx, openableIdentCount)) |> Seq.toList - |> getSuggestionsAsCodeFixes context sourceText - |> Seq.tryHead)) + |> getSuggestionsAsCodeFixes context sourceText)) - |> ValueOption.ofOption + |> Option.defaultValue Seq.empty } diff --git a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs index 2679740fbb3..26206509bd4 100644 --- a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs @@ -230,63 +230,73 @@ 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)) - + /// The declaration to add, `open Foo` or `open type Foo`. + let getOpenDeclarationChange (sourceText: SourceText) (ctx: InsertionContext) (declaration: string) : TextChange = let getLineStr line = - sourceText.Lines.[line].ToString().Trim() - - 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 + if line >= 0 && line < sourceText.Lines.Count then + sourceText.Lines[line].ToString().Trim() 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 "" + let pos = ParsedInput.AdjustInsertionPoint getLineStr ctx + 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 + "" + + 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 + + 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 + declaration + lineBreak + lineBreak + margin + ) + else + TextChange(TextSpan(line.Start, 0), separatorAbove + margin + declaration + 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 ("open " + 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..c6c116ea4e0 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -2,6 +2,8 @@ module FSharp.Editor.Tests.CodeFixes.AddOpenOnTopOffTests +open System + open Microsoft.VisualStudio.FSharp.Editor open Xunit @@ -9,6 +11,13 @@ open CodeFixTestFramework let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) +/// Every `open` suggestion the fix offers, in the order the lightbulb lists them. +let private openFixes code mode = + codeFix + |> multiFix code mode + |> Seq.filter (fun fix -> fix.Message.StartsWith("open ", StringComparison.Ordinal)) + |> Seq.toList + let mode = WithSettings { CodeFixesOptions.Default with @@ -22,7 +31,7 @@ let ``Fixes FS0039 for missing opens - basic`` () = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -31,10 +40,11 @@ let ``Fixes FS0039 for missing opens - basic`` () = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - first line is empty`` () = @@ -44,7 +54,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -54,10 +64,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - multiple first lines are empty`` () = @@ -68,7 +79,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -79,10 +90,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - there is already an open directive`` () = @@ -93,7 +105,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -103,10 +115,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - top level module is explicit`` () = @@ -117,7 +130,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -128,10 +141,37 @@ open System Console.WriteLine 42 """ } + ] + + let actual = openFixes 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 = + [ + { + Message = "open System" + FixedCode = + """[] module Module1 + +open System + +Console.WriteLine 42 +""" + } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - nested module`` () = @@ -142,7 +182,7 @@ let ``Fixes FS0039 for missing opens - nested module`` () = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -153,10 +193,11 @@ let ``Fixes FS0039 for missing opens - nested module`` () = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - explicit module has attributes`` () = @@ -169,7 +210,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -182,10 +223,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - implicit module has attributes`` () = @@ -197,7 +239,7 @@ type MyType() = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -209,10 +251,11 @@ type MyType() = let now = DateTime.Now """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) // TODO: the open statement should actually be within the module [] @@ -226,7 +269,7 @@ module Module1 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -239,10 +282,11 @@ module Module1 = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - module has multiple attributes`` () = @@ -256,7 +300,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -270,10 +314,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - attributes are mixed with empty lines`` () = @@ -288,7 +333,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -303,10 +348,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - multiple modules in one file`` () = @@ -322,7 +368,7 @@ module Module2 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -338,10 +384,11 @@ module Module2 = Console.WriteLine(42) """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - explicit namespace`` () = @@ -355,7 +402,7 @@ module M1 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -369,10 +416,11 @@ module M1 = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Doesn't fix FS0039 for random undefined symbols`` () = @@ -381,11 +429,11 @@ let ``Doesn't fix FS0039 for random undefined symbols`` () = let f = g """ - let expected = None + let expected = [] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0043 for missing opens`` () = @@ -399,7 +447,7 @@ module N = """ let expected = - Some + [ { Message = "open M" FixedCode = @@ -413,10 +461,11 @@ module N = let theAnswer = 4 ++ 2 """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Doesn't fix FS0043 for random unsupported values`` () = @@ -427,11 +476,11 @@ type RecordType = { X : int } let x : RecordType = null """ - let expected = None + let expected = [] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - module has multiline attributes`` () = @@ -450,7 +499,7 @@ module FlatList = """ let expected = - Some + [ { Message = "open System.Collections.Generic" FixedCode = @@ -469,7 +518,8 @@ module FlatList = let a : KeyValuePair = KeyValuePair("key", 1) """ } + ] - let actual = codeFix |> tryFix code mode + let actual = openFixes code mode - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index 2614a54a493..34ff3748f99 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -2,6 +2,8 @@ module FSharp.Editor.Tests.CodeFixes.AddOpenOnTopOnTests +open System + open Microsoft.VisualStudio.FSharp.Editor open Xunit @@ -9,6 +11,15 @@ open CodeFixTestFramework let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) +/// Everything the fix offers, in the order the lightbulb lists it: the opens, then the qualifications. +let private allFixes code mode = + codeFix |> multiFix code mode |> Seq.toList + +/// Just the `open` suggestions, for the tests that are about where the declaration lands. +let private openFixes code mode = + allFixes code mode + |> List.filter (fun fix -> fix.Message.StartsWith("open ", StringComparison.Ordinal)) + [] let ``Fixes FS0039 for missing opens - basic`` () = let code = @@ -16,7 +27,7 @@ let ``Fixes FS0039 for missing opens - basic`` () = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -25,10 +36,11 @@ let ``Fixes FS0039 for missing opens - basic`` () = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - first line is empty`` () = @@ -38,7 +50,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -48,10 +60,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - multiple first lines are empty`` () = @@ -62,7 +75,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -73,10 +86,36 @@ open System Console.WriteLine 42 """ } + ] + + let actual = openFixes code Auto + + 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 = + [ + { + Message = "open System" + FixedCode = + """(* header +*) + open System + + Console.WriteLine 42 +""" + } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - there is already an open directive`` () = @@ -87,7 +126,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -97,10 +136,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - top level module is explicit`` () = @@ -111,7 +151,58 @@ Console.WriteLine 42 """ let expected = - Some + [ + { + Message = "open System" + FixedCode = + """module Module1 + +open System + +Console.WriteLine 42 +""" + } + ] + + let actual = openFixes code Auto + + 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 = + [ + { + Message = "open System" + FixedCode = + """[] module Module1 + +open System + +Console.WriteLine 42 +""" + } + ] + + let actual = openFixes 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 = + [ { Message = "open System" FixedCode = @@ -122,10 +213,38 @@ open System Console.WriteLine 42 """ } + ] + + let actual = openFixes 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 = + [ + { + Message = "open System" + FixedCode = + """namespace N1 + +open System + +module M1 = + Console.WriteLine 42 +""" + } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - nested module`` () = @@ -136,7 +255,7 @@ let ``Fixes FS0039 for missing opens - nested module`` () = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -147,10 +266,11 @@ module Module1 = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - explicit module has attributes`` () = @@ -163,7 +283,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -176,10 +296,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - implicit module has attributes`` () = @@ -191,7 +312,7 @@ type MyType() = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -203,10 +324,11 @@ type MyType() = let now = DateTime.Now """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - nested module has attributes`` () = @@ -219,7 +341,7 @@ module Module1 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -232,10 +354,11 @@ module Module1 = Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - module has multiple attributes`` () = @@ -249,7 +372,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -263,10 +386,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - attributes are mixed with empty lines`` () = @@ -281,7 +405,7 @@ Console.WriteLine 42 """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -296,10 +420,11 @@ open System Console.WriteLine 42 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - multiple modules in one file`` () = @@ -315,7 +440,7 @@ module Module2 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -331,10 +456,11 @@ module Module2 = Console.WriteLine(42) """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - explicit namespace`` () = @@ -348,7 +474,7 @@ module M1 = """ let expected = - Some + [ { Message = "open System" FixedCode = @@ -362,10 +488,178 @@ module M1 = Console.WriteLine 42 """ } + ] + + let actual = openFixes code Auto + + Assert.Equal(expected, actual) + +[] // A plain `open` only reaches namespaces and modules; a type nested in a type needs `open type` +let ``Fixes FS0039 with open type for a type nested in a type`` () = + let code = + """module Module1 + +let folder () = SpecialFolder.Desktop +""" + + let expected = + [ + { + Message = "open type System.Environment" + FixedCode = + """module Module1 + +open type System.Environment + +let folder () = SpecialFolder.Desktop +""" + } + ] + + let actual = openFixes code Auto + + Assert.Equal(expected, actual) + +[] // `File` is both `System.IO.File` and the nested `System.Net.WebRequestMethods.File` +let ``Offers every namespace a name can be resolved from`` () = + let code = + """module Module1 + +let readFile () = File.ReadAllText "example.txt" +""" + + let expected = + [ + { + Message = "open System.IO" + FixedCode = + """module Module1 + +open System.IO + +let readFile () = File.ReadAllText "example.txt" +""" + } + { + Message = "open type System.Net.WebRequestMethods" + FixedCode = + """module Module1 + +open type System.Net.WebRequestMethods + +let readFile () = File.ReadAllText "example.txt" +""" + } + ] + + let actual = openFixes code Auto + + Assert.Equal(expected, actual) + +[] // Qualifying the name in place is offered alongside opening what holds it +let ``Offers qualifying the name after the opens`` () = + let code = + """module Module1 + +let readFile () = File.ReadAllText "example.txt" +""" + + let expected = + [ + "open System.IO" + "open type System.Net.WebRequestMethods" + // Qualifications, three of them at most, `System.IO.File` twice over because the type and + // the member being reached through it are both candidates. + "System.IO.File" + "System.IO.File.ReadAllText" + "System.Net.WebRequestMethods.File" + ] + + let actual = allFixes code Auto |> List.map _.Message + + Assert.Equal(expected, actual) + +[] // `WriteLine` is a static member of four different types, `System` ones offered first +let ``Offers every type a static member can be resolved from`` () = + let code = + """module Module1 + +let write () = WriteLine "hi" +""" + + let expected = + [ + { + Message = "open type System.Console" + FixedCode = + """module Module1 + +open type System.Console + +let write () = WriteLine "hi" +""" + } + { + Message = "open type System.Diagnostics.Debug" + FixedCode = + """module Module1 + +open type System.Diagnostics.Debug + +let write () = WriteLine "hi" +""" + } + { + Message = "open type System.Diagnostics.Trace" + FixedCode = + """module Module1 + +open type System.Diagnostics.Trace + +let write () = WriteLine "hi" +""" + } + { + Message = "open type Microsoft.VisualBasic.FileSystem" + FixedCode = + """module Module1 + +open type Microsoft.VisualBasic.FileSystem + +let write () = WriteLine "hi" +""" + } + ] + + let actual = openFixes code Auto + + Assert.Equal(expected, actual) + +[] // NEGATIVE: a type sitting directly in a namespace is reached by a plain open +let ``Fixes FS0039 with a plain open for a type in a namespace`` () = + let code = + """module Module1 + +let write () = Console.WriteLine "hi" +""" + + let expected = + [ + { + Message = "open System" + FixedCode = + """module Module1 + +open System + +let write () = Console.WriteLine "hi" +""" + } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Doesn't fix FS0039 for random undefined symbols`` () = @@ -374,11 +668,11 @@ let ``Doesn't fix FS0039 for random undefined symbols`` () = let f = g """ - let expected = None + let expected = [] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0043 for missing opens`` () = @@ -392,7 +686,7 @@ module N = """ let expected = - Some + [ { Message = "open M" FixedCode = @@ -406,10 +700,11 @@ module N = let theAnswer = 4 ++ 2 """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Doesn't fix FS0043 for random unsupported values`` () = @@ -420,11 +715,11 @@ type RecordType = { X : int } let x : RecordType = null """ - let expected = None + let expected = [] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual) [] let ``Fixes FS0039 for missing opens - module has multiline attributes`` () = @@ -443,7 +738,7 @@ module FlatList = """ let expected = - Some + [ { Message = "open System.Collections.Generic" FixedCode = @@ -461,7 +756,8 @@ module FlatList = let a : KeyValuePair = KeyValuePair("key", 1) """ } + ] - let actual = codeFix |> tryFix code Auto + let actual = openFixes code Auto - Assert.Equal(expected, actual) + Assert.Equal(expected, actual)