Expose more services (#8291)

Krzysztof-Cieslak · cartermp · cartermp · commit 1a695c9d3ec9 · 2020-01-21T15:56:04.000-08:00
* Expose simplify name analyzer

* Move IsPrivateToFile to the FSharpSymbolUse from extensions

* Expose Unused Declarations analyzer

* Update src/fsharp/symbols/Symbols.fsi

Co-Authored-By: Phillip Carter &lt;pcarter@fastmail.com&gt;

* Apply feedback

Co-authored-by: Phillip Carter &lt;pcarter@fastmail.com&gt;
diff --git a/src/fsharp/service/ServiceAnalysis.fs b/src/fsharp/service/ServiceAnalysis.fs
@@ -226,4 +226,115 @@ module UnusedOpens =
             let symbolUses = splitSymbolUses symbolUses
             let openStatements = getOpenStatements checkFileResults.OpenDeclarations
             return! filterOpenStatements symbolUses openStatements
+        }
+ 
+module SimplifyNames = 
+    type SimplifiableRange = {
+        Range: range
+        RelativeName: string
+    }
+
+    let getPlidLength (plid: string list) = (plid |> List.sumBy String.length) + plid.Length    
+
+    let getSimplifiableNames (checkFileResults: FSharpCheckFileResults, getSourceLineStr: int -> string) : Async<SimplifiableRange list> =
+        async {
+            let result = ResizeArray()
+            let! symbolUses = checkFileResults.GetAllUsesOfAllSymbolsInFile()
+            let symbolUses =
+                symbolUses
+                |> Array.filter (fun symbolUse -> not symbolUse.IsFromOpenStatement)
+                |> Array.Parallel.map (fun symbolUse ->
+                    let lineStr = getSourceLineStr symbolUse.RangeAlternate.StartLine
+                    // for `System.DateTime.Now` it returns ([|"System"; "DateTime"|], "Now")
+                    let partialName = QuickParse.GetPartialLongNameEx(lineStr, symbolUse.RangeAlternate.EndColumn - 1)
+                    // `symbolUse.RangeAlternate.Start` does not point to the start of plid, it points to start of `name`,
+                    // so we have to calculate plid's start ourselves.
+                    let plidStartCol = symbolUse.RangeAlternate.EndColumn - partialName.PartialIdent.Length - (getPlidLength partialName.QualifyingIdents)
+                    symbolUse, partialName.QualifyingIdents, plidStartCol, partialName.PartialIdent)
+                |> Array.filter (fun (_, plid, _, name) -> name <> "" && not (List.isEmpty plid))
+                |> Array.groupBy (fun (symbolUse, _, plidStartCol, _) -> symbolUse.RangeAlternate.StartLine, plidStartCol)
+                |> Array.map (fun (_, xs) -> xs |> Array.maxBy (fun (symbolUse, _, _, _) -> symbolUse.RangeAlternate.EndColumn))
+
+            for symbolUse, plid, plidStartCol, name in symbolUses do
+                if not symbolUse.IsFromDefinition then
+                    let posAtStartOfName =
+                        let r = symbolUse.RangeAlternate
+                        if r.StartLine = r.EndLine then Range.mkPos r.StartLine (r.EndColumn - name.Length)
+                        else r.Start   
+
+                    let getNecessaryPlid (plid: string list) : Async<string list> =
+                        let rec loop (rest: string list) (current: string list) =
+                            async {
+                                match rest with
+                                | [] -> return current
+                                | headIdent :: restPlid ->
+                                    let! res = checkFileResults.IsRelativeNameResolvableFromSymbol(posAtStartOfName, current, symbolUse.Symbol)
+                                    if res then return current
+                                    else return! loop restPlid (headIdent :: current)
+                            }
+                        loop (List.rev plid) []
+                    
+                    let! necessaryPlid = getNecessaryPlid plid 
+                    
+                    match necessaryPlid with
+                    | necessaryPlid when necessaryPlid = plid -> ()
+                    | necessaryPlid ->
+                        let r = symbolUse.RangeAlternate
+                        let necessaryPlidStartCol = r.EndColumn - name.Length - (getPlidLength necessaryPlid)
+                    
+                        let unnecessaryRange = 
+                            Range.mkRange r.FileName (Range.mkPos r.StartLine plidStartCol) (Range.mkPos r.EndLine necessaryPlidStartCol)
+                    
+                        let relativeName = (String.concat "." plid) + "." + name
+                        result.Add({Range = unnecessaryRange; RelativeName = relativeName})
+
+            return List.ofSeq result
+        }
+
+module UnusedDeclarations = 
+    let isPotentiallyUnusedDeclaration (symbol: FSharpSymbol) : bool =
+        match symbol with
+        // Determining that a record, DU or module is used anywhere requires inspecting all their enclosed entities (fields, cases and func / vals)
+        // for usages, which is too expensive to do. Hence we never gray them out.
+        | :? FSharpEntity as e when e.IsFSharpRecord || e.IsFSharpUnion || e.IsInterface || e.IsFSharpModule || e.IsClass || e.IsNamespace -> false
+        // FCS returns inconsistent results for override members; we're skipping these symbols.
+        | :? FSharpMemberOrFunctionOrValue as f when 
+                f.IsOverrideOrExplicitInterfaceImplementation ||
+                f.IsBaseValue ||
+                f.IsConstructor -> false
+        // Usage of DU case parameters does not give any meaningful feedback; we never gray them out.
+        | :? FSharpParameter -> false
+        | _ -> true
+
+    let getUnusedDeclarationRanges (symbolsUses: FSharpSymbolUse[]) (isScript: bool) =
+        let definitions =
+            symbolsUses
+            |> Array.filter (fun su -> 
+                su.IsFromDefinition && 
+                su.Symbol.DeclarationLocation.IsSome && 
+                (isScript || su.IsPrivateToFile) && 
+                not (su.Symbol.DisplayName.StartsWith "_") &&
+                isPotentiallyUnusedDeclaration su.Symbol)
+
+        let usages =
+            let usages = 
+                symbolsUses
+                |> Array.filter (fun su -> not su.IsFromDefinition)
+                |> Array.choose (fun su -> su.Symbol.DeclarationLocation)
+            HashSet(usages)
+
+        let unusedRanges =
+            definitions
+            |> Array.map (fun defSu -> defSu, usages.Contains defSu.Symbol.DeclarationLocation.Value)
+            |> Array.groupBy (fun (defSu, _) -> defSu.RangeAlternate)
+            |> Array.filter (fun (_, defSus) -> defSus |> Array.forall (fun (_, isUsed) -> not isUsed))
+            |> Array.map (fun (m, _) -> m)
+
+        Array.toList unusedRanges
+    
+    let getUnusedDeclarations(checkFileResults: FSharpCheckFileResults, isScriptFile: bool) : Async<range list> = 
+        async {
+            let! allSymbolUsesInFile = checkFileResults.GetAllUsesOfAllSymbolsInFile()
+            let unusedRanges = getUnusedDeclarationRanges allSymbolUsesInFile isScriptFile
+            return unusedRanges
         }
