Open statements can be optionally always added at top level (#2875)

* open statements can be optionally always added at top level

* fixed: existing open statements are not taken into account at top level

* Update EditorOptions.fs

src/fsharp/vs/ServiceAssemblyContent.fs

@@ -15,14 +15,14 @@ open Microsoft.FSharp.Compiler.Ast
 open Microsoft.FSharp.Compiler.Range
 open Microsoft.FSharp.Compiler.AbstractIL.Internal.Library 
 
-type internal ShortIdent = string
+type ShortIdent = string
 type Idents = ShortIdent[]
 type MaybeUnresolvedIdent = { Ident: ShortIdent; Resolved: bool }
 type MaybeUnresolvedIdents = MaybeUnresolvedIdent[]
 type IsAutoOpen = bool
 
 [<AutoOpen>]
-module internal Extensions =
+module Extensions =
 
     type FSharpEntity with
         member x.TryGetFullName() =
@@ -98,7 +98,7 @@ module internal Extensions =
         member x.TryGetEntities() = try x.Entities :> _ seq with _ -> Seq.empty
 
 [<AutoOpen>]
-module internal Utils =
+module Utils =
     let isAttribute<'T> (attribute: FSharpAttribute) =
         // CompiledName throws exception on DataContractAttribute generated by SQLProvider
         match (try Some attribute.AttributeType.CompiledName with _ -> None) with
@@ -129,12 +129,12 @@ module internal Utils =
         |> Option.isSome
 
 [<RequireQualifiedAccess>]
-type internal LookupType =
+type LookupType =
     | Fuzzy
     | Precise
 
 [<NoComparison; NoEquality>]
-type internal AssemblySymbol = 
+type AssemblySymbol = 
     { FullName: string
       CleanedIdents: Idents
       Namespace: Idents option
@@ -148,7 +148,7 @@ type internal AssemblySymbol =
 type AssemblyPath = string
 type AssemblyContentType = Public | Full
 
-type internal Parent = 
+type Parent = 
     { Namespace: Idents option
       ThisRequiresQualifiedAccess: Idents option
       TopRequiresQualifiedAccess: Idents option
@@ -202,7 +202,7 @@ type internal Parent =
                 |> removeGenericParamsCount 
                 |> removeModuleSuffix))
 
-module internal TypedAstPatterns =
+module TypedAstPatterns =
     let (|TypeWithDefinition|_|) (ty: FSharpType) =
         if ty.HasTypeDefinition then Some ty.TypeDefinition
         else None
@@ -214,17 +214,17 @@ module internal TypedAstPatterns =
 
     let (|FSharpModule|_|) (entity: FSharpEntity) = if entity.IsFSharpModule then Some() else None
 
-type internal AssemblyContentCacheEntry =
+type AssemblyContentCacheEntry =
     { FileWriteTime: DateTime 
       ContentType: AssemblyContentType 
       Symbols: AssemblySymbol list }
 
 [<NoComparison; NoEquality>]
-type internal IAssemblyContentCache =
+type IAssemblyContentCache =
     abstract TryGet: AssemblyPath -> AssemblyContentCacheEntry option
     abstract Set: AssemblyPath -> AssemblyContentCacheEntry -> unit
 
-module internal AssemblyContentProvider =
+module AssemblyContentProvider =
     open System.IO
 
     let private createEntity ns (parent: Parent) (entity: FSharpEntity) =
@@ -380,7 +380,7 @@ module internal AssemblyContentProvider =
                 | Some x when x.IsPublic -> true
                 | _ -> false)
 
-type internal EntityCache() =
+type EntityCache() =
     let dic = Dictionary<AssemblyPath, AssemblyContentCacheEntry>()
     interface IAssemblyContentCache with
         member __.TryGet assembly =
@@ -392,9 +392,9 @@ type internal EntityCache() =
     member __.Clear() = dic.Clear()
     member x.Locking f = lock dic <| fun _ -> f (x :> IAssemblyContentCache)
 
-type internal LongIdent = string
+type LongIdent = string
 
