Skip to content

Hints refactoring #15083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 14, 2023
Merged

Hints refactoring #15083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d1cb2e
Hints refactoring
psfinaki Apr 11, 2023
84006b7
up
psfinaki Apr 13, 2023
e119a24
up
psfinaki Apr 13, 2023
60f6bbd
piping, naming
psfinaki Apr 14, 2023
5394f0c
piping, naming
psfinaki Apr 14, 2023
6b6044d
up
psfinaki Apr 14, 2023
f7a4289
up
psfinaki Apr 14, 2023
38fb3af
up
psfinaki Apr 14, 2023
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
70 changes: 24 additions & 46 deletions vsintegration/src/FSharp.Editor/Hints/HintService.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,59 +5,37 @@ namespace Microsoft.VisualStudio.FSharp.Editor.Hints
open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Text
open Microsoft.VisualStudio.FSharp.Editor
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Symbols
open Hints

module HintService =

type private NativeHintResolver = FSharpSymbolUse seq -> NativeHint seq

let inline private getTypeHints parseResults symbol : NativeHintResolver =
Seq.filter (InlineTypeHints.isValidForHint parseResults symbol)
>> Seq.collect (InlineTypeHints.getHints symbol)

let inline private getReturnTypeHints parseResults symbol =
Seq.collect (InlineReturnTypeHints(parseResults, symbol).getHints)

let inline private getHintsForMemberOrFunctionOrValue (sourceText: SourceText) parseResults symbol : NativeHintResolver =
Seq.filter (InlineParameterNameHints.isMemberOrFunctionOrValueValidForHint symbol)
>> Seq.collect (InlineParameterNameHints.getHintsForMemberOrFunctionOrValue sourceText parseResults symbol)

let inline private getHintsForUnionCase parseResults symbol : NativeHintResolver =
Seq.filter (InlineParameterNameHints.isUnionCaseValidForHint symbol)
>> Seq.collect (InlineParameterNameHints.getHintsForUnionCase parseResults symbol)

let private getHintResolvers (sourceText: SourceText) parseResults hintKinds (symbol: FSharpSymbol) : NativeHintResolver seq =
let rec resolve hintKinds resolvers =
let private getHints sourceText parseResults hintKinds symbolUses (symbol: FSharpSymbol) =

let getHintsPerKind hintKind =
match hintKind, symbol with
| HintKind.TypeHint, (:? FSharpMemberOrFunctionOrValue as symbol) ->
symbolUses |> Seq.collect (InlineTypeHints(parseResults, symbol)).getHints
| HintKind.ReturnTypeHint, (:? FSharpMemberOrFunctionOrValue as symbol) ->
symbolUses |> Seq.collect (InlineReturnTypeHints(parseResults, symbol).getHints)
| HintKind.ParameterNameHint, (:? FSharpMemberOrFunctionOrValue as symbol) ->
symbolUses
|> Seq.collect (InlineParameterNameHints(parseResults).getHintsForMemberOrFunctionOrValue sourceText symbol)
| HintKind.ParameterNameHint, (:? FSharpUnionCase as symbol) ->
symbolUses
|> Seq.collect (InlineParameterNameHints(parseResults).getHintsForUnionCase symbol)
| _ -> []

let rec getHints hintKinds acc =
match hintKinds with
| [] -> resolvers |> Seq.choose id
| hintKind :: hintKinds ->
match hintKind with
| HintKind.TypeHint ->
match symbol with
| :? FSharpMemberOrFunctionOrValue as symbol -> getTypeHints parseResults symbol |> Some
| _ -> None
| HintKind.ReturnTypeHint ->
match symbol with
| :? FSharpMemberOrFunctionOrValue as symbol -> getReturnTypeHints parseResults symbol |> Some
| _ -> None
| HintKind.ParameterNameHint ->
match symbol with
| :? FSharpMemberOrFunctionOrValue as symbol ->
getHintsForMemberOrFunctionOrValue sourceText parseResults symbol |> Some
| :? FSharpUnionCase as symbol -> getHintsForUnionCase parseResults symbol |> Some
| _ -> None
// we'll be adding other stuff gradually here
:: resolvers
|> resolve hintKinds
| [] -> acc
| hintKind :: hintKinds -> getHintsPerKind hintKind :: acc |> getHints hintKinds

resolve hintKinds []
getHints (hintKinds |> Set.toList) []

let private getHintsForSymbol (sourceText: SourceText) parseResults hintKinds (symbol: FSharpSymbol, symbolUses: FSharpSymbolUse seq) =
symbol
|> getHintResolvers sourceText parseResults hintKinds
|> Seq.collect (fun resolve -> resolve symbolUses)
let private getHintsForSymbol (sourceText: SourceText) parseResults hintKinds (symbol, symbolUses) =
let hints = getHints sourceText parseResults hintKinds symbolUses symbol
Seq.concat hints

