Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions src/fsharp/service/ServiceAnalysis.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
18 changes: 17 additions & 1 deletion src/fsharp/service/ServiceAnalysis.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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>
25 changes: 25 additions & 0 deletions src/fsharp/symbols/Symbols.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions src/fsharp/symbols/Symbols.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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> }
Expand All @@ -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)
Expand All @@ -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)
Expand Down
Loading