-type internal Entity =
+type Entity =
     { FullRelativeName: LongIdent
       Qualifier: LongIdent
       Namespace: LongIdent option
@@ -403,7 +403,7 @@ type internal Entity =
     override x.ToString() = sprintf "%A" x
 
 [<CompilationRepresentation (CompilationRepresentationFlags.ModuleSuffix)>]
-module internal Entity =
+module Entity =
     let getRelativeNamespace (targetNs: Idents) (sourceNs: Idents) =
         let rec loop index =
             if index > targetNs.Length - 1 then sourceNs.[index..]
@@ -477,7 +477,7 @@ module internal Entity =
                             Name = match restIdents with [|_|] -> "" | _ -> String.concat "." restIdents 
                             LastIdent = Array.tryLast restIdents |> Option.defaultValue "" }) 
 
-type internal ScopeKind =
+type ScopeKind =
     | Namespace
     | TopModule
     | NestedModule
@@ -485,16 +485,22 @@ type internal ScopeKind =
     | HashDirective
     override x.ToString() = sprintf "%A" x
 
-type internal InsertContext =
+type InsertContext =
     { ScopeKind: ScopeKind
       Pos: pos }
 
-module internal ParsedInput =
+type Module =
+    { Idents: Idents
+      Range: range }
+
+type OpenStatementInsertionPoint =
+    | TopLevel
+    | Nearest
+
+module ParsedInput =
     open Microsoft.FSharp.Compiler
     open Microsoft.FSharp.Compiler.Ast
 
-    type private EndLine = int
-
     /// An recursive pattern that collect all sequential expressions to avoid StackOverflowException
     let rec (|Sequentials|_|) = function
         | SynExpr.Sequential(_, _, e, Sequentials es, _) ->
@@ -508,7 +514,7 @@ module internal ParsedInput =
         | SynConstructorArgs.NamePatPairs(xs, _) -> List.map snd xs
 
     /// Returns all `Ident`s and `LongIdent`s found in an untyped AST.
-    let internal getLongIdents (input: ParsedInput option) : IDictionary<Range.pos, LongIdent> =
+    let getLongIdents (input: ParsedInput option) : IDictionary<Range.pos, LongIdent> =
         let identsByEndPos = Dictionary<Range.pos, LongIdent>()
     
         let addLongIdent (longIdent: LongIdent) =
@@ -850,33 +856,36 @@ module internal ParsedInput =
         | true, idents -> Some idents
         | _ -> None
 
-    type Col = int
-
     type Scope =
         { Idents: Idents
           Kind: ScopeKind }
 
-    let tryFindNearestPointAndModules (currentLine: int) (ast: ParsedInput) = 
+    let tryFindNearestPointAndModules (currentLine: int) (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 ErrorScope()  
 
-        let result: (Scope * pos) option ref = ref None
+        let result: (Scope * pos * (* finished *) bool) option ref = ref None
         let ns: string[] option ref = ref None
-        let modules = ResizeArray<Idents * EndLine * Col>()  
+        let modules = ResizeArray<Module>()  
 
         let inline longIdentToIdents ident = ident |> Seq.map (fun x -> string x) |> Seq.toArray
         
-        let addModule (longIdent: LongIdent) endLine col =
-            modules.Add(longIdent |> List.map string |> List.toArray, endLine, col)
+        let addModule (longIdent: LongIdent, range: range) =
+            modules.Add 
+                { Idents = longIdent |> List.map string |> List.toArray 
+                  Range = range }
 
         let doRange kind (scope: LongIdent) line col =
             if line <= currentLine then
-                match !result with
-                | None -> 
-                    result := Some ({ Idents = longIdentToIdents scope; Kind = kind }, mkPos line col)
-                | Some (oldScope, oldPos) ->
+                match !result, insertionPoint with
+                | None, _ -> 
+                    result := Some ({ Idents = longIdentToIdents scope; Kind = kind }, mkPos line col, false)
+                | Some (_, _, true), _ -> ()
+                | Some (oldScope, oldPos, false), OpenStatementInsertionPoint.TopLevel when kind <> OpenDeclaration ->
+                    result := Some (oldScope, oldPos, true)
+                | Some (oldScope, oldPos, _), _ ->
                     match kind, oldScope.Kind with
                     | (Namespace | NestedModule | TopModule), OpenDeclaration
                     | _ when oldPos.Line <= line ->
@@ -886,7 +895,8 @@ module internal ParsedInput =
                                         | [] -> oldScope.Idents 
                                         | _ -> longIdentToIdents scope
                                     Kind = kind },