let getHintsForDocument sourceText (document: Document) hintKinds userOpName cancellationToken =
async {
Expand All @@ -69,6 +47,6 @@ module HintService =
return
checkResults.GetAllUsesOfAllSymbolsInFile cancellationToken
|> Seq.groupBy (fun symbolUse -> symbolUse.Symbol)
|> Seq.collect (getHintsForSymbol sourceText parseResults (hintKinds |> Set.toList))
|> Seq.collect (getHintsForSymbol sourceText parseResults hintKinds)
|> Seq.toList
}
92 changes: 47 additions & 45 deletions vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,28 @@ open FSharp.Compiler.EditorServices
open FSharp.Compiler.Symbols
open FSharp.Compiler.Text
open Hints
open Microsoft.VisualStudio.FSharp.Editor

module InlineParameterNameHints =
type InlineParameterNameHints(parseResults: FSharpParseFileResults) =

let private getParameterHint (range: range, parameter: FSharpParameter) =
let getParameterHint (range: range, parameter: FSharpParameter) =
{
Kind = HintKind.ParameterNameHint
Range = range.StartRange
Parts = [ TaggedText(TextTag.Text, $"{parameter.DisplayName} = ") ]
}

let private getFieldHint (range: range, field: FSharpField) =
let getFieldHint (range: range, field: FSharpField) =
{
Kind = HintKind.ParameterNameHint
Range = range.StartRange
Parts = [ TaggedText(TextTag.Text, $"{field.Name} = ") ]
}

let private doesParameterNameExist (parameter: FSharpParameter) = parameter.DisplayName <> ""
let parameterNameExists (parameter: FSharpParameter) = parameter.DisplayName <> ""

let private doesFieldNameExist (field: FSharpField) = not field.IsNameGenerated
let fieldNameExists (field: FSharpField) = not field.IsNameGenerated

let private getArgumentLocations (symbolUse: FSharpSymbolUse) (parseResults: FSharpParseFileResults) =
let getArgumentLocations (symbolUse: FSharpSymbolUse) =

let position =
Position.mkPos (symbolUse.Range.End.Line) (symbolUse.Range.End.Column + 1)
Expand All @@ -43,20 +42,19 @@ module InlineParameterNameHints =
|> Option.filter (not << Seq.isEmpty)
|> Option.defaultValue Seq.empty

let private getTupleRanges =
Seq.map (fun location -> location.ArgumentRange) >> Seq.toList
let getTupleRanges = Seq.map (fun location -> location.ArgumentRange)

let private getCurryRanges (symbolUse: FSharpSymbolUse) (parseResults: FSharpParseFileResults) =
let getCurryRanges (symbolUse: FSharpSymbolUse) =

parseResults.GetAllArgumentsForFunctionApplicationAtPosition symbolUse.Range.Start
|> Option.defaultValue []

let private isNamedArgument range =
let isNamedArgument range =
Seq.filter (fun location -> location.IsNamedArgument)
>> Seq.map (fun location -> location.ArgumentRange)
>> Seq.contains range

let private getSourceTextAtRange (sourceText: SourceText) (range: range) =
let getSourceTextAtRange (sourceText: SourceText) (range: range) =
(RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) |> sourceText.GetSubText)
.ToString()

Expand All @@ -76,49 +74,51 @@ module InlineParameterNameHints =
false

let isUnionCaseValidForHint (symbol: FSharpUnionCase) (symbolUse: FSharpSymbolUse) =
symbolUse.IsFromUse && symbol.DisplayName <> "(::)"
symbolUse.IsFromUse
&& symbol.DisplayName <> "(::)"
// If a case does not use field names, don't even bother getting applied argument ranges
&& symbol.Fields |> Seq.exists fieldNameExists

let getHintsForMemberOrFunctionOrValue
member _.getHintsForMemberOrFunctionOrValue
(sourceText: SourceText)
(parseResults: FSharpParseFileResults)
(symbol: FSharpMemberOrFunctionOrValue)
(symbolUse: FSharpSymbolUse)
=

let parameters = symbol.CurriedParameterGroups |> Seq.concat
let argumentLocations = parseResults |> getArgumentLocations symbolUse

let tupleRanges = argumentLocations |> getTupleRanges
let curryRanges = parseResults |> getCurryRanges symbolUse
if isMemberOrFunctionOrValueValidForHint symbol symbolUse then
let parameters = Seq.concat symbol.CurriedParameterGroups
let argumentLocations = getArgumentLocations symbolUse

let ranges =
if tupleRanges |> (not << Seq.isEmpty) then
tupleRanges
else
curryRanges
|> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))
let tupleRanges = getTupleRanges argumentLocations
let curryRanges = getCurryRanges symbolUse

