Skip to content

Commit

Permalink
Merge pull request dotnet#3803 from vasily-kirichenko/typed-open-decl…
Browse files Browse the repository at this point in the history 
…arations-2

Typed Unused Opens analyzer
  • Loading branch information
@KevinRansom
KevinRansom authored Oct 30, 2017
2 parents eb6e9df + 7c7be84 commit 99a9940
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 565 deletions.
2 changes: 1 addition & 1 deletion Diagnostics/UnusedDeclarationsAnalyzer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type internal UnusedDeclarationsAnalyzer() =
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 -> false
| :? 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 ||
Expand Down
142 changes: 13 additions & 129 deletions Diagnostics/UnusedOpensDiagnosticAnalyzer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,129 +15,7 @@ open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.Ast
open Microsoft.FSharp.Compiler.Range
open Microsoft.FSharp.Compiler.SourceCodeServices
open Symbols


module private UnusedOpens =


let rec visitSynModuleOrNamespaceDecls (parent: Ast.LongIdent) decls : (Set<string> * range) list =
[ for decl in decls do
match decl with
| SynModuleDecl.Open(LongIdentWithDots.LongIdentWithDots(id = longId), range) ->
yield
set [ yield (longId |> List.map(fun l -> l.idText) |> String.concat ".")
// `open N.M` can open N.M module from parent module as well, if it's non empty
if not (List.isEmpty parent) then
yield (parent @ longId |> List.map(fun l -> l.idText) |> String.concat ".") ], range
| SynModuleDecl.NestedModule(SynComponentInfo.ComponentInfo(longId = longId),_, decls,_,_) ->
yield! visitSynModuleOrNamespaceDecls longId decls
| _ -> () ]

let getOpenStatements = function
| ParsedInput.ImplFile (ParsedImplFileInput(modules = modules)) ->
[ for md in modules do
let SynModuleOrNamespace(longId = longId; decls = decls) = md
yield! visitSynModuleOrNamespaceDecls longId decls ]
| _ -> []

let getAutoOpenAccessPath (ent:FSharpEntity) =
// Some.Namespace+AutoOpenedModule+Entity

// HACK: I can't see a way to get the EnclosingEntity of an Entity
// Some.Namespace + Some.Namespace.AutoOpenedModule are both valid
ent.TryFullName |> Option.bind(fun _ ->
if (not ent.IsNamespace) && ent.QualifiedName.Contains "+" then
Some ent.QualifiedName.[0..ent.QualifiedName.IndexOf "+" - 1]
else
None)

let entityNamespace (entOpt: FSharpEntity option) =
match entOpt with
| Some ent ->
if ent.IsFSharpModule then
[ yield Some ent.QualifiedName
yield Some ent.LogicalName
yield Some ent.AccessPath
yield Some ent.FullName
yield Some ent.DisplayName
yield ent.TryGetFullDisplayName()
if ent.HasFSharpModuleSuffix then
yield Some (ent.AccessPath + "." + ent.DisplayName)]
else
[ yield ent.Namespace
yield Some ent.AccessPath
yield getAutoOpenAccessPath ent
for path in ent.AllCompilationPaths do
yield Some path
]
| None -> []

let symbolIsFullyQualified (sourceText: SourceText) (sym: FSharpSymbolUse) (fullName: string) =
match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, sym.RangeAlternate) with
| Some span // check that the symbol hasn't provided an invalid span
when sourceText.Length < span.Start
|| sourceText.Length < span.End -> false
| Some span -> sourceText.ToString span = fullName
| None -> false

let getUnusedOpens (sourceText: SourceText) (parsedInput: ParsedInput) (symbolUses: FSharpSymbolUse[]) =

let getPartNamespace (symbolUse: FSharpSymbolUse) (fullName: string) =
// given a symbol range such as `Text.ISegment` and a full name of `MonoDevelop.Core.Text.ISegment`, return `MonoDevelop.Core`
let length = symbolUse.RangeAlternate.EndColumn - symbolUse.RangeAlternate.StartColumn
let lengthDiff = fullName.Length - length - 2
if lengthDiff <= 0 || lengthDiff > fullName.Length - 1 then None
else Some fullName.[0..lengthDiff]