-                                  mkPos line col)
+                                  mkPos line col,
+                                  false)
                     | _ -> ()
 
         let getMinColumn (decls: SynModuleDecls) =
@@ -931,7 +941,7 @@ module internal ParsedInput =
                     | _ -> Namespace
 
                 doRange scopeKind fullIdent startLine range.StartColumn
-                addModule fullIdent range.EndLine range.StartColumn
+                addModule (fullIdent, range)
                 List.iter (walkSynModuleDecl fullIdent) decls
 
         and walkSynModuleDecl (parent: LongIdent) (decl: SynModuleDecl) =
@@ -939,7 +949,7 @@ module internal ParsedInput =
             | SynModuleDecl.NamespaceFragment fragment -> walkSynModuleOrNamespace parent fragment
             | SynModuleDecl.NestedModule(ComponentInfo(_, _, _, ident, _, _, _, _), _, decls, _, range) ->
                 let fullIdent = parent @ ident
-                addModule fullIdent range.EndLine range.StartColumn
+                addModule (fullIdent, range)
                 if range.EndLine >= currentLine then
                     let moduleBodyIdentation = getMinColumn decls |> Option.defaultValue (range.StartColumn + 4)
                     doRange NestedModule fullIdent range.StartLine moduleBodyIdentation
@@ -954,31 +964,32 @@ module internal ParsedInput =
 
         let res =
             !result
-            |> Option.map (fun (scope, pos) ->
+            |> Option.map (fun (scope, pos, _) ->
                 let ns = !ns |> Option.map longIdentToIdents
                 scope, ns, mkPos (pos.Line + 1) pos.Column)
         
         let modules = 
             modules 
-            |> Seq.filter (fun (_, endLine, _) -> endLine < currentLine)
-            |> Seq.sortBy (fun (m, _, _) -> -m.Length)
+            |> Seq.filter (fun x -> x.Range.EndLine < currentLine)
+            |> Seq.sortBy (fun x -> -x.Idents.Length)
             |> Seq.toList
 
         res, modules
 
-    let findBestPositionToInsertOpenDeclaration (modules: (Idents * EndLine * Col) list) scope pos (entity: Idents) =
-        match modules |> List.filter (fun (m, _, _) -> entity |> Array.startsWith m ) with
+    let findBestPositionToInsertOpenDeclaration (modules: Module list) scope pos (entity: Idents) =
+        match modules |> List.filter (fun x -> entity |> Array.startsWith x.Idents) with
         | [] -> { ScopeKind = scope.Kind; Pos = pos }
-        | (_, endLine, startCol) :: _ ->
+        | m :: _ ->
             //printfn "All modules: %A, Win module: %A" modules m
             let scopeKind =
                 match scope.Kind with
                 | TopModule -> NestedModule
                 | x -> x
-            { ScopeKind = scopeKind; Pos = mkPos (Line.fromZ endLine) startCol }
+            { ScopeKind = scopeKind
+              Pos = mkPos (Line.fromZ m.Range.EndLine) m.Range.StartColumn }
 
-    let tryFindInsertionContext (currentLine: int) (ast: ParsedInput) (partiallyQualifiedName: MaybeUnresolvedIdents) = 
-        let res, modules = tryFindNearestPointAndModules currentLine ast
+    let tryFindInsertionContext (currentLine: int) (ast: ParsedInput) (partiallyQualifiedName: MaybeUnresolvedIdents) (insertionPoint: OpenStatementInsertionPoint) = 
+        let res, modules = tryFindNearestPointAndModules currentLine ast insertionPoint
         // CLEANUP: does this really need to be a partial application with pre-computation?  Can this be made more explicit?
         fun (requiresQualifiedAccessParent: Idents option, autoOpenParent: Idents option, entityNamespace: Idents option, entity: Idents) ->
 
@@ -1020,8 +1031,8 @@ module internal ParsedInput =
 
         mkPos line ctx.Pos.Column
     
-    let tryFindNearestPointToInsertOpenDeclaration (currentLine: int) (ast: ParsedInput) (entity: Idents) =
-        match tryFindNearestPointAndModules currentLine ast with
+    let tryFindNearestPointToInsertOpenDeclaration (currentLine: int) (ast: ParsedInput) (entity: Idents) (insertionPoint: OpenStatementInsertionPoint) =
+        match tryFindNearestPointAndModules currentLine ast insertionPoint with
         | Some (scope, _, point), modules -> 
             Some (findBestPositionToInsertOpenDeclaration modules scope point entity)
         | _ -> None
\ No newline at end of file

src/fsharp/vs/ServiceAssemblyContent.fsi

@@ -128,13 +128,23 @@ type internal InsertContext =
       /// Current position (F# compiler line number).
       Pos: pos }
 