diff --git a/src/fsharp/service/ServiceAnalysis.fsi b/src/fsharp/service/ServiceAnalysis.fsi
@@ -8,4 +8,20 @@ open FSharp.Compiler.Range
 
 module public UnusedOpens =
     /// Get all unused open declarations in a file
-    val getUnusedOpens : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<range list>
+    val getUnusedOpens : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<range list>
+
+module public SimplifyNames = 
+   /// Data for use in finding unnecessarily-qualified names and generating diagnostics to simplify them
+   type SimplifiableRange = {
+        /// The range of a name that can be simplified
+        Range: range
+        /// The relative name that can be applied to a simplifiable name
+        RelativeName: string
+    }
+
+    /// Get all ranges that can be simplified in a file
+    val getSimplifiableNames : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<SimplifiableRange list>
+
+module public UnusedDeclarations = 
+    /// Get all unused declarations in a file
+    val getUnusedDeclarations : checkFileResults: FSharpCheckFileResults * isScriptFile: bool -> Async<range list>
diff --git a/src/fsharp/symbols/Symbols.fs b/src/fsharp/symbols/Symbols.fs
@@ -2518,5 +2518,30 @@ type FSharpSymbolUse(g:TcGlobals, denv: DisplayEnv, symbol:FSharpSymbol, itemOcc
     member __.Range = Range.toZ range
 
     member __.RangeAlternate = range
+     
+    member this.IsPrivateToFile = 
+        let isPrivate =
+            match this.Symbol with
+            | :? FSharpMemberOrFunctionOrValue as m -> not m.IsModuleValueOrMember || m.Accessibility.IsPrivate
+            | :? FSharpEntity as m -> m.Accessibility.IsPrivate
+            | :? FSharpGenericParameter -> true
+            | :? FSharpUnionCase as m -> m.Accessibility.IsPrivate
+            | :? FSharpField as m -> m.Accessibility.IsPrivate
+            | _ -> false
+            
+        let declarationLocation =
+            match this.Symbol.SignatureLocation with
+            | Some x -> Some x
+            | _ ->
+                match this.Symbol.DeclarationLocation with
+                | Some x -> Some x
+                | _ -> this.Symbol.ImplementationLocation
+            
+        let declaredInTheFile = 
+            match declarationLocation with
+            | Some declRange -> declRange.FileName = this.RangeAlternate.FileName
+            | _ -> false
+            
+        isPrivate && declaredInTheFile   
 
     override __.ToString() = sprintf "%O, %O, %O" symbol itemOcc range 
diff --git a/src/fsharp/symbols/Symbols.fsi b/src/fsharp/symbols/Symbols.fsi
@@ -1101,3 +1101,5 @@ type public FSharpSymbolUse =
     /// The range of text representing the reference to the symbol
     member RangeAlternate: range
 
+    /// Indicates if the FSharpSymbolUse is declared as private
+    member IsPrivateToFile : bool 
diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/SimplifyNameDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/SimplifyNameDiagnosticAnalyzer.fs
@@ -16,6 +16,7 @@ open FSharp.Compiler.Range
 open System.Runtime.Caching
 open Microsoft.CodeAnalysis.Host.Mef
 open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics
+open FSharp.Compiler.SourceCodeServices
 
 type private TextVersionHash = int
 type private PerDocumentSavedData = { Hash: int; Diagnostics: ImmutableArray<Diagnostic> }
@@ -26,7 +27,7 @@ type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () =
     static let userOpName = "SimplifyNameDiagnosticAnalyzer"
     let getProjectInfoManager (document: Document) = document.Project.Solution.Workspace.Services.GetService<FSharpCheckerWorkspaceService>().FSharpProjectOptionsManager
     let getChecker (document: Document) = document.Project.Solution.Workspace.Services.GetService<FSharpCheckerWorkspaceService>().Checker
-    let getPlidLength (plid: string list) = (plid |> List.sumBy String.length) + plid.Length
+
     static let cache = new MemoryCache("FSharp.Editor." + userOpName)
     // Make sure only one document is being analyzed at a time, to be nice
     static let guard = new SemaphoreSlim(1)
@@ -52,62 +53,15 @@ type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () =
                         let! sourceText = document.GetTextAsync()
                         let checker = getChecker document
                         let! _, _, checkResults = checker.ParseAndCheckDocument(document, projectOptions, sourceText = sourceText, userOpName=userOpName)
-                        let! symbolUses = checkResults.GetAllUsesOfAllSymbolsInFile() |> liftAsync
-                        let mutable result = ResizeArray()
-                        let symbolUses =
-                            symbolUses
-                            |> Array.filter (fun symbolUse -> not symbolUse.IsFromOpenStatement)
-                            |> Array.Parallel.map (fun symbolUse ->
-                                let lineStr = sourceText.Lines.[Line.toZ symbolUse.RangeAlternate.StartLine].ToString()
-                                // for `System.DateTime.Now` it returns ([|"System"; "DateTime"|], "Now")
-                                let partialName = QuickParse.GetPartialLongNameEx(lineStr, symbolUse.RangeAlternate.EndColumn - 1)
-                                // `symbolUse.RangeAlternate.Start` does not point to the start of plid, it points to start of `name`,
-                                // so we have to calculate plid's start ourselves.
-                                let plidStartCol = symbolUse.RangeAlternate.EndColumn - partialName.PartialIdent.Length - (getPlidLength partialName.QualifyingIdents)
-                                symbolUse, partialName.QualifyingIdents, plidStartCol, partialName.PartialIdent)
-                            |> Array.filter (fun (_, plid, _, name) -> name <> "" && not (List.isEmpty plid))
-                            |> Array.groupBy (fun (symbolUse, _, plidStartCol, _) -> symbolUse.RangeAlternate.StartLine, plidStartCol)
-                            |> Array.map (fun (_, xs) -> xs |> Array.maxBy (fun (symbolUse, _, _, _) -> symbolUse.RangeAlternate.EndColumn))
-                    
-                        for symbolUse, plid, plidStartCol, name in symbolUses do
-                            if not symbolUse.IsFromDefinition then
-                                let posAtStartOfName =
-                                    let r = symbolUse.RangeAlternate
-                                    if r.StartLine = r.EndLine then Range.mkPos r.StartLine (r.EndColumn - name.Length)
-                                    else r.Start
-                    
-                                let getNecessaryPlid (plid: string list) : Async<string list> =
-                                    let rec loop (rest: string list) (current: string list) =
-                                        async {
-                                            match rest with
-                                            | [] -> return current
-                                            | headIdent :: restPlid ->
-                                                let! res = checkResults.IsRelativeNameResolvableFromSymbol(posAtStartOfName, current, symbolUse.Symbol, userOpName=userOpName)
-                                                if res then return current
-                                                else return! loop restPlid (headIdent :: current)
-                                        }
-                                    loop (List.rev plid) []
-                               
-                                do! Async.Sleep DefaultTuning.SimplifyNameEachItemDelay |> liftAsync // be less intrusive, give other work priority most of the time
-                                let! necessaryPlid = getNecessaryPlid plid |> liftAsync
-                                
-                                match necessaryPlid with
-                                | necessaryPlid when necessaryPlid = plid -> ()
-                                | necessaryPlid ->
-                                    let r = symbolUse.RangeAlternate
-                                    let necessaryPlidStartCol = r.EndColumn - name.Length - (getPlidLength necessaryPlid)
-                    
-                                    let unnecessaryRange = 
-                                        Range.mkRange r.FileName (Range.mkPos r.StartLine plidStartCol) (Range.mkPos r.EndLine necessaryPlidStartCol)
-                                
-                                    let relativeName = (String.concat "." plid) + "." + name
-                                    result.Add(
-                                        Diagnostic.Create(
-                                           descriptor,
-                                           RoslynHelpers.RangeToLocation(unnecessaryRange, sourceText, document.FilePath),
-                                           properties = (dict [SimplifyNameDiagnosticAnalyzer.LongIdentPropertyKey, relativeName]).ToImmutableDictionary()))
-                    
-                        let diagnostics = result.ToImmutableArray()
+                        let! result = SimplifyNames.getSimplifiableNames(checkResults, fun lineNumber -> sourceText.Lines.[Line.toZ lineNumber].ToString()) |> liftAsync
+                        let mutable diag = ResizeArray()
+                        for r in result do
+                            diag.Add(
+                                Diagnostic.Create(
+                                   descriptor,
+                                   RoslynHelpers.RangeToLocation(r.Range, sourceText, document.FilePath),
+                                   properties = (dict [SimplifyNameDiagnosticAnalyzer.LongIdentPropertyKey, r.RelativeName]).ToImmutableDictionary()))
+                        let diagnostics = diag.ToImmutableArray()
                         cache.Remove(key) |> ignore
                         let data = { Hash = textVersionHash; Diagnostics=diagnostics }
                         let cacheItem = CacheItem(key, data)
diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/UnusedDeclarationsAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/UnusedDeclarationsAnalyzer.fs
diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs
