Skip to content
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.

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

Don't show parameter name hints when they coincide with argument names #14581

Merged
merged 5 commits into from
Jan 11, 2023
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
19 changes: 10 additions & 9 deletions vsintegration/src/FSharp.Editor/Hints/HintService.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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
Expand All @@ -16,15 +17,15 @@ module HintService =
Seq.filter (InlineTypeHints.isValidForHint parseResults symbol)
>> Seq.collect (InlineTypeHints.getHints symbol)

let inline private getHintsForMemberOrFunctionOrValue parseResults symbol: NativeHintResolver =
let inline private getHintsForMemberOrFunctionOrValue (sourceText: SourceText) parseResults symbol: NativeHintResolver =
Seq.filter (InlineParameterNameHints.isMemberOrFunctionOrValueValidForHint symbol)
>> Seq.collect (InlineParameterNameHints.getHintsForMemberOrFunctionOrValue parseResults 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 parseResults hintKinds (symbol: FSharpSymbol): NativeHintResolver seq =
let private getHintResolvers (sourceText: SourceText) parseResults hintKinds (symbol: FSharpSymbol): NativeHintResolver seq =
let rec resolve hintKinds resolvers =
match hintKinds with
| [] -> resolvers |> Seq.choose id
Expand All @@ -36,31 +37,31 @@ module HintService =
| _ -> None
| HintKind.ParameterNameHint ->
match symbol with
| :? FSharpMemberOrFunctionOrValue as symbol -> getHintsForMemberOrFunctionOrValue parseResults symbol |> Some
| :? 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

in resolve hintKinds []

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

let getHintsForDocument (document: Document) hintKinds userOpName cancellationToken =
let getHintsForDocument sourceText (document: Document) hintKinds userOpName cancellationToken =
async {
if isSignatureFile document.FilePath
then
return []
else
let! parseResults, checkResults =
document.GetFSharpParseAndCheckResultsAsync userOpName
document.GetFSharpParseAndCheckResultsAsync userOpName

return
checkResults.GetAllUsesOfAllSymbolsInFile cancellationToken
|> Seq.groupBy (fun symbolUse -> symbolUse.Symbol)
|> Seq.collect (getHintsForSymbol parseResults (hintKinds |> Set.toList))
|> Seq.collect (getHintsForSymbol sourceText parseResults (hintKinds |> Set.toList))
|> Seq.toList
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Microsoft.VisualStudio.FSharp.Editor.Hints

open Microsoft.CodeAnalysis.Text
open Microsoft.VisualStudio.FSharp.Editor
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.EditorServices
Expand Down Expand Up @@ -60,6 +61,14 @@ module InlineParameterNameHints =
>> Seq.map (fun location -> location.ArgumentRange)
>> Seq.contains range

let private getSourceTextAtRange
(sourceText: SourceText)
(range: range) =

let line = sourceText.Lines[range.Start.Line - 1].ToString()
let length = range.EndColumn - range.StartColumn
line.Substring(range.Start.Column, length)

let isMemberOrFunctionOrValueValidForHint
(symbol: FSharpMemberOrFunctionOrValue)
(symbolUse: FSharpSymbolUse) =
Expand All @@ -83,6 +92,7 @@ module InlineParameterNameHints =
&& symbol.DisplayName <> "(::)"

let getHintsForMemberOrFunctionOrValue
(sourceText: SourceText)
(parseResults: FSharpParseFileResults)
(symbol: FSharpMemberOrFunctionOrValue)
(symbolUse: FSharpSymbolUse) =
Expand All @@ -97,10 +107,16 @@ module InlineParameterNameHints =
if tupleRanges |> (not << Seq.isEmpty) then tupleRanges else curryRanges
|> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))

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.map getParameterHint
|> Seq.zip argumentNames
|> Seq.choose (fun (argumentName, (range, parameter)) ->
if argumentName <> parameter.DisplayName then Some (getParameterHint (range, parameter)) else None)
|> Seq.toList

let getHintsForUnionCase
Expand Down
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/Hints/RoslynAdapter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type internal RoslynAdapter
let! sourceText = document.GetTextAsync cancellationToken |> Async.AwaitTask
let! nativeHints =
HintService.getHintsForDocument
sourceText
document
hintKinds
userOpName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace FSharp.Editor.Tests.Hints

open System.Threading
open Microsoft.CodeAnalysis
open Microsoft.VisualStudio.FSharp.Editor
open Microsoft.VisualStudio.FSharp.Editor.Hints
open Microsoft.CodeAnalysis.Text
Expand Down Expand Up @@ -40,9 +40,11 @@ module HintTestFramework =
let getFsiAndFsDocuments (fsiCode: string) (fsCode: string) =
RoslynTestHelpers.CreateTwoDocumentSolution("test.fsi", SourceText.From fsiCode, "test.fs", SourceText.From fsCode)

let getHints document hintKinds =
let getHints (document: Document) hintKinds =
async {
let! hints = HintService.getHintsForDocument document hintKinds "test" CancellationToken.None
let! ct = Async.CancellationToken
let! sourceText = document.GetTextAsync ct |> Async.AwaitTask
let! hints = HintService.getHintsForDocument sourceText document hintKinds "test" ct
return hints |> Seq.map convert
}
|> Async.RunSynchronously
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,4 +491,29 @@ let q = query { for x in { 1 .. 10 } do select x }

let actual = getParameterNameHints document

Assert.IsEmpty actual
Assert.IsEmpty actual

[<Test>]
let ``Hints are not shown when parameter names coinside with variable names`` () =
let code =
"""
let getFullName name surname = $"{name} {surname}"

let name = "Robert"
let lastName = "Smith"
let fullName = getFullName name lastName
"""

let document = getFsDocument code

let expected =
[
{
Content = "surname = "
Location = (5, 33)
}
]

let actual = getParameterNameHints document

Assert.AreEqual(expected, actual)