+/// Where open statements should be added.
+type internal OpenStatementInsertionPoint =
+    | TopLevel
+    | Nearest
+
 /// Parse AST helpers.
 module internal ParsedInput =
+
     /// Returns `InsertContext` based on current position and symbol idents.
-    val tryFindInsertionContext : currentLine: int -> ast: Ast.ParsedInput -> MaybeUnresolvedIdents -> (( (* requiresQualifiedAccessParent: *) Idents option * (* autoOpenParent: *) Idents option * (*  entityNamespace *) Idents option * (* entity: *) Idents) -> (Entity * InsertContext)[])
+    val tryFindInsertionContext : 
+        currentLine: int -> 
+        ast: Ast.ParsedInput -> MaybeUnresolvedIdents -> 
+        insertionPoint: OpenStatementInsertionPoint ->
+        (( (* requiresQualifiedAccessParent: *) Idents option * (* autoOpenParent: *) Idents option * (*  entityNamespace *) Idents option * (* entity: *) Idents) -> (Entity * InsertContext)[])
     
     /// Returns `InsertContext` based on current position and symbol idents.
-    val tryFindNearestPointToInsertOpenDeclaration : currentLine: int -> ast: Ast.ParsedInput -> entity: Idents -> InsertContext option
+    val tryFindNearestPointToInsertOpenDeclaration : currentLine: int -> ast: Ast.ParsedInput -> entity: Idents -> insertionPoint: OpenStatementInsertionPoint -> InsertContext option
 
     /// Returns lond identifier at position.
     val getLongIdentAt : ast: Ast.ParsedInput -> pos: Range.pos -> Ast.LongIdent option

vsintegration/src/FSharp.Editor/CodeFix/AddOpenCodeFixProvider.fs

@@ -141,7 +141,11 @@ type internal FSharpAddOpenCodeFixProvider
                           Resolved = not (ident.idRange = unresolvedIdentRange)})
                     |> List.toArray)
                                                     
-            let createEntity = ParsedInput.tryFindInsertionContext unresolvedIdentRange.StartLine parsedInput maybeUnresolvedIdents
+            let insertionPoint = 
+                if Settings.CodeFixes.AlwaysPlaceOpensAtTopLevel then OpenStatementInsertionPoint.TopLevel
+                else OpenStatementInsertionPoint.Nearest
+
+            let createEntity = ParsedInput.tryFindInsertionContext unresolvedIdentRange.StartLine parsedInput maybeUnresolvedIdents insertionPoint
             return entities |> Seq.map createEntity |> Seq.concat |> Seq.toList |> getSuggestions context
         } 
         |> Async.Ignore 

vsintegration/src/FSharp.Editor/Completion/CompletionProvider.fs

@@ -270,7 +270,12 @@ type internal FSharpCompletionProvider
                     let! options = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document)
                     let! parsedInput = checkerProvider.Checker.ParseDocument(document, options)
                     let fullNameIdents = fullName |> Option.map (fun x -> x.Split '.') |> Option.defaultValue [||]
-                    let! ctx = ParsedInput.tryFindNearestPointToInsertOpenDeclaration line.LineNumber parsedInput fullNameIdents
+                    
+                    let insertionPoint = 
+                        if Settings.CodeFixes.AlwaysPlaceOpensAtTopLevel then OpenStatementInsertionPoint.TopLevel
+                        else OpenStatementInsertionPoint.Nearest
+
+                    let! ctx = ParsedInput.tryFindNearestPointToInsertOpenDeclaration line.LineNumber parsedInput fullNameIdents insertionPoint
                     let finalSourceText, changedSpanStartPos = OpenDeclarationHelper.insertOpenDeclaration textWithItemCommitted ctx ns
                     let fullChangingSpan = TextSpan.FromBounds(changedSpanStartPos, item.Span.End)
                     let changedSpan = TextSpan.FromBounds(changedSpanStartPos, item.Span.End + (finalSourceText.Length - sourceText.Length))