let getPossibleNamespaces (symbolUse: FSharpSymbolUse) : string list =
let isQualified = symbolIsFullyQualified sourceText symbolUse
maybe {
let! fullNames, declaringEntity =
match symbolUse with
| SymbolUse.Entity (ent, cleanFullNames) when not (cleanFullNames |> List.exists isQualified) ->
Some (cleanFullNames, Some ent)
| SymbolUse.Field f when not (isQualified f.FullName) ->
Some ([f.FullName], Some f.DeclaringEntity)
| SymbolUse.MemberFunctionOrValue mfv when not (isQualified mfv.FullName) ->
Some ([mfv.FullName], mfv.EnclosingEntity)
| SymbolUse.Operator op when not (isQualified op.FullName) ->
Some ([op.FullName], op.EnclosingEntity)
| SymbolUse.ActivePattern ap when not (isQualified ap.FullName) ->
Some ([ap.FullName], ap.EnclosingEntity)
| SymbolUse.ActivePatternCase apc when not (isQualified apc.FullName) ->
Some ([apc.FullName], apc.Group.EnclosingEntity)
| SymbolUse.UnionCase uc when not (isQualified uc.FullName) ->
Some ([uc.FullName], Some uc.ReturnType.TypeDefinition)
| SymbolUse.Parameter p when not (isQualified p.FullName) && p.Type.HasTypeDefinition ->
Some ([p.FullName], Some p.Type.TypeDefinition)
| _ -> None

return
[ for name in fullNames do
yield getPartNamespace symbolUse name
yield! entityNamespace declaringEntity ]
} |> Option.toList |> List.concat |> List.choose id

let namespacesInUse =
symbolUses
|> Seq.filter (fun (s: FSharpSymbolUse) -> not s.IsFromDefinition)
|> Seq.collect getPossibleNamespaces
|> Set.ofSeq

let filter list: (Set<string> * range) list =
let rec filterInner acc list (seenNamespaces: Set<string>) =
let notUsed ns = not (namespacesInUse.Contains ns) || seenNamespaces.Contains ns
match list with
| (ns, range) :: xs when ns |> Set.forall notUsed ->
filterInner ((ns, range) :: acc) xs (seenNamespaces |> Set.union ns)
| (ns, _) :: xs ->
filterInner acc xs (seenNamespaces |> Set.union ns)
| [] -> List.rev acc
filterInner [] list Set.empty

let openStatements = getOpenStatements parsedInput
openStatements |> filter |> List.map snd
open Microsoft.VisualStudio.FSharp.Editor.Symbols

[<DiagnosticAnalyzer(FSharpConstants.FSharpLanguageName)>]
type internal UnusedOpensDiagnosticAnalyzer() =
Expand All @@ -160,13 +38,19 @@ type internal UnusedOpensDiagnosticAnalyzer() =
override __.SupportedDiagnostics = ImmutableArray.Create Descriptor
override this.AnalyzeSyntaxAsync(_, _) = Task.FromResult ImmutableArray<Diagnostic>.Empty

static member GetUnusedOpenRanges(document: Document, options, checker: FSharpChecker) =
static member GetUnusedOpenRanges(document: Document, options, checker: FSharpChecker) : Async<Option<range list>> =
asyncMaybe {
do! Option.guard Settings.CodeFixes.UnusedOpens
let! sourceText = document.GetTextAsync()
let! _, parsedInput, checkResults = checker.ParseAndCheckDocument(document, options, sourceText = sourceText, allowStaleResults = true, userOpName = userOpName)
let! symbolUses = checkResults.GetAllUsesOfAllSymbolsInFile() |> liftAsync
return UnusedOpens.getUnusedOpens sourceText parsedInput symbolUses
let! _, _, checkResults = checker.ParseAndCheckDocument(document, options, sourceText = sourceText, allowStaleResults = true, userOpName = userOpName)
#if DEBUG
let sw = Stopwatch.StartNew()
#endif
let! unusedOpens = UnusedOpens.getUnusedOpens(checkResults, fun lineNumber -> sourceText.Lines.[Line.toZ lineNumber].ToString()) |> liftAsync
#if DEBUG
Logging.Logging.logInfof "*** Got %d unused opens in %O" unusedOpens.Length sw.Elapsed
#endif
return unusedOpens
}

