Skip to content
This repository was archived by the owner on Dec 23, 2024. It is now read-only.

Commit a25d03c

Browse files
Krzysztof-Cieslakcartermp
authored and
nosami
committed
Expose more services (dotnet#8291)
* 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 <[email protected]> * Apply feedback Co-authored-by: Phillip Carter <[email protected]>
1 parent 77df9b8 commit a25d03c

File tree

7 files changed

+168
-149
lines changed

7 files changed

+168
-149
lines changed

‎src/fsharp/service/ServiceAnalysis.fs‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,4 +226,115 @@ module UnusedOpens =
226226
let symbolUses = splitSymbolUses symbolUses
227227
let openStatements = getOpenStatements checkFileResults.OpenDeclarations
228228
return! filterOpenStatements symbolUses openStatements
229+
}
230+
231+
module SimplifyNames =
232+
type SimplifiableRange = {
233+
Range: range
234+
RelativeName: string
235+
}
236+
237+
let getPlidLength (plid: string list) = (plid |> List.sumBy String.length) + plid.Length
238+
239+
let getSimplifiableNames (checkFileResults: FSharpCheckFileResults, getSourceLineStr: int -> string) : Async<SimplifiableRange list> =
240+
async {
241+
let result = ResizeArray()
242+
let! symbolUses = checkFileResults.GetAllUsesOfAllSymbolsInFile()
243+
let symbolUses =
244+
symbolUses
245+
|> Array.filter (fun symbolUse -> not symbolUse.IsFromOpenStatement)
246+
|> Array.Parallel.map (fun symbolUse ->
247+
let lineStr = getSourceLineStr symbolUse.RangeAlternate.StartLine
248+
// for `System.DateTime.Now` it returns ([|"System"; "DateTime"|], "Now")
249+
let partialName = QuickParse.GetPartialLongNameEx(lineStr, symbolUse.RangeAlternate.EndColumn - 1)
250+
// `symbolUse.RangeAlternate.Start` does not point to the start of plid, it points to start of `name`,
251+
// so we have to calculate plid's start ourselves.
252+
let plidStartCol = symbolUse.RangeAlternate.EndColumn - partialName.PartialIdent.Length - (getPlidLength partialName.QualifyingIdents)
253+
symbolUse, partialName.QualifyingIdents, plidStartCol, partialName.PartialIdent)
254+
|> Array.filter (fun (_, plid, _, name) -> name <> "" && not (List.isEmpty plid))
255+
|> Array.groupBy (fun (symbolUse, _, plidStartCol, _) -> symbolUse.RangeAlternate.StartLine, plidStartCol)
256+
|> Array.map (fun (_, xs) -> xs |> Array.maxBy (fun (symbolUse, _, _, _) -> symbolUse.RangeAlternate.EndColumn))
257+
258+
for symbolUse, plid, plidStartCol, name in symbolUses do
259+
if not symbolUse.IsFromDefinition then
260+
let posAtStartOfName =
261+
let r = symbolUse.RangeAlternate
262+
if r.StartLine = r.EndLine then Range.mkPos r.StartLine (r.EndColumn - name.Length)
263+
else r.Start
264+
265+
let getNecessaryPlid (plid: string list) : Async<string list> =
266+
let rec loop (rest: string list) (current: string list) =
267+
async {
268+
match rest with
269+
| [] -> return current
270+
| headIdent :: restPlid ->
271+
let! res = checkFileResults.IsRelativeNameResolvableFromSymbol(posAtStartOfName, current, symbolUse.Symbol)
272+
if res then return current
273+
else return! loop restPlid (headIdent :: current)
274+
}
275+
loop (List.rev plid) []
276+
277+
let! necessaryPlid = getNecessaryPlid plid
278+
279+
match necessaryPlid with
280+
| necessaryPlid when necessaryPlid = plid -> ()
281+
| necessaryPlid ->
282+
let r = symbolUse.RangeAlternate
283+
let necessaryPlidStartCol = r.EndColumn - name.Length - (getPlidLength necessaryPlid)
284+
285+
let unnecessaryRange =
286+
Range.mkRange r.FileName (Range.mkPos r.StartLine plidStartCol) (Range.mkPos r.EndLine necessaryPlidStartCol)
287+
288+
let relativeName = (String.concat "." plid) + "." + name
289+
result.Add({Range = unnecessaryRange; RelativeName = relativeName})
290+
291+
return List.ofSeq result
292+
}
293+
294+
module UnusedDeclarations =
295+
let isPotentiallyUnusedDeclaration (symbol: FSharpSymbol) : bool =
296+
match symbol with
297+
// Determining that a record, DU or module is used anywhere requires inspecting all their enclosed entities (fields, cases and func / vals)
298+
// for usages, which is too expensive to do. Hence we never gray them out.
299+
| :? FSharpEntity as e when e.IsFSharpRecord || e.IsFSharpUnion || e.IsInterface || e.IsFSharpModule || e.IsClass || e.IsNamespace -> false
300+
// FCS returns inconsistent results for override members; we're skipping these symbols.
301+
| :? FSharpMemberOrFunctionOrValue as f when
302+
f.IsOverrideOrExplicitInterfaceImplementation ||
303+
f.IsBaseValue ||
304+
f.IsConstructor -> false
305+
// Usage of DU case parameters does not give any meaningful feedback; we never gray them out.
306+
| :? FSharpParameter -> false
307+
| _ -> true
308+
309+
let getUnusedDeclarationRanges (symbolsUses: FSharpSymbolUse[]) (isScript: bool) =
310+
let definitions =
311+
symbolsUses
312+
|> Array.filter (fun su ->
313+
su.IsFromDefinition &&
314+
su.Symbol.DeclarationLocation.IsSome &&
315+
(isScript || su.IsPrivateToFile) &&
316+
not (su.Symbol.DisplayName.StartsWith "_") &&
317+
isPotentiallyUnusedDeclaration su.Symbol)
318+
319+
let usages =
320+
let usages =
321+
symbolsUses
322+
|> Array.filter (fun su -> not su.IsFromDefinition)
323+
|> Array.choose (fun su -> su.Symbol.DeclarationLocation)
324+
HashSet(usages)
325+
326+
let unusedRanges =
327+
definitions
328+
|> Array.map (fun defSu -> defSu, usages.Contains defSu.Symbol.DeclarationLocation.Value)
329+
|> Array.groupBy (fun (defSu, _) -> defSu.RangeAlternate)
330+
|> Array.filter (fun (_, defSus) -> defSus |> Array.forall (fun (_, isUsed) -> not isUsed))
331+
|> Array.map (fun (m, _) -> m)
332+
333+
Array.toList unusedRanges
334+
335+
let getUnusedDeclarations(checkFileResults: FSharpCheckFileResults, isScriptFile: bool) : Async<range list> =
336+
async {
337+
let! allSymbolUsesInFile = checkFileResults.GetAllUsesOfAllSymbolsInFile()
338+
let unusedRanges = getUnusedDeclarationRanges allSymbolUsesInFile isScriptFile
339+
return unusedRanges
229340
}

