diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md index 17bcd329342..0780074fa5b 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md @@ -14,6 +14,7 @@ * Checker: report Bind resolution failures at the bound expression ([PR #20747](https://github.com/dotnet/fsharp/pull/20747)) * A project reusing cached framework imports is checked with its own `--pathmap` instead of the map of the project that filled the cache, so the ranges of its in-memory reference data name its real files. ([Issue #20474](https://github.com/dotnet/fsharp/issues/20474), [PR #20476](https://github.com/dotnet/fsharp/pull/20476)) * Split Linux CoreCLR CI into two parallel test batches to reduce test turnaround time. ([PR #20587](https://github.com/dotnet/fsharp/pull/20587)) +* `ParsedInput.FindNearestPointToInsertOpenDeclaration` reports where an `open` belongs from the syntax tree: `InsertionContext.Pos` is the first line inside the scope for every `ScopeKind`, so an `open` no longer lands above a header written as `[] module Ns.Name` or below the first declaration when no blank line follows the header, and it stays out of a block comment that runs on from the header. `ParsedInput.AdjustInsertionPoint` only skips the blank line under a header. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500)) * Fix wrong results or non-terminating loops in optimized code when an integer `for` loop's start or end expression binds a local. ([Issue #20732](https://github.com/dotnet/fsharp/issues/20732), [PR #20733](https://github.com/dotnet/fsharp/pull/20733)) * Checker: resolve named arguments of failed and unfinished method calls ([PR #20749](https://github.com/dotnet/fsharp/pull/20749)) * Fix `NativePtr.stackalloc` used inside the `with` handler, filter or `finally` block of a `try` expression producing an assembly that throws `InvalidProgramException` at method load. Direct or mandatory-inline allocations are now reported at compile time as error FS3924; calls to non-inline allocation helpers, including handler-local helpers, remain legal and retain their method boundary during inlining and let-binding elimination. ([Issue #20295](https://github.com/dotnet/fsharp/issues/20295), [PR #20491](https://github.com/dotnet/fsharp/pull/20491)) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2357fbcdff4..281b3703645 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -12,11 +12,15 @@ * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128), [PR #20695](https://github.com/dotnet/fsharp/pull/20695)) +* The "open namespace" code fix and the completion that adds an `open` place it from the syntax tree, so it no longer lands above the file's own module declaration (`[] module Ns.Name` on one line) or below the first declaration when no blank line follows the header, or inside a block comment that runs on from the header, and both follow the file's line endings. ([PR #20500](https://github.com/dotnet/fsharp/pull/20500)) +* The "open namespace" code fix offers `open type System.Console` where it used to offer `open System.Console`, which does not compile: a plain `open` only reaches namespaces and F# modules. A type inside a generic type is not offered, since `open type` needs type arguments the name does not carry. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501)) +* The "open namespace" code fix lists every namespace and type a name can be resolved from instead of one arbitrary suggestion: an unresolved `File` was answered with `System.Net.WebRequestMethods` and `System.IO` was never offered. As in Roslyn's add-import fix, the list is capped at five opens and three qualifications and ordered with `System` first. ([PR #20501](https://github.com/dotnet/fsharp/pull/20501)) * Fix record-type Rename corrupting copy-and-update expressions, including from abstract signatures, while retaining Find All References results. ([Issue #20791](https://github.com/dotnet/fsharp/issues/20791), [PR #20797](https://github.com/dotnet/fsharp/pull/20797)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Avoid using `cancellableTask` in `DocumentCache`; the editor cache now uses direct `CancellationToken`-aware `task` wrappers, avoiding the background `Task.Run` offload and a larger wrapper closure from the `cancellableTask` builder. ([Issue #20268](https://github.com/dotnet/fsharp/issues/20268)) * Cache document diagnostics by version stamp, so an unchanged document is not reanalyzed on every crawler pass. ([Issue #20120](https://github.com/dotnet/fsharp/issues/20120), [PR #20121](https://github.com/dotnet/fsharp/pull/20121)) +* The "open namespace" code fix offers the `open` that brings an extension member into scope when the name is used as a member of a value or a type: the namespace of a C#-style extension method (`open System.Linq` for `xs.Where(...)`), or the module of an F# extension member and, for an `[]` module, also what opens it. A C#-style extension method written as a bare name no longer gets `open type` of its class, which does not make the method available by name. ([PR #20670](https://github.com/dotnet/fsharp/pull/20670)) * Find All References for external DLL symbols now only searches projects that reference the specific assembly. ([Issue #10227](https://github.com/dotnet/fsharp/issues/10227), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297)) * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421)) diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index cd68e7eb608..ed38383cf2d 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/src/Compiler/Service/ServiceParsedInputOps.fs @@ -266,12 +266,16 @@ module Entity = | _ -> let fullRelativeName = Array.append (getRelativeNs fullOpenableNs) restIdents + let withoutQualifierWritten (idents: ShortIdents) = + if identCount > 1 && relativeNs.Length >= identCount then + idents[0 .. idents.Length - identCount] + else + idents + let ns = match relativeNs with | [||] -> None - | _ when identCount > 1 && relativeNs.Length >= identCount -> - Some(relativeNs[0 .. relativeNs.Length - identCount] |> formatIdents false) - | _ -> Some(formatIdents false relativeNs) + | _ -> Some(withoutQualifierWritten relativeNs |> formatIdents false) let qualifier = if fullRelativeName.Length > 1 && fullRelativeName.Length >= identCount then @@ -279,7 +283,7 @@ module Entity = else fullRelativeName - Some + let entity = { FullRelativeName = formatIdents true fullRelativeName Qualifier = formatIdents (qualifier.Length = fullRelativeName.Length) qualifier @@ -289,7 +293,9 @@ module Entity = | [| _ |] -> "" | _ -> formatIdents true restIdents LastIdent = Array.tryLast restIdents |> Option.defaultValue "" - }) + } + + Some(struct (entity, (withoutQualifierWritten openableNs).Length))) module ParsedInput = @@ -2434,12 +2440,23 @@ module ParsedInput = Kind: ScopeKind } - let tryFindNearestPointAndModules currentLine (ast: ParsedInput) (insertionPoint: OpenStatementInsertionPoint) = + let tryFindNearestPointAndModules (currentPos: pos) (ast: ParsedInput) (insertionPoint: OpenStatementInsertionPoint) = // We ignore all diagnostics during this operation // // Based on an initial review, no diagnostics should be generated. However the code should be checked more closely. use _ignoreAllDiagnostics = new DiagnosticsScope(false) + let currentLine = currentPos.Line + + let blockComments = + match ast with + | ParsedInput.ImplFile input -> + input.Trivia.CodeComments + |> List.choose (function + | CommentTrivia.BlockComment range -> Some range + | CommentTrivia.LineComment _ -> None) + | ParsedInput.SigFile _ -> [] + let mutable result = None let mutable ns = None let modules = ResizeArray() @@ -2487,9 +2504,9 @@ module ParsedInput = ) | _ -> () - let getMinColumn decls = + let tryGetFirstDeclStart decls = match decls with - | [] -> None + | [] -> ValueNone | firstDecl :: _ -> match firstDecl with | SynModuleDecl.NestedModule(range = r) @@ -2498,9 +2515,33 @@ module ParsedInput = | SynModuleDecl.Types(range = r) | SynModuleDecl.Exception(range = r) | SynModuleDecl.Open(range = r) - | SynModuleDecl.HashDirective(range = r) -> Some r - | _ -> None - |> Option.map (fun r -> r.StartColumn) + | SynModuleDecl.HashDirective(range = r) -> ValueSome r.Start + | _ -> ValueNone + + let rec skipTrailingBlockComments (firstDeclStart: pos voption) line = + match + blockComments + |> List.tryFind (fun comment -> + comment.StartLine <= line + && comment.EndLine > line + && firstDeclStart |> ValueOption.forall (posLt comment.Start)) + with + | Some comment -> skipTrailingBlockComments firstDeclStart comment.EndLine + | None -> line + + let headerEndLine (keyword: range) (ident: LongIdent) (equals: range option) firstDeclStart = + let nameEndLine = + ident + |> List.fold (fun line part -> max line part.idRange.EndLine) keyword.EndLine + + equals + |> Option.fold (fun line equalsSign -> max line equalsSign.EndLine) nameEndLine + |> skipTrailingBlockComments firstDeclStart + + let bodyAnchor headerLine (firstDeclStart: pos voption) defaultColumn = + match firstDeclStart with + | ValueSome start when start.Line = headerLine -> struct (headerLine - 1, start.Column) + | _ -> struct (headerLine, defaultColumn) let rec walkImplFileInput (file: ParsedImplFileInput) = List.iter (walkSynModuleOrNamespace []) file.Contents @@ -2522,14 +2563,16 @@ module ParsedInput = let fullIdent = parent @ ident - // Use trivia to get the actual module/namespace keyword line, which excludes attributes - let startLine = + let firstDeclStart = tryGetFirstDeclStart decls + + 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 firstDeclStart + | SynModuleOrNamespaceLeadingKeyword.None -> range.StartLine - 1 + + let struct (lineAboveBody, column) = + bodyAnchor headerLine firstDeclStart range.StartColumn let scopeKind = match isModule, parent with @@ -2537,7 +2580,7 @@ module ParsedInput = | true, _ -> NestedModule | _ -> Namespace - doRange scopeKind fullIdent startLine range.StartColumn + doRange scopeKind fullIdent lineAboveBody column addModule (fullIdent, range) List.iter (walkSynModuleDecl fullIdent) decls @@ -2545,22 +2588,36 @@ module ParsedInput = match decl with | SynModuleDecl.NamespaceFragment fragment -> walkSynModuleOrNamespace parent fragment | SynModuleDecl.NestedModule(moduleInfo = compInfo; decls = decls; range = range; trivia = trivia) -> + let (SynComponentInfo(attributes = attributes)) = compInfo let ident = compInfo.LongIdent let fullIdent = parent @ ident addModule (fullIdent, range) if range.EndLine >= currentLine then - // Use trivia to get the actual module keyword line, which excludes attributes - let moduleKeywordLine = + let firstDeclStart = tryGetFirstDeclStart decls + + let headerLine = match trivia.ModuleKeyword with - | Some moduleKeywordRange -> moduleKeywordRange.StartLine + | Some moduleKeyword -> headerEndLine moduleKeyword ident trivia.EqualsRange firstDeclStart | None -> range.StartLine // Fallback if trivia unavailable let moduleBodyIndentation = - getMinColumn decls |> Option.defaultValue (range.StartColumn + 4) - - doRange NestedModule fullIdent moduleKeywordLine moduleBodyIndentation - List.iter (walkSynModuleDecl fullIdent) decls + firstDeclStart + |> ValueOption.map _.Column + |> ValueOption.defaultValue (range.StartColumn + 4) + + let struct (lineAboveBody, bodyColumn) = + bodyAnchor headerLine firstDeclStart moduleBodyIndentation + + let useMayBeInModuleAttribute = + firstDeclStart + |> ValueOption.exists (fun start -> start.Line = headerLine && posLt currentPos start) + && attributes + |> List.exists (fun attributeList -> attributeList.Range.EndLine = headerLine) + + if headerLine <= currentLine && not useMayBeInModuleAttribute then + doRange NestedModule fullIdent lineAboveBody bodyColumn + List.iter (walkSynModuleDecl fullIdent) decls | SynModuleDecl.Open(_, range) -> doRange OpenDeclaration [] range.EndLine (range.StartColumn - 5) | SynModuleDecl.HashDirective(_, range) -> doRange HashDirective [] range.EndLine range.StartColumn | _ -> () @@ -2604,14 +2661,14 @@ module ParsedInput = Pos = mkPos (Line.fromZ m.Range.EndLine) m.Range.StartColumn }) - let TryFindInsertionContext - (currentLine: int) + let TryFindInsertionContextAt + (currentPos: pos) (parsedInput: ParsedInput) (partiallyQualifiedName: MaybeUnresolvedIdent[]) (insertionPoint: OpenStatementInsertionPoint) = let res, modules = - tryFindNearestPointAndModules currentLine parsedInput insertionPoint + tryFindNearestPointAndModules currentPos parsedInput insertionPoint let res = res @@ -2654,63 +2711,38 @@ module ParsedInput = ) entities - |> Array.map (fun e -> + |> Array.map (fun (struct (e, namespaceIdentCount)) -> match find () with - | struct (_, context) -> e, context) - - /// Corrects insertion line number based on kind of scope and text surrounding the insertion point. - let AdjustInsertionPoint (getLineStr: int -> string) ctx = - let line = - match ctx.ScopeKind with - | ScopeKind.TopModule -> - if ctx.Pos.Line > 1 then - // it's an implicit module without any open declarations - let line = getLineStr (ctx.Pos.Line - 2) - - let isImplicitTopLevelModule = - not (line.StartsWithOrdinal("module") && not (line.EndsWithOrdinal("="))) + | struct (_, context) -> struct (e, context, namespaceIdentCount)) - 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 + let TryFindInsertionContext + (currentLine: int) + (parsedInput: ParsedInput) + (partiallyQualifiedName: MaybeUnresolvedIdent[]) + (insertionPoint: OpenStatementInsertionPoint) + = + let findAt = + TryFindInsertionContextAt (mkPos currentLine 0) parsedInput partiallyQualifiedName insertionPoint - mkPos line ctx.Pos.Column + fun candidate -> + findAt candidate + |> Array.map (fun (struct (entity, context, _)) -> entity, context) - let FindNearestPointToInsertOpenDeclaration - (currentLine: int) + let AdjustInsertionPoint (getLineStr: int -> string) ctx = + 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 FindNearestPointToInsertOpenDeclarationAt + (currentPos: pos) (parsedInput: ParsedInput) (entity: ShortIdents) (insertionPoint: OpenStatementInsertionPoint) = let ctx = - match tryFindNearestPointAndModules currentLine parsedInput insertionPoint with + match tryFindNearestPointAndModules currentPos parsedInput insertionPoint with | Some(scope, _, point), modules -> let struct (_, context) = findBestPositionToInsertOpenDeclaration modules scope point entity @@ -2748,8 +2780,7 @@ module ParsedInput = | _ -> 0 if lastReferenceLine > 0 then - // `AdjustInsertionPoint` snaps a `TopModule` position up to line 1, above the directives; - // remap it to `HashDirective`, which (like the other scopes) it passes through unchanged. + // AdjustInsertionPoint would move a TopModule position below the blank line under the directives. let scopeKind = if ctx.ScopeKind = ScopeKind.TopModule then ScopeKind.HashDirective @@ -2763,3 +2794,11 @@ module ParsedInput = else ctx | _ -> ctx + + let FindNearestPointToInsertOpenDeclaration + (currentLine: int) + (parsedInput: ParsedInput) + (entity: ShortIdents) + (insertionPoint: OpenStatementInsertionPoint) + = + FindNearestPointToInsertOpenDeclarationAt (mkPos currentLine 0) parsedInput entity insertionPoint diff --git a/src/Compiler/Service/ServiceParsedInputOps.fsi b/src/Compiler/Service/ServiceParsedInputOps.fsi index ed83c88f5a9..fa0c2fdf932 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, below its header, + /// opens and directives. For a nested module whose body starts on the header's line, that line. Pos: pos } @@ -198,6 +199,21 @@ module public ParsedInput = ShortIdents) -> (InsertionContextEntity * InsertionContext)[]) + /// Like `TryFindInsertionContext`, for a use at a position: on the line of a nested module declared in one line + /// that carries an attribute, only a use after the start of the module's body is inside the module. + /// Each result also says how many leading idents of the entity's full name its `Namespace` covers, which tells + /// whether a plain `open` reaches it (a namespace or F# module) or only `open type` does (a type). + val internal TryFindInsertionContextAt: + currentPos: pos -> + parsedInput: ParsedInput -> + partiallyQualifiedName: MaybeUnresolvedIdent[] -> + insertionPoint: OpenStatementInsertionPoint -> + (( (* requiresQualifiedAccessParent: *) ShortIdents option (* autoOpenParent: *) * + ShortIdents option (* entityNamespace *) * + ShortIdents option (* entity: *) * + ShortIdents) + -> struct (InsertionContextEntity * InsertionContext * (* namespaceIdentCount: *) int)[]) + /// Returns `InsertContext` based on current position and symbol idents. val FindNearestPointToInsertOpenDeclaration: currentLine: int -> @@ -206,10 +222,19 @@ module public ParsedInput = insertionPoint: OpenStatementInsertionPoint -> InsertionContext + /// Like `FindNearestPointToInsertOpenDeclaration`, for a use at a position. + val internal FindNearestPointToInsertOpenDeclarationAt: + currentPos: pos -> + parsedInput: ParsedInput -> + entity: ShortIdents -> + insertionPoint: OpenStatementInsertionPoint -> + InsertionContext + /// Returns long identifier at position. val GetLongIdentAt: parsedInput: ParsedInput -> pos: pos -> LongIdent option - /// Corrects insertion line number based on kind of scope and text surrounding the insertion point. + /// Moves the insertion point below the blank line that follows a declaration header. `getLineStr` returns + /// the trimmed text of a zero-based line, or an empty string past the end of the file. val AdjustInsertionPoint: getLineStr: (int -> string) -> ctx: InsertionContext -> pos [] diff --git a/tests/FSharp.Compiler.Service.Tests/ActivePatternCompletionTests.fs b/tests/FSharp.Compiler.Service.Tests/ActivePatternCompletionTests.fs index b7f00cffcd0..6acb6e3c5d5 100644 --- a/tests/FSharp.Compiler.Service.Tests/ActivePatternCompletionTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/ActivePatternCompletionTests.fs @@ -509,7 +509,7 @@ let ``referenced case names retain the namespace-level insertion scope`` (patter |> Assert.Single Assert.Equal(Some ns, entity.Namespace) Assert.Equal(qualifier, entity.Qualifier) - Assert.Equal((if atTop then Position.mkPos 1 0 else Position.mkPos 4 8), insertion.Pos) + Assert.Equal((if atTop then Position.mkPos 2 0 else Position.mkPos 4 8), insertion.Pos) let opened = if atTop then context.Source.Replace("namespace N\n", $"namespace N\nopen {entity.Namespace.Value}\n") diff --git a/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs b/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs index ef1f09f466b..886e0507ce3 100644 --- a/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/OpenDeclarationInsertionTests.fs @@ -3,25 +3,17 @@ module FSharp.Compiler.Service.Tests.OpenDeclarationInsertionTests open Xunit open FSharp.Compiler.EditorServices open FSharp.Compiler.Service.Tests.Common +open FSharp.Compiler.Text /// 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 private adjustedInsertion findNearest fileName (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 ctx = findNearest parseTree (entity.Split('.')) let getLineStr line = if line >= 0 && line < lines.Length then lines.[line].Trim() else "" @@ -29,6 +21,25 @@ let private findOpenInsertion (insertionPoint: OpenStatementInsertionPoint) (fil let pos = ParsedInput.AdjustInsertionPoint getLineStr ctx pos.Line, pos.Column +let private findOpenInsertionAt currentLine insertionPoint fileName source entity = + adjustedInsertion + (fun parseTree idents -> + ParsedInput.FindNearestPointToInsertOpenDeclaration currentLine parseTree idents insertionPoint) + fileName + source + entity + +let private findOpenInsertionFrom (usePos: pos) insertionPoint fileName source entity = + adjustedInsertion + (fun parseTree idents -> + ParsedInput.FindNearestPointToInsertOpenDeclarationAt usePos parseTree idents insertionPoint) + fileName + source + entity + +let private findOpenInsertion insertionPoint fileName (source: string) entity = + findOpenInsertionAt (source.TrimEnd().Split('\n').Length) insertionPoint fileName source entity + let private findOpenInsertionLine fileName source entity = findOpenInsertion OpenStatementInsertionPoint.TopLevel fileName source entity |> fst @@ -116,8 +127,85 @@ 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. - Assert.Equal(2, line) + Assert.Equal(3, line) + +[] +[] module Foo + +let x = System.IO.File.ReadAllText "a" +""", 3)>] +[] +[] +[] +[] +let ``Open goes below the header of a module or namespace, with or without a blank line`` (source: string, expectedLine: int) = + Assert.Equal(expectedLine, findOpenInsertionLine "test.fs" source "System.IO") + +[] +let ``Open goes in front of the declaration that follows a block comment ending the header`` () = + let source = """module Foo (* header +*) let x = System.IO.File.ReadAllText "a" +""" + Assert.Equal((2, 3), findOpenInsertion OpenStatementInsertionPoint.TopLevel "test.fs" source "System.IO") + +[] +let ``Open goes below every block comment that runs on from the header`` () = + let source = """module Top + +module M = (* first +*) (* second +*) + let x = System.IO.File.ReadAllText "a" +""" + Assert.Equal((6, 4), findOpenInsertion OpenStatementInsertionPoint.Nearest "test.fs" source "System.IO") + +[] +let ``Open placed at the first declaration of a nested module declared on one line`` () = + let source = """module Top + +module M = let x = System.IO.File.ReadAllText "a" +""" + let pos = findOpenInsertion OpenStatementInsertionPoint.Nearest "test.fs" source "System.IO" + Assert.Equal((3, 11), pos) + +[] +let ``Open ignores a block comment that starts inside the first declaration of a nested module`` () = + let source = """module Top +module M = let x = (* body + *) System.IO.File.ReadAllText "a" +""" + let pos = findOpenInsertion OpenStatementInsertionPoint.Nearest "test.fs" source "System.IO" + Assert.Equal((2, 11), pos) + +[] +[] +[] module M = let y = 1", 2)>] +[] module M = let y = 1", 3)>] +let ``Open stays out of a nested module declared on one line that the use is not in`` (nestedModule: string, useLine: int) = + let source = $"module Top\nlet x = Console.WriteLine 42\n{nestedModule}\n" + let pos = findOpenInsertionAt useLine OpenStatementInsertionPoint.Nearest "test.fs" source "System" + Assert.Equal((2, 0), pos) + +[] +[] +[] +let ``Open goes into an attributed nested module declared on one line only for a use in its body`` (useColumn: int, openColumn: int) = + let source = "module Top\n[] module M = let y = System.IO.File.ReadAllText \"a\"\n" + let usePos = Position.mkPos 2 useColumn + let pos = findOpenInsertionFrom usePos OpenStatementInsertionPoint.Nearest "test.fs" source "System.IO" + Assert.Equal((2, openColumn), pos) [] // Only #r/#load drive placement; other directives (#time/#help/#I/...) must not let ``Non reference directive does not drive placement in fsx`` () = diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index 10b0db02ef3..1893bcd2928 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -3,13 +3,16 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System -open System.Collections.Generic open System.Composition +open System.Collections.Generic open System.Collections.Immutable +open System.Runtime.CompilerServices +open System.Threading open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes +open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.EditorServices open FSharp.Compiler.Symbols open FSharp.Compiler.Syntax @@ -17,60 +20,59 @@ open FSharp.Compiler.Text open CancellableTasks -[] -type internal AddOpenCodeFixProvider [] (assemblyContentProvider: AssemblyContentProvider) = - inherit CodeFixProvider() +[] +type internal MemberReceiver = + | Value of FSharpType voption + | Type of FSharpEntity - static let br = Environment.NewLine +type internal ReceiverTypes = ResizeArray - let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__") +type internal EntityFinder = + (ShortIdents option * ShortIdents option * ShortIdents option * ShortIdents) + -> struct (InsertionContextEntity * InsertionContext * int)[] - let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) = - { - Name = CodeFix.AddOpen - Message = fixUnderscoresInMenuText fullName - Changes = [ TextChange(context.Span, qualifier) ] - } +type internal NameCandidate = + { + Entity: InsertionContextEntity + Context: InsertionContext + Symbol: AssemblySymbol + NamespaceIdentCount: int + } - 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) +[] +type internal AddOpenCodeFixProvider [] (assemblyContentProvider: AssemblyContentProvider) = + inherit CodeFixProvider() - let getLineStr line = - sourceText.Lines[line].ToString().Trim() + let maxOpenSuggestions = 5 + let maxQualifySuggestions = 3 - let skipBlank line = - if getLineStr line = "" then line + 1 else line + let systemFirst (name: ReadOnlySpan) (sortText: string) = + let isInSystem = + name.Equals("System".AsSpan(), StringComparison.Ordinal) + || name.StartsWith("System.".AsSpan(), StringComparison.Ordinal) - let startLineNumber, openDeclaration = - match ctx.ScopeKind with - | ScopeKind.TopModule -> - match getLineStr insertionLineNumber with + (if isInSystem then 0 else 1), sortText - // explicit top level module - | line when line.StartsWith "module" && not (line.EndsWith "=") -> - skipBlank (insertionLineNumber + 1), $"{margin}open {ns}{br}{br}" + let openOrder (declaration: string) = + systemFirst (declaration.AsSpan(declaration.LastIndexOf ' ' + 1)) declaration - // nested module, shouldn't be here - | line when line.StartsWith "module" -> insertionLineNumber, $"{margin}open {ns}{br}{br}" + let qualificationOrder (fullName: string) = + systemFirst (fullName.AsSpan()) fullName - // implicit top level module - | _ -> insertionLineNumber, $"{margin}open {ns}{br}{br}" + let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__") - | ScopeKind.Namespace - | ScopeKind.NestedModule -> - let pos = OpenDeclarationHelper.getInsertionPosition getLineStr ctx - skipBlank (Line.toZ pos.Line), $"{margin}open {ns}{br}{br}" - | ScopeKind.OpenDeclaration -> insertionLineNumber + 1, $"{margin}open {ns}{br}" - | ScopeKind.HashDirective -> insertionLineNumber + 1, $"open {ns}{br}{br}" + let qualifySymbolFix (firstIdentSpan: TextSpan) (fullName, qualifier) = + { + Name = CodeFix.AddOpen + Message = fixUnderscoresInMenuText fullName + Changes = [ TextChange(firstIdentSpan, qualifier) ] + } - let start = sourceText.Lines[startLineNumber].Start - TextChange(TextSpan(start, 0), openDeclaration) + let openNamespaceFix ctx name declaration multipleNames sourceText = + let displayText = declaration + (if multipleNames then " (" + name + ")" else "") - 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 @@ -78,44 +80,434 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr Changes = [ change ] } - let getSuggestionsAsCodeFixes - (context: CodeFixContext) + let declaringEntity (symbol: FSharpSymbol) = + match symbol with + | :? FSharpEntity as entity -> entity.DeclaringEntity + | :? FSharpMemberOrFunctionOrValue as mfv -> mfv.DeclaringEntity + | _ -> None + + let rec tryFindModuleOrTypeNamedByLeading (leadingIdentCount: int) (nameIdentCount: int) (symbol: FSharpSymbol) = + match declaringEntity symbol with + | Some parent when not parent.IsNamespace -> + if nameIdentCount - 1 = leadingIdentCount then + ValueSome parent + else + tryFindModuleOrTypeNamedByLeading leadingIdentCount (nameIdentCount - 1) parent + | _ -> ValueNone + + let isCSharpStyleExtension (method: FSharpMemberOrFunctionOrValue) = + not method.IsExtensionMember && method.HasAttribute() + + let tryGetOpenDeclaration (candidate: NameCandidate) ns = + let symbol = candidate.Symbol + + match symbol.Symbol, tryFindModuleOrTypeNamedByLeading candidate.NamespaceIdentCount symbol.CleanedIdents.Length symbol.Symbol with + | (:? FSharpMemberOrFunctionOrValue as method), _ when isCSharpStyleExtension method -> ValueNone + | _, ValueSome opened when opened.IsFSharpModule -> ValueSome $"open {ns}" + | _, ValueSome opened when opened.GenericParameters.Count > 0 -> ValueNone + | _, ValueSome _ -> ValueSome $"open type {ns}" + | _, ValueNone -> ValueSome $"open {ns}" + + let isReachedByName (symbol: AssemblySymbol) = + match symbol.Symbol with + | :? FSharpMemberOrFunctionOrValue as mfv -> not mfv.IsExtensionMember + | _ -> true + + let isQualifiable (entity: InsertionContextEntity) = not (entity.LastIdent.StartsWith "op_") + + let memberReceiverOf (symbol: FSharpSymbol option) = + match symbol with + | Some(:? FSharpEntity as entity) when entity.IsNamespace || entity.IsFSharpModule -> ValueNone + | Some(:? FSharpEntity as entity) -> ValueSome(MemberReceiver.Type entity) + | Some(:? FSharpMemberOrFunctionOrValue as value) -> ValueSome(MemberReceiver.Value(ValueSome value.FullType)) + | Some(:? FSharpField as field) -> ValueSome(MemberReceiver.Value(ValueSome field.FieldType)) + | Some(:? FSharpParameter as parameter) -> ValueSome(MemberReceiver.Value(ValueSome parameter.Type)) + | Some _ -> ValueSome(MemberReceiver.Value ValueNone) + | None -> ValueNone + + let tryGetMemberReceiver + (parseResults: FSharpParseFileResults) + (checkResults: FSharpCheckFileResults) (sourceText: SourceText) - (candidates: (InsertionContextEntity * InsertionContext) list) + (longIdent: LongIdent) + (unresolvedIdentRange: range) + = + match ParsedInput.GetRangeOfExprLeftOfDot(unresolvedIdentRange.Start, parseResults.ParseTree) with + | Some receiverRange when not (Position.posGt receiverRange.End unresolvedIdentRange.Start) -> + match longIdent |> List.takeWhile (fun ident -> ident.idRange <> unresolvedIdentRange) with + | [] -> ValueSome(MemberReceiver.Value ValueNone) + | receiverIdents -> + let lastIdent = List.last receiverIdents + let lineText = sourceText.Lines[Line.toZ lastIdent.idRange.EndLine].ToString() + + checkResults.GetSymbolUseAtLocation( + lastIdent.idRange.EndLine, + lastIdent.idRange.EndColumn, + lineText, + receiverIdents |> List.map _.idText + ) + |> Option.map _.Symbol + |> memberReceiverOf + | _ -> ValueNone + + let tryGetDefinitionName (ty: FSharpType) = + let ty = ty.ErasedType + + if ty.HasTypeDefinition then + ty.TypeDefinition.TryFullName + else + None + + let getReceiverTypes (ty: FSharpType) = + let types = ReceiverTypes() + let baseTypeNames = HashSet(StringComparer.Ordinal) + + let rec addWithBaseTypes (ty: FSharpType) = + match tryGetDefinitionName ty with + | Some name when baseTypeNames.Add name -> + types.Add(struct (name, ty)) + ty.BaseType |> Option.iter addWithBaseTypes + | _ -> () + + addWithBaseTypes ty + + for implemented in ty.AllInterfaces do + match tryGetDefinitionName implemented with + | Some name -> types.Add(struct (name, implemented)) + | None -> () + + types + + let rec typeArgumentMatches (extended: FSharpType) (actual: FSharpType) = + extended.IsGenericParameter + || actual.IsGenericParameter + || match tryGetDefinitionName extended, tryGetDefinitionName actual with + | Some extendedName, Some actualName -> extendedName = actualName && typeArgumentsMatch extended actual + | None, None -> true + | _ -> false + + and typeArgumentsMatch (extended: FSharpType) (actual: FSharpType) = + let extendedArguments = extended.ErasedType.GenericArguments + let actualArguments = actual.ErasedType.GenericArguments + + extendedArguments.Count = actualArguments.Count + && Seq.forall2 typeArgumentMatches extendedArguments actualArguments + + let receiverTypesInclude (receiverTypes: ReceiverTypes) (extended: FSharpType) = + match tryGetDefinitionName extended with + | Some extendedName -> receiverTypes.Exists(fun struct (name, ty) -> name = extendedName && typeArgumentsMatch extended ty) + | None -> true + + let fsharpExtensionExtendsOneOf (receiverTypes: ReceiverTypes) (extension: FSharpMemberOrFunctionOrValue) = + match extension.ApparentEnclosingEntity |> Option.bind _.TryFullName with + | Some extendedName -> receiverTypes.Exists(fun struct (name, _) -> name = extendedName) + | None -> true + + let hasPublicDefaultConstructor (entity: FSharpEntity) = + entity.MembersFunctionsAndValues + |> Seq.exists (fun candidate -> + candidate.IsConstructor + && candidate.Accessibility.IsPublic + && candidate.CurriedParameterGroups |> Seq.forall (fun group -> group.Count = 0)) + + let definitionMeetsKindConstraint (definition: FSharpEntity) (typeConstraint: FSharpGenericParameterConstraint) = + (not typeConstraint.IsNonNullableValueTypeConstraint || definition.IsValueType) + && (not typeConstraint.IsReferenceTypeConstraint || not definition.IsValueType) + && (not typeConstraint.IsUnmanagedConstraint || definition.IsValueType) + && (not typeConstraint.IsEnumConstraint || definition.IsEnum) + && (not typeConstraint.IsDelegateConstraint || definition.IsDelegate) + && (not typeConstraint.IsRequiresDefaultConstructorConstraint + || definition.IsValueType + || hasPublicDefaultConstructor definition) + + let receiverMeetsKindConstraint (receiverTypes: ReceiverTypes) (typeConstraint: FSharpGenericParameterConstraint) = + receiverTypes.Count = 0 + || (let struct (_, receiver) = receiverTypes[0] + definitionMeetsKindConstraint receiver.ErasedType.TypeDefinition typeConstraint) + + let receiverMeetsConstraint (receiverTypes: ReceiverTypes) (typeConstraint: FSharpGenericParameterConstraint) = + receiverMeetsKindConstraint receiverTypes typeConstraint + && (not typeConstraint.IsCoercesToConstraint + || typeConstraint.CoercesToTarget.IsGenericParameter + || receiverTypesInclude receiverTypes typeConstraint.CoercesToTarget) + + let csharpExtensionExtendsOneOf (receiverTypes: ReceiverTypes) (extension: FSharpMemberOrFunctionOrValue) = + match extension.CurriedParameterGroups |> Seq.tryHead |> Option.bind Seq.tryHead with + | Some thisParameter when thisParameter.Type.IsGenericParameter -> + thisParameter.Type.GenericParameter.Constraints + |> Seq.forall (receiverMeetsConstraint receiverTypes) + | Some thisParameter -> receiverTypesInclude receiverTypes thisParameter.Type + | None -> true + + let extendsOneOf (receiverTypes: ReceiverTypes voption) (extension: FSharpMemberOrFunctionOrValue) = + match receiverTypes with + | ValueNone -> true + | ValueSome types when extension.IsExtensionMember -> fsharpExtensionExtendsOneOf types extension + | ValueSome types -> csharpExtensionExtendsOneOf types extension + + let moduleExtensionMembers = + ConditionalWeakTable() + + // Asked of every module in reach on each failed member access, so cached per module. + let getExtensionMembers (cancellationToken: CancellationToken) (moduleEntity: FSharpEntity) = + moduleExtensionMembers.GetValue( + moduleEntity, + fun moduleEntity -> + moduleEntity.TryGetMembersFunctionsAndValues() + |> Seq.filter (fun mfv -> + cancellationToken.ThrowIfCancellationRequested() + + mfv.IsExtensionMember + && not mfv.IsPropertyGetterMethod + && not mfv.IsPropertySetterMethod) + |> Seq.toArray + ) + + let getModuleOpenTargets (moduleSymbol: AssemblySymbol) (moduleEntity: FSharpEntity) = + if moduleEntity.HasAttribute() then + [] + else + [ + if moduleEntity.HasAttribute() then + let outermostAutoOpen = + moduleSymbol.AutoOpenParent |> Option.defaultValue moduleSymbol.CleanedIdents + + if outermostAutoOpen.Length > 1 then + outermostAutoOpen[.. outermostAutoOpen.Length - 2] + + moduleSymbol.CleanedIdents + ] + + let getExtensionOpenTargets + (checkResults: FSharpCheckFileResults) + (receiver: MemberReceiver) + (name: string) + (symbols: AssemblySymbol[]) + (cancellationToken: CancellationToken) = + let receiverTypes = + match receiver with + | MemberReceiver.Value(ValueSome ty) -> ValueSome(getReceiverTypes ty) + | MemberReceiver.Value ValueNone -> ValueNone + | MemberReceiver.Type entity -> ValueSome(getReceiverTypes (entity.AsType())) + + let isNamed (symbol: FSharpSymbol) = symbol.DisplayNameCore = name + + let accessibilityRights = checkResults.ProjectContext.AccessibilityRights + + let appliesToReceiver (extension: FSharpMemberOrFunctionOrValue) = + extension.IsInstanceMember = receiver.IsValue + && extension.IsAccessible(accessibilityRights) + && extendsOneOf receiverTypes extension + + let anyOverloadExtends (method: FSharpMemberOrFunctionOrValue) = + match receiverTypes, method.DeclaringEntity with + | ValueSome _, Some staticClass -> + staticClass.TryGetMembersFunctionsAndValues() + |> Seq.exists (fun overload -> + cancellationToken.ThrowIfCancellationRequested() + + isNamed overload + && isCSharpStyleExtension overload + && extendsOneOf receiverTypes overload) + | _ -> extendsOneOf receiverTypes method + + [| + for symbol in symbols do + cancellationToken.ThrowIfCancellationRequested() + + match symbol.Symbol with + | :? FSharpMemberOrFunctionOrValue as method when + receiver.IsValue + && Array.last symbol.CleanedIdents = name + && isCSharpStyleExtension method + && anyOverloadExtends method + -> + match symbol.Namespace with + | Some ns -> struct (symbol, ns) + | None -> () + | :? FSharpEntity as moduleEntity when + moduleEntity.IsFSharpModule + && getExtensionMembers cancellationToken moduleEntity + |> Array.exists (fun extension -> isNamed extension && appliesToReceiver extension) + -> + for target in getModuleOpenTargets symbol moduleEntity do + struct (symbol, target) + | _ -> () + |] + + let getExtensionOpens + (checkResults: FSharpCheckFileResults) + (receiver: MemberReceiver) + (name: string) + (symbols: AssemblySymbol[]) + (cancellationToken: CancellationToken) + (findEntities: EntityFinder) + = + [ + for struct (symbol, target) in getExtensionOpenTargets checkResults receiver name symbols cancellationToken do + for struct (entity, ctx, _) in findEntities (None, None, symbol.Namespace, Array.append target [| name |]) do + match entity.Namespace with + | Some ns -> $"open {ns}", entity.FullDisplayName, ctx + | None -> () + ] + + let nameIdentsOf (symbol: AssemblySymbol) isAttribute = seq { + yield symbol.CleanedIdents + + if isAttribute then + let lastIdent = Array.last symbol.CleanedIdents + + if + lastIdent.EndsWith "Attribute" + && symbol.Kind LookupType.Precise = EntityKind.Attribute + then + yield + symbol.CleanedIdents + |> Array.replace (symbol.CleanedIdents.Length - 1) (lastIdent.Substring(0, lastIdent.Length - 9)) + } + + let getCasesInScope + (context: CodeFixContext) + (parseResults: FSharpParseFileResults) + (checkResults: FSharpCheckFileResults) + (symbols: AssemblySymbol[]) + (unresolvedIdentRange: range) + (line: TextLine) + (linePos: LinePosition) + = + let casesInScope = HashSet(StringComparer.Ordinal) + let sourceLine = line.ToString() + + match ParsedInput.TryGetCompletionContext(unresolvedIdentRange.End, parseResults.ParseTree, sourceLine) with + | Some(CompletionContext.Pattern _) -> + context.CancellationToken.ThrowIfCancellationRequested() + + let cases = + [ + for symbol in symbols do + if symbol.Symbol :? FSharpActivePatternCase then + yield symbol + ] + + let partialName = + { QuickParse.GetPartialLongNameEx(sourceLine, linePos.Character - 1) with + QualifyingIdents = [] + PartialIdent = "" + LastDotPos = None + } + + let visible = + checkResults.GetDeclarationListSymbols( + Some parseResults, + unresolvedIdentRange.EndLine, + sourceLine, + partialName, + getAllEntities = (fun () -> cases) + ) + + for group in visible do + for symbol in group do + if symbol.Symbol :? FSharpActivePatternCase then + casesInScope.Add symbol.Symbol.FullName |> ignore + | _ -> () + + casesInScope + + let withPatternName (patternName: string) (symbol: AssemblySymbol) (entity: InsertionContextEntity) = + if not (symbol.Symbol :? FSharpActivePatternCase) then + entity + elif entity.FullDisplayName <> "" && entity.FullDisplayName <> patternName then + { entity with Namespace = None } + else + { entity with FullDisplayName = "" } + + let getNameCandidates + (symbols: AssemblySymbol[]) + isAttribute + (casesInScope: HashSet) + (patternName: string) + (findEntities: EntityFinder) + = + symbols + |> Seq.filter isReachedByName + |> Seq.filter (fun symbol -> + not (symbol.Symbol :? FSharpActivePatternCase) + || casesInScope.Contains symbol.FullName) + |> Seq.collect (fun symbol -> + nameIdentsOf symbol isAttribute + |> Seq.collect (fun idents -> + findEntities (symbol.TopRequireQualifiedAccessParent, symbol.AutoOpenParent, symbol.Namespace, idents) + |> Seq.map (fun (struct (entity, ctx, namespaceIdentCount)) -> + { + Entity = withPatternName patternName symbol entity + Context = ctx + Symbol = symbol + NamespaceIdentCount = namespaceIdentCount + }))) + |> Seq.toList + + let toMaybeUnresolvedIdents (unresolvedIdentRange: range) (longIdent: LongIdent) = + longIdent + |> List.map (fun ident -> + { + Ident = ident.idText + Resolved = not (ident.idRange = unresolvedIdentRange) + }) + |> List.toArray + + let getSuggestionsAsCodeFixes + (firstIdentSpan: TextSpan) + (sourceText: SourceText) + (candidates: NameCandidate list) + (extensionOpens: (string * string * InsertionContext) list) + = + let nameOpens = 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.chooseV (fun candidate -> + match candidate.Entity.Namespace with + | Some ns -> + tryGetOpenDeclaration candidate ns + |> ValueOption.map (fun declaration -> declaration, candidate.Entity.FullDisplayName, candidate.Context) + | None -> ValueNone) + + seq { + Seq.append nameOpens extensionOpens + |> 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 >> openOrder) + |> 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.map _.Entity + |> Seq.filter isQualifiable + |> Seq.map (fun entity -> entity.FullRelativeName, entity.Qualifier) |> Seq.distinct - |> Seq.sort - |> Seq.map (qualifySymbolFix context) + |> Seq.sortBy (fst >> qualificationOrder) + |> Seq.truncate maxQualifySuggestions + |> Seq.map (qualifySymbolFix firstIdentSpan) } |> Seq.concat 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 @@ -166,113 +558,55 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr let isAttribute = ParsedInput.GetEntityKind(unresolvedIdentRange.Start, parseResults.ParseTree) = Some EntityKind.Attribute - let catalogue = + let symbols = assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies checkResults - let casesInScope = HashSet(StringComparer.Ordinal) - let sourceLine = line.ToString() - - match ParsedInput.TryGetCompletionContext(unresolvedIdentRange.End, parseResults.ParseTree, sourceLine) with - | Some(CompletionContext.Pattern _) -> - context.CancellationToken.ThrowIfCancellationRequested() - - let cases = - [ - for symbol in catalogue do - if symbol.Symbol :? FSharpActivePatternCase then - yield symbol - ] - - let partialName = - { QuickParse.GetPartialLongNameEx(sourceLine, linePos.Character - 1) with - QualifyingIdents = [] - PartialIdent = "" - LastDotPos = None - } - - let symbols = - checkResults.GetDeclarationListSymbols( - Some parseResults, - unresolvedIdentRange.EndLine, - sourceLine, - partialName, - getAllEntities = (fun () -> cases) - ) - - for group in symbols do - for symbol in group do - if symbol.Symbol :? FSharpActivePatternCase then - casesInScope.Add symbol.Symbol.FullName |> ignore - | _ -> () - - let entities = - catalogue - |> Array.collect (fun s -> - [| - if not (s.Symbol :? FSharpActivePatternCase) || casesInScope.Contains s.FullName then - yield s, s.CleanedIdents - - if isAttribute then - let lastIdent = s.CleanedIdents.[s.CleanedIdents.Length - 1] - - if - lastIdent.EndsWith "Attribute" - && s.Kind LookupType.Precise = EntityKind.Attribute - then - yield - s, - s.CleanedIdents - |> Array.replace - (s.CleanedIdents.Length - 1) - (lastIdent.Substring(0, lastIdent.Length - 9)) - |]) - ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End - |> Option.bind (fun longIdent -> - let maybeUnresolvedIdents = - longIdent - |> List.map (fun ident -> - { - Ident = ident.idText - Resolved = not (ident.idRange = unresolvedIdentRange) - }) - |> List.toArray - + |> Option.map (fun longIdent -> let insertionPoint = if document.Project.IsFSharpCodeFixesAlwaysPlaceOpensAtTopLevelEnabled then OpenStatementInsertionPoint.TopLevel else OpenStatementInsertionPoint.Nearest - let createEntity = - ParsedInput.TryFindInsertionContext - unresolvedIdentRange.StartLine + let createEntity (maybeUnresolvedIdents: MaybeUnresolvedIdent[]) : EntityFinder = + ParsedInput.TryFindInsertionContextAt + unresolvedIdentRange.Start parseResults.ParseTree maybeUnresolvedIdents insertionPoint - let patternName = - longIdent - |> List.map (fun ident -> PrettyNaming.NormalizeIdentifierBackticks ident.idText) - |> String.concat "." - - entities - |> Seq.collect (fun (symbol, idents) -> - createEntity (symbol.TopRequireQualifiedAccessParent, symbol.AutoOpenParent, symbol.Namespace, idents) - |> Seq.map (fun (entity, insertionContext) -> - let entity = - if symbol.Symbol :? FSharpActivePatternCase then - if entity.FullDisplayName <> "" && entity.FullDisplayName <> patternName then - { entity with Namespace = None } - else - { entity with FullDisplayName = "" } - else - entity - - entity, insertionContext)) - |> Seq.toList - |> getSuggestionsAsCodeFixes context sourceText - |> Seq.tryHead)) - - |> ValueOption.ofOption + let receiver = + tryGetMemberReceiver parseResults checkResults sourceText longIdent unresolvedIdentRange + + let candidates = + match receiver with + | ValueSome(MemberReceiver.Value _) -> [] + | _ -> + let casesInScope = + getCasesInScope context parseResults checkResults symbols unresolvedIdentRange line linePos + + let patternName = + longIdent + |> List.map (fun ident -> PrettyNaming.NormalizeIdentifierBackticks ident.idText) + |> String.concat "." + + createEntity (toMaybeUnresolvedIdents unresolvedIdentRange longIdent) + |> getNameCandidates symbols isAttribute casesInScope patternName + + let extensionOpens = + match receiver, longIdent |> List.tryFind (fun ident -> ident.idRange = unresolvedIdentRange) with + | ValueSome receiver, Some unresolvedIdent -> + let name = unresolvedIdent.idText + + createEntity [| { Ident = name; Resolved = false } |] + |> getExtensionOpens checkResults receiver name symbols context.CancellationToken + | _ -> [] + + getSuggestionsAsCodeFixes + (RoslynHelpers.FSharpRangeToTextSpan(sourceText, longIdent.Head.idRange)) + sourceText + candidates + extensionOpens)) + |> Option.defaultValue Seq.empty } diff --git a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs index b3c0e8e5765..e868287bbdc 100644 --- a/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs @@ -231,12 +231,53 @@ module internal RoslynHelpers = Async.Start(computation, cancellationToken) module internal OpenDeclarationHelper = - let getInsertionPosition (getLineStr: int -> string) (ctx: InsertionContext) = - match ctx.ScopeKind with - | ScopeKind.Namespace when (getLineStr (ctx.Pos.Line - 1)).StartsWith("namespace", StringComparison.Ordinal) -> - Position.mkPos (ctx.Pos.Line + 1) ctx.Pos.Column - | ScopeKind.Namespace -> ctx.Pos - | _ -> ParsedInput.AdjustInsertionPoint getLineStr ctx + let getOpenDeclarationChange (sourceText: SourceText) (ctx: InsertionContext) (declaration: string) : TextChange = + let getLineStr line = + if line >= 0 && line < sourceText.Lines.Count then + sourceText.Lines[line].ToString().Trim() + else + "" + + let pos = ParsedInput.AdjustInsertionPoint getLineStr ctx + let line = sourceText.Lines[min (Line.toZ pos.Line) (sourceText.Lines.Count - 1)] + + let lineBreakOf (line: TextLine) = + sourceText.ToString(TextSpan.FromBounds(line.End, line.EndIncludingLineBreak)) + + let lineBreak = + match lineBreakOf line with + | "" when line.LineNumber > 0 -> lineBreakOf sourceText.Lines[line.LineNumber - 1] + | "" -> Environment.NewLine + | breakChars -> breakChars + + let blankLineAboveOpen = + match ctx.ScopeKind with + | ScopeKind.TopModule + | ScopeKind.Namespace + | ScopeKind.NestedModule when getLineStr (line.LineNumber - 1) <> "" -> lineBreak + | _ -> "" + + let blankLineBelowOpen = + if ctx.ScopeKind = ScopeKind.OpenDeclaration || getLineStr line.LineNumber = "" then + "" + else + lineBreak + + let margin = String(' ', pos.Column) + let column = min pos.Column (line.End - line.Start) + + let textBeforeDeclaration = + sourceText.ToString(TextSpan(line.Start, column)).TrimEnd() + + let openLine = margin + declaration + lineBreak + + if textBeforeDeclaration.Length > 0 then + TextChange( + TextSpan(line.Start + textBeforeDeclaration.Length, column - textBeforeDeclaration.Length), + lineBreak + openLine + lineBreak + margin + ) + else + TextChange(TextSpan(line.Start, 0), blankLineAboveOpen + openLine + blankLineBelowOpen) /// /// Inserts open declaration into `SourceText`. @@ -245,58 +286,8 @@ module internal OpenDeclarationHelper = /// 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 getLineStr line = - sourceText.Lines.[line].ToString().Trim() - - let pos = getInsertionPosition 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 - 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 - - sourceText, minPos |> Option.defaultValue 0 + 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/src/FSharp.Editor/Completion/CompletionProvider.fs b/vsintegration/src/FSharp.Editor/Completion/CompletionProvider.fs index 8e402c59132..85eea6c76fd 100644 --- a/vsintegration/src/FSharp.Editor/Completion/CompletionProvider.fs +++ b/vsintegration/src/FSharp.Editor/Completion/CompletionProvider.fs @@ -405,11 +405,13 @@ type internal FSharpCompletionProvider | true, keywordDescription -> Task.FromResult(CompletionDescription.FromText(keywordDescription)) | false, _ -> Task.FromResult(CompletionDescription.Empty) - override _.GetChangeAsync(document, item, _, cancellationToken) : Task = + static member GetChangeAsyncAux(document: Document, item: Completion.CompletionItem, insertionPoint: OpenStatementInsertionPoint) = cancellableTask { use _logBlock = Logger.LogBlockMessage document.Name LogEditorFunctionId.Completion_GetChangeAsync + let! cancellationToken = CancellableTask.getCancellationToken () + let fullName = match item.Properties.TryGetValue FullNamePropName with | true, x -> ValueSome x @@ -469,15 +471,9 @@ type internal FSharpCompletionProvider |> ValueOption.map (fun x -> x.Split '.') |> ValueOption.defaultValue [||] - let insertionPoint = - if settings.CodeFixes.AlwaysPlaceOpensAtTopLevel then - OpenStatementInsertionPoint.TopLevel - else - OpenStatementInsertionPoint.Nearest - let ctx = - ParsedInput.FindNearestPointToInsertOpenDeclaration - (Line.fromZ line.LineNumber) + ParsedInput.FindNearestPointToInsertOpenDeclarationAt + (Position.fromZ line.LineNumber (item.Span.Start - line.Start)) parseResults.ParseTree fullNameIdents insertionPoint @@ -494,4 +490,13 @@ type internal FSharpCompletionProvider return CompletionChange.Create(TextChange(fullChangingSpan, changedText)).WithNewPosition(Nullable(changedSpan.End)) } + + override _.GetChangeAsync(document, item, _, cancellationToken) : Task = + let insertionPoint = + if settings.CodeFixes.AlwaysPlaceOpensAtTopLevel then + OpenStatementInsertionPoint.TopLevel + else + OpenStatementInsertionPoint.Nearest + + FSharpCompletionProvider.GetChangeAsyncAux(document, item, insertionPoint) |> CancellableTask.start cancellationToken diff --git a/vsintegration/tests/FSharp.Editor.Tests/ActivePatternEditorTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ActivePatternEditorTests.fs index 47dbc65c2ae..c5aa0844943 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/ActivePatternEditorTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/ActivePatternEditorTests.fs @@ -120,8 +120,18 @@ let private assertFixedCode (expected: string) (actual: string) = Assert.Equal(expected, actual.Replace("\r\n", "\n")) assertCompiles actual +let private tryFirstFix code mode (provider: AddOpenCodeFixProvider) = + provider |> multiFix code mode |> Seq.tryHead + +let private fixNamed (message: string) code mode (provider: AddOpenCodeFixProvider) = + let fixes = provider |> multiFix code mode |> Seq.toList + + match fixes |> List.tryFind (fun fix -> fix.Message = message) with + | Some fix -> fix + | None -> failwith $"""No fix "{message}" among: {fixes |> List.map _.Message |> String.concat "; "}""" + let private addOpenFix code mode = - AddOpenCodeFixProvider(AssemblyContentProvider()) |> tryFix code mode + AddOpenCodeFixProvider(AssemblyContentProvider()) |> tryFirstFix code mode [] [] @@ -140,7 +150,12 @@ let private addOpenFix code mode = [] [] [] -[] +[] let ``Add Open applied case uses exact target and placement`` (caseName: string, parameters: string, ns: string, atTop: bool) = let fallback = if caseName = "Total" then "" else " | _ -> 0" @@ -151,10 +166,9 @@ let ``Add Open applied case uses exact target and placement`` (caseName: string, let mode = WithSettings(placement atTop) - let fix = provider |> tryFix code mode |> Option.get - Assert.Equal($"open {ns}", fix.Message) + let fix = provider |> fixNamed $"open {ns}" code mode assertFixedCode (withOpen atTop ns code) fix.FixedCode - Assert.Equal(None, provider |> tryFix fix.FixedCode mode) + Assert.Equal(None, provider |> tryFirstFix fix.FixedCode mode) [] [ n | _ -> 0", "Candidates", true, false, "")>] @@ -169,7 +183,7 @@ let ``Add Open applied case uses exact target and placement`` (caseName: string, [", "Patterns", false, true, "Outer")>] [ n | _ -> 0", "Closed.Patterns", true, true, "Closed")>] [ n | _ -> 0", "Closed.Patterns", false, true, "Closed")>] -let ``Add Open supports adjacent headers without changing existing spacing`` +let ``Add Open supports adjacent headers and separates the open from them`` (body: string, ns: string, atTop: bool, sameFile: bool, container: string) = let code = @@ -182,23 +196,23 @@ let ``Add Open supports adjacent headers without changing existing spacing`` let mode = WithSettings(placement atTop) - let fix = provider |> tryFix code mode |> Option.get + let fix = provider |> tryFirstFix code mode |> Option.get Assert.Equal($"open {ns}", fix.Message) let expected = if container = "Closed" then - code.Replace("module Outer =\n", $"open {ns}\n\nmodule Outer =\n") + code.Replace("module Outer =\n", $"\nopen {ns}\n\nmodule Outer =\n") elif sameFile then let margin = if container = "" then "" else " " let anchor = $"{margin}module Use =\n" - code.Replace(anchor, $"{margin}open {ns}\n\n{anchor}") + code.Replace(anchor, $"\n{margin}open {ns}\n\n{anchor}") elif atTop then - code.Replace("namespace Consumer\n", $"namespace Consumer\nopen {ns}\n\n") + code.Replace("namespace Consumer\n", $"namespace Consumer\n\nopen {ns}\n\n") else - code.Replace("module Use =\n", $"module Use =\n open {ns}\n\n") + code.Replace("module Use =\n", $"module Use =\n\n open {ns}\n\n") assertFixedCode expected fix.FixedCode - Assert.Equal(None, provider |> tryFix fix.FixedCode mode) + Assert.Equal(None, provider |> tryFirstFix fix.FixedCode mode) [] [] @@ -213,7 +227,7 @@ let ``Add Open supports a top module header without a blank line`` (atTop: bool) Assert.Equal("open System", fix.Message) - assertFixedCode "module Consumer\nopen System\n\nlet value: DateTime = Unchecked.defaultof<_>\nlet other = 1\n" fix.FixedCode + assertFixedCode "module Consumer\n\nopen System\n\nlet value: DateTime = Unchecked.defaultof<_>\nlet other = 1\n" fix.FixedCode [] [ n | _ -> 0", "Candidates.Qualified.Restricted")>] @@ -221,9 +235,8 @@ let ``Add Open supports a top module header without a blank line`` (atTop: bool) let ``RQA bare case fix supplies the qualification still required`` (body: string, message: string) = let code = source body - let fix = addOpenFix code Auto |> Option.get - - Assert.Equal(message, fix.Message) + let fix = + AddOpenCodeFixProvider(AssemblyContentProvider()) |> fixNamed message code Auto let expected = if message = "Candidates.Qualified.Restricted" then @@ -370,14 +383,13 @@ let ``completion commit applies case text and avoids duplicate opens`` elif sameFile then let margin = if container = "" then "" else " " let anchor = $"{margin}module Use =\n" - let before = if atTop || container = "" then "\n" else "" - committed.Replace(anchor, $"{before}{margin}open {ns}\n\n{anchor}") + committed.Replace(anchor, $"\n{margin}open {ns}\n\n{anchor}") elif atTop then withOpen true ns committed elif closedInner then - committed.Replace("module Use =\n", $"module Use =\n open {ns}\n\n") + committed.Replace("module Use =\n", $"module Use =\n\n open {ns}\n\n") else - committed.Replace("module Use =\n\n", $"module Use =\n open {ns}\n\n") + withOpen false ns committed assertFixedCode expected actual @@ -543,15 +555,20 @@ let ``referenced case Add Open uses the nearest physical scope without self-qual | ValueNone -> failwith "Expected automatic referenced-case diagnostic" let fix = - (provider :> IFSharpCodeFixProvider).GetCodeFixIfAppliesAsync context + (provider :> IFSharpMultiCodeFixProvider).GetCodeFixesAsync context |> CancellableTask.runSynchronouslyWithoutCancellation + |> Seq.tryHead |> function - | ValueSome fix -> fix - | ValueNone -> failwith "Expected referenced-case Add Open" + | Some fix -> fix + | None -> failwith "Expected referenced-case Add Open" Assert.Equal("open Outer.Patterns", fix.Message) let actual = text.WithChanges(fix.Changes).ToString() - Assert.Equal(code.Replace(" let classify", " open Outer.Patterns\n\n let classify"), actual.Replace("\r\n", "\n")) + + Assert.Equal( + code.Replace(" let classify", "\n open Outer.Patterns\n\n let classify"), + actual.Replace("\r\n", "\n") + ) let _, results = document.WithText(SourceText.From actual).GetFSharpParseAndCheckResultsAsync("referenced AP edit") diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenForExtensionMembersTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenForExtensionMembersTests.fs new file mode 100644 index 00000000000..c115d8f42e3 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenForExtensionMembersTests.fs @@ -0,0 +1,223 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddOpenForExtensionMembersTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework +open AddOpenOnTopSharedTests + +let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) + +let private messages code = + codeFix |> allFixes code Auto |> List.map _.Message + +let private openMessages code = + codeFix |> openFixes code Auto |> List.map _.Message + +let private shoutOnSequenceOf elementType = + $"""namespace Lib + +open System.Collections.Generic +open System.Runtime.CompilerServices + +[] +type SequenceExtensions = + [] + static member Shout(words: IEnumerable) = words |> Seq.map (fun word -> word.ToUpper()) + +namespace App + +module Use = + let loud (xs: seq<{elementType}>) = xs.Shout() +""" + +let private releaseOn receiverType = + $"""namespace Lib + +open System +open System.Runtime.CompilerServices + +[] +type DisposableExtensions = + [] + static member Release<'T when 'T :> IDisposable>(resource: 'T) = resource.Dispose() + +namespace App + +module Use = + let release (x: {receiverType}) = x.Release() +""" + +let private constrainedOn constrainedTo receiverType = + $"""namespace Lib + +open System.Runtime.CompilerServices + +[] +type ConstrainedExtensions = + [] + static member Constrained<'T when 'T : {constrainedTo}>(value: 'T) = value + +namespace App + +module Use = + let constrained (x: {receiverType}) = x.Constrained() +""" + +let extensionOpens = + scenarios + [ + ("a C# extension method called on an expression of unknown type: every namespace with one", + "module Module1\n\nlet evens () = [ 1; 2; 3 ].Where(fun x -> x % 2 = 0)\n", + [| "open System.Data"; "open System.Linq" |]) + + ("a C# extension method whose other overload extends the receiver", + "module Module1\n\nlet span (numbers: int[]) = numbers.AsSpan()\n", + [| "open System" |]) + + ("a C# extension on IEnumerable extends a sequence of strings", shoutOnSequenceOf "string", [| "open Lib" |]) + + ("a C# extension on IEnumerable does not extend a sequence of numbers", shoutOnSequenceOf "int", [||]) + + ("a C# extension on a disposable type parameter extends a stream", releaseOn "System.IO.MemoryStream", [| "open Lib" |]) + + ("a C# extension on a disposable type parameter does not extend a number", releaseOn "int", [||]) + + ("a C# extension on a struct type parameter extends a number", constrainedOn "struct" "int", [| "open Lib" |]) + + ("a C# extension on a struct type parameter does not extend a string", constrainedOn "struct" "string", [||]) + + ("a C# extension on a class type parameter extends a string", constrainedOn "not struct" "string", [| "open Lib" |]) + + ("a C# extension on a class type parameter does not extend a number", constrainedOn "not struct" "int", [||]) + + ("a C# extension on a type parameter with a default constructor extends a builder", + constrainedOn "(new: unit -> 'T)" "System.Text.StringBuilder", + [| "open Lib" |]) + + ("a C# extension on a type parameter with a default constructor does not extend a string", + constrainedOn "(new: unit -> 'T)" "string", + [||]) + + ("a C# extension on an enum type parameter does not extend a number", constrainedOn "enum" "int", [||]) + + ("a C# extension on a delegate type parameter does not extend a string", constrainedOn "delegate" "string", [||]) + + ("only the modules whose F# extension member extends the receiver", + """module Module1 + +module StringExtensions = + type System.String with + member s.Shout = s.ToUpper() + +module NumberExtensions = + type System.Int32 with + member n.Shout = string n + +let greeting = "hi" +let loud = greeting.Shout +""", + [| "open StringExtensions" |]) + + ("an F# extension member of another type", + """module Module1 + +module Extensions = + type System.String with + member s.Shout = s.ToUpper() + +let n = 42 +let loud = n.Shout +""", + [||]) + + ("a module that requires qualified access", + """module Module1 + +[] +module Extensions = + type System.String with + member s.Shout = s.ToUpper() + +let greeting = "hi" +let loud = greeting.Shout +""", + [||]) + ] + +[] +let ``Offers the opens of the extension members that apply`` (_scenario: string, code: string, expected: string[]) = + Assert.Equal(List.ofArray expected, messages code) + +let extensionOpensBesideQualifications = + scenarios + [ + ("the namespace that opens an AutoOpen module, and the module itself", + """namespace Lib + +[] +module StringExtensions = + type System.String with + member s.Shout() = s.ToUpper() + +namespace App + +module Use = + let loud (s: string) = s.Shout() +""", + [| "open Lib"; "open Lib.StringExtensions" |]) + + ("the module of an F# static extension member", + """module Module1 + +module Extensions = + type System.String with + static member Shout(s: string) = s.ToUpper() + +let loud = System.String.Shout "hi" +""", + [| "open Extensions" |]) + + ("no open for the class of a C# extension method called by name", + """module Module1 + +let evens (xs: int list) = Where(xs, fun x -> x % 2 = 0) +""", + [||]) + ] + +[] +let ``Offers the opens of the extension members, whatever else is offered`` (_scenario: string, code: string, expected: string[]) = + Assert.Equal(List.ofArray expected, openMessages code) + +[] +let ``Offers the namespace of a C# extension method called on a value`` () = + let code = + "module Module1\n\nlet evens (xs: int list) = xs.Where(fun x -> x % 2 = 0)\n" + + let expected = + [ + { + Message = "open System.Linq" + FixedCode = code.Replace("let evens", "open System.Linq\n\nlet evens") + } + ] + + Assert.Equal(expected, codeFix |> allFixes code Auto) + +[] +let ``Offers the module of an F# extension property`` () = + let code = + "module Module1\n\nmodule Extensions =\n type System.String with\n member s.Shout = s.ToUpper()\n\nlet greeting = \"hi\"\nlet loud = greeting.Shout\n" + + let expected = + [ + { + Message = "open Extensions" + FixedCode = code.Replace("let greeting", "open Extensions\n\nlet greeting") + } + ] + + Assert.Equal(expected, codeFix |> allFixes code Auto) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs index 765b1aa6210..4744594d60d 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -6,6 +6,7 @@ open Microsoft.VisualStudio.FSharp.Editor open Xunit open CodeFixTestFramework +open AddOpenOnTopSharedTests let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) @@ -158,6 +159,55 @@ let ``Fixes FS0039 for missing opens - nested module`` () = Assert.Equal(expected, actual) +let private bodyMargin = String.replicate 37 " " +let private declarationMargin = String.replicate 17 " " +let private continuationMargin = String.replicate 21 " " + +let placements = + scenarios + [ + ("nested module declared on one line", + "module Module1 = let x = Console.WriteLine 42\n", + "module Module1 =\n open System\n\n let x = Console.WriteLine 42\n") + + ("use before a nested module declared on one line", + "module Module1\nlet x = Console.WriteLine 42\n[] module Module2 = let y = 1\n", + "module Module1\n\nopen System\n\nlet x = Console.WriteLine 42\n[] module Module2 = let y = 1\n") + + ("use in the attribute of a nested module declared on one line", + "module Module1\n[] module Module2 = let y = 1\n", + "module Module1\n\nopen System\n\n[] module Module2 = let y = 1\n") + + ("use in the body of a nested module declared on one line with an attribute", + "module Module1\n[] module Module2 = let y = Console.WriteLine 42\n", + $"module Module1\n[] module Module2 =\n{bodyMargin}open System\n\n{bodyMargin}let y = Console.WriteLine 42\n") + + ("use after a block comment inside the body of a nested module declared on one line", + $"module Module1\nmodule Module2 = let y = (* body\n{continuationMargin}*) Console.WriteLine 42\n", + $"module Module1\nmodule Module2 =\n{declarationMargin}open System\n\n{declarationMargin}let y = (* body\n{continuationMargin}*) Console.WriteLine 42\n") + ] + +[] +let ``Fixes FS0039 for missing opens - placement`` (_scenario: string, code: string, fixedCode: string) = + expectOpenSystem mode code fixedCode + +[] +let ``Offers open type inside a nested module declared on one line with an attribute for a use in its body`` () = + let bodyMargin = String.replicate 31 " " + + let code = + "module Top\nmodule Original =\n let ReadLine () = 123\nopen Original\n[] module M = let x = WriteLine \"hi\"\nlet number: int = ReadLine()\n" + + let expected = + $"module Top\nmodule Original =\n let ReadLine () = 123\nopen Original\n[] module M =\n{bodyMargin}open type System.Console\n\n{bodyMargin}let x = WriteLine \"hi\"\nlet number: int = ReadLine()\n" + + let actual = + codeFix + |> openFixes code mode + |> List.find (fun fix -> fix.Message = "open type System.Console") + + Assert.Equal(expected, actual.FixedCode) + [] let ``Fixes FS0039 for missing opens - explicit module has attributes`` () = let code = diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index 2614a54a493..4569dba8d83 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -6,6 +6,7 @@ open Microsoft.VisualStudio.FSharp.Editor open Xunit open CodeFixTestFramework +open AddOpenOnTopSharedTests let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) @@ -78,6 +79,28 @@ Console.WriteLine 42 Assert.Equal(expected, actual) +let placements = + scenarios + [ + ("declaration shares its line with the end of a comment", + "(* header\n*) Console.WriteLine 42\n", + "(* header\n*)\n open System\n\n Console.WriteLine 42\n") + + ("explicit top level module without a blank line", + "module Module1\nConsole.WriteLine 42\n", + "module Module1\n\nopen System\n\nConsole.WriteLine 42\n") + + ("last line with no line break", "module Module1\nConsole.WriteLine 42", "module Module1\n\nopen System\n\nConsole.WriteLine 42") + + ("namespace without a blank line", + "namespace N1\nmodule M1 =\n Console.WriteLine 42\n", + "namespace N1\n\nopen System\n\nmodule M1 =\n Console.WriteLine 42\n") + ] + +[] +let ``Fixes FS0039 for missing opens - placement`` (_scenario: string, code: string, fixedCode: string) = + expectOpenSystem Auto code fixedCode + [] let ``Fixes FS0039 for missing opens - there is already an open directive`` () = let code = @@ -367,6 +390,92 @@ module M1 = Assert.Equal(expected, actual) +let openSuggestions = + scenarios + [ + ("a type nested in a type is opened with open type", + "let folder () = SpecialFolder.Desktop", + [| "open type System.Environment" |]) + + ("every namespace the name resolves from", + "let readFile () = File.ReadAllText \"example.txt\"", + [| "open System.IO"; "open type System.Net.WebRequestMethods" |]) + + ("every type a static member resolves from", + "let write () = WriteLine \"hi\"", + [| + "open type System.Console" + "open type System.Diagnostics.Debug" + "open type System.Diagnostics.Trace" + "open type Microsoft.VisualBasic.FileSystem" + |]) + + ("a type in a namespace is opened with a plain open", "let write () = Console.WriteLine \"hi\"", [| "open System" |]) + + ("a generic type is not opened", "let keys (collection: KeyCollection) = collection", [||]) + ] + +[] +let ``Offers the opens that resolve the name`` (_scenario: string, usage: string, declarations: string[]) = + let expected = + [ + for declaration in declarations do + { + Message = declaration + FixedCode = $"module Module1\n\n{declaration}\n\n{usage}\n" + } + ] + + let actual = codeFix |> openFixes $"module Module1\n\n{usage}\n" Auto + + Assert.Equal(expected, actual) + +let suggestionMessages = + scenarios + [ + ("the qualifications follow the opens", + "let readFile () = File.ReadAllText \"example.txt\"", + [| + "open System.IO" + "open type System.Net.WebRequestMethods" + "System.IO.File" + "System.IO.File.ReadAllText" + "System.Net.WebRequestMethods.File" + |]) + + ("the System qualifications come first", + "let write () = WriteLine \"hi\"", + [| + "open type System.Console" + "open type System.Diagnostics.Debug" + "open type System.Diagnostics.Trace" + "open type Microsoft.VisualBasic.FileSystem" + "System.Console.WriteLine" + "System.Diagnostics.Debug.WriteLine" + "System.Diagnostics.Trace.WriteLine" + |]) + ] + +[] +let ``Lists the suggestions in the order of the lightbulb`` (_scenario: string, usage: string, messages: string[]) = + let actual = + codeFix |> allFixes $"module Module1\n\n{usage}\n" Auto |> List.map _.Message + + Assert.Equal(List.ofArray messages, actual) + +[] +let ``Qualifying a partly qualified name replaces what is written before the unresolved part`` () = + let code = + "module Review\nmodule M = let marker = ()\nmodule N =\n module M =\n type FixTarget817() = class end\nlet x = M.FixTarget817()\n" + + let actual = + codeFix + |> allFixes code Auto + |> List.filter (fun fix -> fix.Message = "N.M.FixTarget817") + |> List.map _.FixedCode + + Assert.Equal([ code.Replace("x = M.", "x = N.M.") ], actual) + [] let ``Doesn't fix FS0039 for random undefined symbols`` () = let code = diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopSharedTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopSharedTests.fs new file mode 100644 index 00000000000..16153fcb362 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopSharedTests.fs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddOpenOnTopSharedTests + +open System + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) + +let scenarios (cases: ('name * 'code * 'expected) list) : obj[] seq = + cases + |> Seq.map (fun (name, code, expected) -> [| box name; box code; box expected |]) + +let internal allFixes code mode (provider: AddOpenCodeFixProvider) = + provider |> multiFix code mode |> Seq.toList + +let internal openFixes code mode provider = + allFixes code mode provider + |> List.filter (fun fix -> fix.Message.StartsWith("open ", StringComparison.Ordinal)) + +let internal tryFix code mode provider = + match openFixes code mode provider with + | [] -> None + | [ fix ] -> Some fix + | fixes -> failwith $"""Expected at most one open, got {String.Join("; ", fixes |> List.map _.Message)}""" + +let expectOpenSystem mode code fixedCode = + let expected = + Some + { + Message = "open System" + FixedCode = fixedCode + } + + Assert.Equal(expected, codeFix |> tryFix code mode) + +let modeOf alwaysPlaceOpensAtTopLevel = + WithSettings + { CodeFixesOptions.Default with + AlwaysPlaceOpensAtTopLevel = alwaysPlaceOpensAtTopLevel + } + +let expectOpenSystemInBothModes code fixedCode = + for alwaysPlaceOpensAtTopLevel in [ true; false ] do + expectOpenSystem (modeOf alwaysPlaceOpensAtTopLevel) code fixedCode + +[] +[] +[] +let ``Fixes FS0039 for missing opens - module has an attribute on the same line`` (alwaysPlaceOpensAtTopLevel: bool) = + expectOpenSystem + (modeOf alwaysPlaceOpensAtTopLevel) + "[] module Module1\n\nConsole.WriteLine 42\n" + "[] module Module1\n\nopen System\n\nConsole.WriteLine 42\n" + +[] +let ``Fixes FS0039 for missing opens - block comment runs on from the module header`` () = + expectOpenSystemInBothModes + "module Module1 (* header\n*)\nConsole.WriteLine 42\n" + "module Module1 (* header\n*)\n\nopen System\n\nConsole.WriteLine 42\n" + +[] +let ``Fixes FS0039 for missing opens - block comment runs on from the namespace header`` () = + expectOpenSystemInBothModes + "namespace Ns (* header\n*)\ntype T() =\n member _.M() = Console.WriteLine 42\n" + "namespace Ns (* header\n*)\n\nopen System\n\ntype T() =\n member _.M() = Console.WriteLine 42\n" + +[] +let ``Fixes FS0039 for missing opens - block comment ending the header shares its line with the first declaration`` () = + expectOpenSystemInBothModes + "module Module1 (* header\n*) let x = Console.WriteLine 42\n" + "module Module1 (* header\n*)\n open System\n\n let x = Console.WriteLine 42\n" diff --git a/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs index 68048314673..81522214d5e 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs @@ -149,6 +149,61 @@ module CompletionProviderTests = Assert.Equal(expected, sourceText.ToString(resultSpan)) + let VerifyCompletionWithOpen (fileContents: string, marker: string, itemName: string, expected: string) = + let caretPosition = fileContents.IndexOf(marker) + marker.Length + let sourceText = SourceText.From(fileContents) + + let document = + RoslynTestHelpers.CreateSolution(fileContents) + |> RoslynTestHelpers.GetSingleDocument + + let span = + CompletionUtils.getDefaultCompletionListSpan ( + sourceText, + caretPosition, + document.Id, + filePath, + [], + None, + CancellationToken.None + ) + + let context = + let provider = + { new CompletionProvider() with + member _.ProvideCompletionsAsync _ = Tasks.Task.CompletedTask + } + + CompletionContext(provider, document, caretPosition, span, CompletionTrigger.Invoke, null, CancellationToken.None) + + let change = + cancellableTask { + let! items = + FSharpCompletionProvider.ProvideCompletionsAsyncAux( + document, + caretPosition, + AssemblyContentProvider().GetAllEntitiesInProjectAndReferencedAssemblies, + false + ) + + let item = + items + |> Seq.find (fun item -> item.DisplayText = itemName && item.Properties.ContainsKey "NamespaceToOpen") + + // CompletionItem.WithSpan is obsolete: the context hands the item its span. + context.AddItem item + + return! + FSharpCompletionProvider.GetChangeAsyncAux( + document, + item, + FSharp.Compiler.EditorServices.OpenStatementInsertionPoint.Nearest + ) + } + |> CancellableTask.start CancellationToken.None + + Assert.Equal(expected, sourceText.WithChanges(change.Result.TextChange).ToString()) + [] let ShouldTriggerCompletionAtCorrectMarkers () = let testCases = @@ -2174,3 +2229,32 @@ let _ = ] |> Map.ofList ) + + [] + let ``Completion that adds an open keeps it out of a nested module declared on one line`` () = + let fileContents = + """module Top +let x = Console.WriteLine 42 +[] module M = let y = 1 +""" + + let expected = + """module Top + +open System + +let x = Console.WriteLine 42 +[] module M = let y = 1 +""" + + VerifyCompletionWithOpen(fileContents, "[] + let ``Completion that adds an open goes into a nested module declared on one line with an attribute for a use in its body`` () = + let fileContents = "module Top\n[] module M = let y = Conso\n" + let margin = String.replicate 24 " " + + let expected = + $"module Top\n[] module M =\n{margin}open System\n\n{margin}let y = Console\n" + + VerifyCompletionWithOpen(fileContents, "let y = Conso", "Console", expected) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index d5d1a09c79a..939f332049d 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -66,8 +66,10 @@ + +