let argumentNames = ranges |> Seq.map (getSourceTextAtRange sourceText)

parameters
|> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths
|> Seq.where (snd >> doesParameterNameExist)
|> Seq.zip argumentNames
|> Seq.choose (fun (argumentName, (range, parameter)) ->
if argumentName <> parameter.DisplayName then
Some(getParameterHint (range, parameter))
else
None)
|> Seq.toList
let ranges =
if Seq.isEmpty tupleRanges then
curryRanges |> List.toSeq
else
tupleRanges
|> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))

let argumentNames = Seq.map (getSourceTextAtRange sourceText) ranges

parameters
|> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths
|> Seq.where (snd >> parameterNameExists)
|> Seq.zip argumentNames
|> Seq.choose (fun (argumentName, (range, parameter)) ->
if argumentName <> parameter.DisplayName then
Some(getParameterHint (range, parameter))
else
None)
|> Seq.toList
else
[]

let getHintsForUnionCase (parseResults: FSharpParseFileResults) (symbol: FSharpUnionCase) (symbolUse: FSharpSymbolUse) =
member _.getHintsForUnionCase (symbol: FSharpUnionCase) (symbolUse: FSharpSymbolUse) =
if isUnionCaseValidForHint symbol symbolUse then

let fields = Seq.toList symbol.Fields
let fields = Seq.toList symbol.Fields

// If a case does not use field names, don't even bother getting applied argument ranges
if fields |> List.exists doesFieldNameExist |> not then
[]
else
let ranges =
parseResults.GetAllArgumentsForFunctionApplicationAtPosition symbolUse.Range.Start

Expand All @@ -127,7 +127,9 @@ module InlineParameterNameHints =
| Some ranges when ranges.Length = fields.Length ->
fields
|> List.zip ranges
|> List.where (snd >> doesFieldNameExist)
|> List.where (snd >> fieldNameExists)
|> List.map getFieldHint

| _ -> []
else
[]
4 changes: 3 additions & 1 deletion vsintegration/src/FSharp.Editor/Hints/InlineReturnTypeHints.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ type InlineReturnTypeHints(parseFileResults: FSharpParseFileResults, symbol: FSh
Parts = parts
})

let isValidForHint (symbol: FSharpMemberOrFunctionOrValue) = symbol.IsFunction

member _.getHints(symbolUse: FSharpSymbolUse) =
[
if symbol.IsFunction then
if isValidForHint symbol then
yield!
parseFileResults.TryRangeOfReturnTypeHint symbolUse.Range.Start
|> Option.bind (getHint symbolUse)
Expand Down
18 changes: 11 additions & 7 deletions vsintegration/src/FSharp.Editor/Hints/InlineTypeHints.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ open FSharp.Compiler.Text
open FSharp.Compiler.Text.Position
open Hints

module InlineTypeHints =
type InlineTypeHints(parseResults: FSharpParseFileResults, symbol: FSharpMemberOrFunctionOrValue) =

let private getHintParts (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =
let getHintParts (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =

match symbol.GetReturnTypeLayout symbolUse.DisplayContext with
| Some typeInfo ->
Expand All @@ -21,14 +21,14 @@ module InlineTypeHints =
// not sure when this can happen
| None -> []

let private getHint symbol (symbolUse: FSharpSymbolUse) =
let getHint symbol (symbolUse: FSharpSymbolUse) =
{
Kind = HintKind.TypeHint
Range = symbolUse.Range.EndRange
Parts = getHintParts symbol symbolUse
}

let private isSolved (symbol: FSharpMemberOrFunctionOrValue) =
let isSolved (symbol: FSharpMemberOrFunctionOrValue) =
if symbol.GenericParameters.Count > 0 then
symbol.GenericParameters |> Seq.forall (fun p -> p.IsSolveAtCompileTime)

Expand All @@ -38,7 +38,7 @@ module InlineTypeHints =
else
true

let isValidForHint (parseFileResults: FSharpParseFileResults) (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =
let isValidForHint (symbolUse: FSharpSymbolUse) =

let isOptionalParameter =
symbolUse.IsFromDefinition
Expand All @@ -53,7 +53,7 @@ module InlineTypeHints =
symbolUse.Range.Start

let isNotAnnotatedManually =
not (parseFileResults.IsTypeAnnotationGivenAtPosition adjustedRangeStart)
not (parseResults.IsTypeAnnotationGivenAtPosition adjustedRangeStart)

let isNotAfterDot = symbolUse.IsFromDefinition && not symbol.IsMemberThisValue

Expand All @@ -65,4 +65,8 @@ module InlineTypeHints =
&& isNotAfterDot
&& isNotTypeAlias

let getHints symbol symbolUse = [ getHint symbol symbolUse ]
member _.getHints symbolUse =
[
if isValidForHint symbolUse then
getHint symbol symbolUse
]