override this.AnalyzeSemanticsAsync(document: Document, cancellationToken: CancellationToken) =
Expand All @@ -180,10 +64,10 @@ type internal UnusedOpensDiagnosticAnalyzer() =

return
unusedOpens
|> List.map (fun m ->
|> List.map (fun range ->
Diagnostic.Create(
Descriptor,
RoslynHelpers.RangeToLocation(m, sourceText, document.FilePath)))
RoslynHelpers.RangeToLocation(range, sourceText, document.FilePath)))
|> Seq.toImmutableArray
}
|> Async.map (Option.defaultValue ImmutableArray.Empty)
Expand Down
1 change: 0 additions & 1 deletion FSharp.Editor.fsproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
<Compile Include="Options\EditorOptions.fs" />
<Compile Include="LanguageService\Tokenizer.fs" />
<Compile Include="LanguageService\Symbols.fs" />
<Compile Include="LanguageService\TypedAstUtils.fs" />
<Compile Include="LanguageService\FSharpCheckerExtensions.fs" />
<Compile Include="LanguageService\LanguageService.fs" />
<Compile Include="LanguageService\AssemblyContentProvider.fs" />
Expand Down
19 changes: 9 additions & 10 deletions LanguageService/FSharpCheckerExtensions.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ open Microsoft.CodeAnalysis.Text
open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.Ast
open Microsoft.FSharp.Compiler.SourceCodeServices
open TypedAstUtils

type CheckResults =
| Ready of (FSharpParseFileResults * FSharpCheckFileResults) option
Expand Down Expand Up @@ -108,7 +107,7 @@ type FSharpChecker with
match symbolUse.Symbol with
// Make sure that unsafe manipulation isn't executed if unused opens are disabled
| _ when not checkForUnusedOpens -> None
| TypedAstPatterns.MemberFunctionOrValue func when func.IsExtensionMember ->
| Symbol.MemberFunctionOrValue func when func.IsExtensionMember ->
if func.IsProperty then
let fullNames =
[| if func.HasGetterMethod then
Expand All @@ -125,9 +124,9 @@ type FSharpChecker with
| [||] -> None
| _ -> Some fullNames
else
match func.EnclosingEntity.Value with
match func.EnclosingEntity with
// C# extension method
| TypedAstPatterns.FSharpEntity TypedAstPatterns.Class ->
| Some (Symbol.FSharpEntity Symbol.Class) ->
let fullName = symbolUse.Symbol.FullName.Split '.'
if fullName.Length > 2 then
(* For C# extension methods FCS returns full name including the class name, like:
Expand All @@ -142,9 +141,9 @@ type FSharpChecker with
else None
| _ -> None
// Operators
| TypedAstPatterns.MemberFunctionOrValue func ->
| Symbol.MemberFunctionOrValue func ->
match func with
| TypedAstPatterns.Constructor _ ->
| Symbol.Constructor _ ->
// full name of a constructor looks like "UnusedSymbolClassifierTests.PrivateClass.( .ctor )"
// to make well formed full name parts we cut "( .ctor )" from the tail.
let fullName = func.FullName
Expand All @@ -160,16 +159,16 @@ type FSharpChecker with
| Some idents -> yield String.concat "." idents
| None -> ()
|]
| TypedAstPatterns.FSharpEntity e ->
| Symbol.FSharpEntity e ->
match e with
| e, TypedAstPatterns.Attribute, _ ->
| e, Symbol.Attribute, _ ->
e.TryGetFullName ()
|> Option.map (fun fullName ->
[| fullName; fullName.Substring(0, fullName.Length - "Attribute".Length) |])
| e, _, _ ->
e.TryGetFullName () |> Option.map (fun fullName -> [| fullName |])
| TypedAstPatterns.RecordField _
| TypedAstPatterns.UnionCase _ as symbol ->
| Symbol.RecordField _
| Symbol.UnionCase _ as symbol ->
Some [| let fullName = symbol.FullName
yield fullName
let idents = fullName.Split '.'
Expand Down
Loading

0 comments on commit 99a9940

Please sign in to comment.