‎src/fsharp/service/ServiceAnalysis.fsi‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,20 @@ open FSharp.Compiler.Range
88

99
module public UnusedOpens =
1010
/// Get all unused open declarations in a file
11-
val getUnusedOpens : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<range list>
11+
val getUnusedOpens : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<range list>
12+
13+
module public SimplifyNames =
14+
/// Data for use in finding unnecessarily-qualified names and generating diagnostics to simplify them
15+
type SimplifiableRange = {
16+
/// The range of a name that can be simplified
17+
Range: range
18+
/// The relative name that can be applied to a simplifiable name
19+
RelativeName: string
20+
}
21+
22+
/// Get all ranges that can be simplified in a file
23+
val getSimplifiableNames : checkFileResults: FSharpCheckFileResults * getSourceLineStr: (int -> string) -> Async<SimplifiableRange list>
24+
25+
module public UnusedDeclarations =
26+
/// Get all unused declarations in a file
27+
val getUnusedDeclarations : checkFileResults: FSharpCheckFileResults * isScriptFile: bool -> Async<range list>

‎src/fsharp/symbols/Symbols.fs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,5 +2518,30 @@ type FSharpSymbolUse(g:TcGlobals, denv: DisplayEnv, symbol:FSharpSymbol, itemOcc
25182518
member __.Range = Range.toZ range
25192519

25202520
member __.RangeAlternate = range
2521+
2522+
member this.IsPrivateToFile =
2523+
let isPrivate =
2524+
match this.Symbol with
2525+
| :? FSharpMemberOrFunctionOrValue as m -> not m.IsModuleValueOrMember || m.Accessibility.IsPrivate
2526+
| :? FSharpEntity as m -> m.Accessibility.IsPrivate
2527+
| :? FSharpGenericParameter -> true
2528+
| :? FSharpUnionCase as m -> m.Accessibility.IsPrivate
2529+
| :? FSharpField as m -> m.Accessibility.IsPrivate
2530+
| _ -> false
2531+
2532+
let declarationLocation =
2533+
match this.Symbol.SignatureLocation with
2534+
| Some x -> Some x
2535+
| _ ->
2536+
match this.Symbol.DeclarationLocation with
2537+
| Some x -> Some x
2538+
| _ -> this.Symbol.ImplementationLocation
2539+
2540+
let declaredInTheFile =
2541+
match declarationLocation with
2542+
| Some declRange -> declRange.FileName = this.RangeAlternate.FileName
2543+
| _ -> false
2544+
2545+
isPrivate && declaredInTheFile
25212546

25222547
override __.ToString() = sprintf "%O, %O, %O" symbol itemOcc range

‎src/fsharp/symbols/Symbols.fsi‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,3 +1101,5 @@ type public FSharpSymbolUse =
11011101
/// The range of text representing the reference to the symbol
11021102
member RangeAlternate: range
11031103

1104+
/// Indicates if the FSharpSymbolUse is declared as private
1105+
member IsPrivateToFile : bool

‎vsintegration/src/FSharp.Editor/Diagnostics/SimplifyNameDiagnosticAnalyzer.fs‎

Lines changed: 11 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ open FSharp.Compiler.Range
1616
open System.Runtime.Caching
1717
open Microsoft.CodeAnalysis.Host.Mef
1818
open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics
19+
open FSharp.Compiler.SourceCodeServices
1920

2021
type private TextVersionHash = int
2122
type private PerDocumentSavedData = { Hash: int; Diagnostics: ImmutableArray<Diagnostic> }
@@ -26,7 +27,7 @@ type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () =
2627
static let userOpName = "SimplifyNameDiagnosticAnalyzer"
2728
let getProjectInfoManager (document: Document) = document.Project.Solution.Workspace.Services.GetService<FSharpCheckerWorkspaceService>().FSharpProjectOptionsManager
2829
let getChecker (document: Document) = document.Project.Solution.Workspace.Services.GetService<FSharpCheckerWorkspaceService>().Checker
29-
let getPlidLength (plid: string list) = (plid |> List.sumBy String.length) + plid.Length
30+
3031
static let cache = new MemoryCache("FSharp.Editor." + userOpName)
3132
// Make sure only one document is being analyzed at a time, to be nice
3233
static let guard = new SemaphoreSlim(1)
@@ -52,62 +53,15 @@ type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () =
5253
let! sourceText = document.GetTextAsync()
5354
let checker = getChecker document
5455
let! _, _, checkResults = checker.ParseAndCheckDocument(document, projectOptions, sourceText = sourceText, userOpName=userOpName)
55-
let! symbolUses = checkResults.GetAllUsesOfAllSymbolsInFile() |> liftAsync
56-
let mutable result = ResizeArray()
57-
let symbolUses =
58-
symbolUses
59-
|> Array.filter (fun symbolUse -> not symbolUse.IsFromOpenStatement)
60-
|> Array.Parallel.map (fun symbolUse ->
61-
let lineStr = sourceText.Lines.[Line.toZ symbolUse.RangeAlternate.StartLine].ToString()
62-
// for `System.DateTime.Now` it returns ([|"System"; "DateTime"|], "Now")
63-
let partialName = QuickParse.GetPartialLongNameEx(lineStr, symbolUse.RangeAlternate.EndColumn - 1)
64-
// `symbolUse.RangeAlternate.Start` does not point to the start of plid, it points to start of `name`,
65-
// so we have to calculate plid's start ourselves.
66-
let plidStartCol = symbolUse.RangeAlternate.EndColumn - partialName.PartialIdent.Length - (getPlidLength partialName.QualifyingIdents)
67-
symbolUse, partialName.QualifyingIdents, plidStartCol, partialName.PartialIdent)
68-
|> Array.filter (fun (_, plid, _, name) -> name <> "" && not (List.isEmpty plid))
69-
|> Array.groupBy (fun (symbolUse, _, plidStartCol, _) -> symbolUse.RangeAlternate.StartLine, plidStartCol)
70-
|> Array.map (fun (_, xs) -> xs |> Array.maxBy (fun (symbolUse, _, _, _) -> symbolUse.RangeAlternate.EndColumn))
71-
72-
for symbolUse, plid, plidStartCol, name in symbolUses do
73-
if not symbolUse.IsFromDefinition then
74-
let posAtStartOfName =
75-
let r = symbolUse.RangeAlternate
76-
if r.StartLine = r.EndLine then Range.mkPos r.StartLine (r.EndColumn - name.Length)
77-
else r.Start
78-
79-
let getNecessaryPlid (plid: string list) : Async<string list> =
80-
let rec loop (rest: string list) (current: string list) =
81-
async {
82-
match rest with
83-
| [] -> return current
84-
| headIdent :: restPlid ->
85-
let! res = checkResults.IsRelativeNameResolvableFromSymbol(posAtStartOfName, current, symbolUse.Symbol, userOpName=userOpName)
86-
if res then return current
87-
else return! loop restPlid (headIdent :: current)
88-
}
89-
loop (List.rev plid) []
90-
91-
do! Async.Sleep DefaultTuning.SimplifyNameEachItemDelay |> liftAsync // be less intrusive, give other work priority most of the time
92-
let! necessaryPlid = getNecessaryPlid plid |> liftAsync
93-
94-
match necessaryPlid with
95-
| necessaryPlid when necessaryPlid = plid -> ()
96-
| necessaryPlid ->
97-
let r = symbolUse.RangeAlternate
98-
let necessaryPlidStartCol = r.EndColumn - name.Length - (getPlidLength necessaryPlid)
99-
100-
let unnecessaryRange =
101-
Range.mkRange r.FileName (Range.mkPos r.StartLine plidStartCol) (Range.mkPos r.EndLine necessaryPlidStartCol)
102-
103-
let relativeName = (String.concat "." plid) + "." + name
104-
result.Add(
105-
Diagnostic.Create(
106-
descriptor,
107-
RoslynHelpers.RangeToLocation(unnecessaryRange, sourceText, document.FilePath),
108-
properties = (dict [SimplifyNameDiagnosticAnalyzer.LongIdentPropertyKey, relativeName]).ToImmutableDictionary()))
109-
110-
let diagnostics = result.ToImmutableArray()
56+
let! result = SimplifyNames.getSimplifiableNames(checkResults, fun lineNumber -> sourceText.Lines.[Line.toZ lineNumber].ToString()) |> liftAsync
57+
let mutable diag = ResizeArray()
58+
for r in result do
59+
diag.Add(
60+
Diagnostic.Create(
61+
descriptor,
62+
RoslynHelpers.RangeToLocation(r.Range, sourceText, document.FilePath),
63+
properties = (dict [SimplifyNameDiagnosticAnalyzer.LongIdentPropertyKey, r.RelativeName]).ToImmutableDictionary()))
64+
let diagnostics = diag.ToImmutableArray()
11165
cache.Remove(key) |> ignore
11266
let data = { Hash = textVersionHash; Diagnostics=diagnostics }
11367
let cacheItem = CacheItem(key, data)

0 commit comments

Comments
 (0)