Skip to content

Commit 8d73cb4

Browse files
xperiandriclaude
andcommitted
Offer every open that resolves the name, not the first one found
The fix computed a suggestion for every namespace and type a name could be resolved from, and then `Seq.tryHead` threw all but one away — whichever one the assembly crawler happened to reach first. An unresolved `File` was answered with `System.Net.WebRequestMethods` and never with `System.IO`; an unresolved `WriteLine` with `System.Diagnostics.Trace` and never with `System.Console`. `IFSharpMultiCodeFixProvider` already exists for providers with more than one suggestion, so the provider implements that instead and registers them all. Following Roslyn's add-import fix, which faces the same problem: the list is capped, at five opens and three qualifications, because a lightbulb is a menu somebody reads, and it is ordered with what `System` holds first rather than by whatever order the crawler walked the assemblies in -- without that, `Microsoft.VisualBasic.FileSystem` sorts above `System.Console` for `WriteLine`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 80554c3 commit 8d73cb4

4 files changed

Lines changed: 294 additions & 127 deletions

File tree

‎docs/release-notes/.VisualStudio/18.vNext.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
### Fixed
77

8+
* 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.
9+
810
* 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`.
911

1012
* 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 (`[<AutoOpen>] 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.

‎vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Microsoft.VisualStudio.FSharp.Editor
44

5+
open System
56
open System.Composition
67
open System.Collections.Immutable
78

@@ -18,6 +19,22 @@ open CancellableTasks
1819
type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentProvider: AssemblyContentProvider) =
1920
inherit CodeFixProvider()
2021

22+
// A name can be reachable from a great many places, and the lightbulb is a menu a person reads.
23+
// The same reason Roslyn's add-import fix stops at five suggestions and its fully-qualify at three.
24+
let maxOpenSuggestions = 5
25+
let maxQualifySuggestions = 3
26+
27+
// Which assembly the entity crawler reached first is no order to offer suggestions in. Sort them
28+
// the way Roslyn's add-import fix does: what `System` holds first, the rest alphabetically after.
29+
let suggestionOrder (declaration: string) =
30+
let opened = declaration.Substring(declaration.LastIndexOf ' ' + 1)
31+
32+
let isSystem =
33+
opened.Equals("System", StringComparison.Ordinal)
34+
|| opened.StartsWith("System.", StringComparison.Ordinal)
35+
36+
(if isSystem then 0 else 1), declaration
37+
2138
let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__")
2239

2340
let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) =
@@ -65,28 +82,31 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentPr
6582
|> Seq.distinctBy (fun (name, _) -> name)
6683
|> Seq.sortBy fst
6784
|> Seq.toArray)
85+
|> Seq.sortBy (fst >> suggestionOrder)
6886
|> Seq.map (fun (declaration, names) ->
6987
let multipleNames = names |> Array.length > 1
7088
names |> Seq.map (fun (name, ctx) -> declaration, name, ctx, multipleNames))
7189
|> Seq.concat
90+
|> Seq.truncate maxOpenSuggestions
7291
|> Seq.map (fun (declaration, name, ctx, multipleNames) -> openNamespaceFix ctx name declaration multipleNames sourceText)
7392

7493
candidates
7594
|> 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.
7695
|> Seq.map (fun (entity, _, _) -> entity.FullRelativeName, entity.Qualifier)
7796
|> Seq.distinct
7897
|> Seq.sort
98+
|> Seq.truncate maxQualifySuggestions
7999
|> Seq.map (qualifySymbolFix context)
80100

81101
}
82102
|> Seq.concat
83103

84104
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039", "FS0043")
85105

86-
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
106+
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFixes this
87107

88-
interface IFSharpCodeFixProvider with
89-
member _.GetCodeFixIfAppliesAsync context =
108+
interface IFSharpMultiCodeFixProvider with
109+
member _.GetCodeFixesAsync context =
90110
cancellableTask {
91111
let document = context.Document
92112

@@ -163,7 +183,7 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentPr
163183
|])
164184

165185
ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End
166-
|> Option.bind (fun longIdent ->
186+
|> Option.map (fun longIdent ->
167187
let maybeUnresolvedIdents =
168188
longIdent
169189
|> List.map (fun ident ->
@@ -191,8 +211,7 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentPr
191211
createEntity symbol
192212
|> Seq.map (fun (entity, ctx) -> entity, ctx, openableIdentCount))
193213
|> Seq.toList
194-
|> getSuggestionsAsCodeFixes context sourceText
195-
|> Seq.tryHead))
214+
|> getSuggestionsAsCodeFixes context sourceText))
196215

197-
|> ValueOption.ofOption
216+
|> Option.defaultValue Seq.empty
198217
}

0 commit comments

Comments
 (0)