vsintegration/src/FSharp.Editor/Options/EditorOptions.fs

-namespace Microsoft.VisualStudio.FSharp.Editor
+namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System.ComponentModel.Composition
 open System.Runtime.InteropServices
@@ -16,31 +16,33 @@ type IntelliSenseOptions =
 [<RequireQualifiedAccess>]
 type QuickInfoUnderlineStyle = Dot | Dash | Solid
 
-// autoproperties can be used to both define defaults and faciliate data binding in WPF controls,
-// but the type should otherwise be treated as immutable.
-type QuickInfoOptions() =
-    member val DisplayLinks = true with get, set
-    member val UnderlineStyle = QuickInfoUnderlineStyle.Solid with get, set
+[<CLIMutable>]
+type QuickInfoOptions =
+    { DisplayLinks: bool
+      UnderlineStyle: QuickInfoUnderlineStyle }
 
 [<CLIMutable>]
 type CodeFixesOptions =
     { SimplifyName: bool
+      AlwaysPlaceOpensAtTopLevel: bool
       UnusedOpens: bool }
 
 [<Export(typeof<ISettings>)>]
 type internal Settings [<ImportingConstructor>](store: SettingsStore) =
     do  // Initialize default settings
         
-        { ShowAfterCharIsTyped = true
-          ShowAfterCharIsDeleted = false }
-        |> store.RegisterDefault
+        store.RegisterDefault
+            { ShowAfterCharIsTyped = true
+              ShowAfterCharIsDeleted = false }
 
-        QuickInfoOptions() 
-        |> store.RegisterDefault
+        store.RegisterDefault
+            { DisplayLinks = true
+              UnderlineStyle = QuickInfoUnderlineStyle.Solid }
 
-        { SimplifyName = true
-          UnusedOpens = true }
-        |> store.RegisterDefault
+        store.RegisterDefault
+            { SimplifyName = true 
+              AlwaysPlaceOpensAtTopLevel = false
+              UnusedOpens = true }
 
     interface ISettings
 
@@ -74,4 +76,4 @@ module internal OptionsUI =
     type internal CodeFixesOptionPage() =
         inherit AbstractOptionPage<CodeFixesOptions>()
         override this.CreateView() =
-            upcast CodeFixesOptionControl()            
\ No newline at end of file
+            upcast CodeFixesOptionControl()            

vsintegration/src/FSharp.UIResources/CodeFixesOptionControl.xaml

-<UserControl x:Class="Microsoft.VisualStudio.FSharp.UIResources.CodeFixesOptionControl"
+<UserControl x:Class="Microsoft.VisualStudio.FSharp.UIResources.CodeFixesOptionControl"
              x:ClassModifier="internal"
              xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
              xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
@@ -21,6 +21,9 @@
                     <StackPanel>
                         <CheckBox x:Name="simplifyName" IsChecked="{Binding SimplifyName}"
                                   Content="{x:Static local:Strings.Simplify_name_code_fix}"/>
+                        <CheckBox x:Name="alwaysPlaceOpensAtTopLevel" IsChecked="{Binding AlwaysPlaceOpensAtTopLevel}"
+                            Content="{x:Static local:Strings.Always_place_opens_at_top_level}"/>
+                        <StackPanel Margin="15 0 0 0"/>
                         <CheckBox x:Name="unusedOpens" IsChecked="{Binding UnusedOpens}"
                                   Content="{x:Static local:Strings.Unused_opens_code_fix}"/>
                     </StackPanel>

vsintegration/src/FSharp.UIResources/Strings.Designer.cs

@@ -60,6 +60,15 @@ public class Strings {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Always place open statements at the top level.
+        /// </summary>
+        public static string Always_place_opens_at_top_level {
+            get {
+                return ResourceManager.GetString("Always_place_opens_at_top_level", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Code Fixes.
         /// </summary>

vsintegration/src/FSharp.UIResources/Strings.resx

@@ -117,6 +117,9 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="Always_place_opens_at_top_level" xml:space="preserve">
+    <value>Always place open statements at the top level</value>
+  </data>
   <data name="Code_Fixes" xml:space="preserve">
     <value>Code Fixes</value>
   </data>