From 09e8b3453a8e9b7d88e8381f637c287ca5bc8583 Mon Sep 17 00:00:00 2001 From: Petr Date: Fri, 6 Jan 2023 18:59:19 +0100 Subject: [PATCH 1/4] Don't show parameter name hints when they coincide with argument names --- .../src/FSharp.Editor/Hints/HintService.fs | 19 ++++++------- .../Hints/InlineParameterNameHints.fs | 17 ++++++++++++ .../src/FSharp.Editor/Hints/RoslynAdapter.fs | 1 + .../Hints/HintTestFramework.fs | 6 +++-- .../Hints/InlineParameterNameHintTests.fs | 27 ++++++++++++++++++- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Hints/HintService.fs b/vsintegration/src/FSharp.Editor/Hints/HintService.fs index f196e3caac5..349ec56ee3e 100644 --- a/vsintegration/src/FSharp.Editor/Hints/HintService.fs +++ b/vsintegration/src/FSharp.Editor/Hints/HintService.fs @@ -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 @@ -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 @@ -36,7 +37,7 @@ 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 @@ -44,23 +45,23 @@ module HintService = 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 } diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs index db8298d826f..254d8a46464 100644 --- a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs +++ b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs @@ -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 @@ -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]}" + let length = range.EndColumn - range.StartColumn + line.Substring(range.Start.Column, length) + let isMemberOrFunctionOrValueValidForHint (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) = @@ -83,6 +92,7 @@ module InlineParameterNameHints = && symbol.DisplayName <> "(::)" let getHintsForMemberOrFunctionOrValue + (sourceText: SourceText) (parseResults: FSharpParseFileResults) (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) = @@ -97,9 +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.zip argumentNames + |> Seq.where (fun (argumentName, (_, parameter)) -> argumentName <> parameter.DisplayName) + |> Seq.map snd |> Seq.map getParameterHint |> Seq.toList diff --git a/vsintegration/src/FSharp.Editor/Hints/RoslynAdapter.fs b/vsintegration/src/FSharp.Editor/Hints/RoslynAdapter.fs index b6026d6da44..f79e1dba1fa 100644 --- a/vsintegration/src/FSharp.Editor/Hints/RoslynAdapter.fs +++ b/vsintegration/src/FSharp.Editor/Hints/RoslynAdapter.fs @@ -30,6 +30,7 @@ type internal RoslynAdapter let! sourceText = document.GetTextAsync cancellationToken |> Async.AwaitTask let! nativeHints = HintService.getHintsForDocument + sourceText document hintKinds userOpName diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs index f009feba036..259c78acfc7 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs @@ -3,6 +3,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 @@ -40,9 +41,10 @@ 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! sourceText = document.GetTextAsync CancellationToken.None |> Async.AwaitTask + let! hints = HintService.getHintsForDocument sourceText document hintKinds "test" CancellationToken.None return hints |> Seq.map convert } |> Async.RunSynchronously diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs index 60b7fa0f0e4..23d6c48c150 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs @@ -491,4 +491,29 @@ let q = query { for x in { 1 .. 10 } do select x } let actual = getParameterNameHints document - Assert.IsEmpty actual \ No newline at end of file + Assert.IsEmpty actual + + [] + 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) \ No newline at end of file From 5547c965852b4a17d85371604ab996a5f805679a Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 11 Jan 2023 13:31:59 +0100 Subject: [PATCH 2/4] Update InlineParameterNameHints.fs --- .../src/FSharp.Editor/Hints/InlineParameterNameHints.fs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs index 254d8a46464..32af341e583 100644 --- a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs +++ b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs @@ -65,7 +65,7 @@ module InlineParameterNameHints = (sourceText: SourceText) (range: range) = - let line = $"{sourceText.Lines[range.Start.Line - 1]}" + let line = sourceText.Lines[range.Start.Line - 1].ToString() let length = range.EndColumn - range.StartColumn line.Substring(range.Start.Column, length) @@ -115,9 +115,8 @@ module InlineParameterNameHints = |> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths |> Seq.where (snd >> doesParameterNameExist) |> Seq.zip argumentNames - |> Seq.where (fun (argumentName, (_, parameter)) -> argumentName <> parameter.DisplayName) - |> Seq.map snd - |> Seq.map getParameterHint + |> Seq.choose (fun (argumentName, (range, parameter)) -> + if argumentName <> parameter.DisplayName then Some (getParameterHint (range, parameter)) else None) |> Seq.toList let getHintsForUnionCase From dc2bf0389fe8ea081c573f43a53e98d757c11c45 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 11 Jan 2023 13:36:34 +0100 Subject: [PATCH 3/4] Update HintTestFramework.fs --- .../tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs index 259c78acfc7..70ab8900c05 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs @@ -2,7 +2,6 @@ namespace FSharp.Editor.Tests.Hints -open System.Threading open Microsoft.CodeAnalysis open Microsoft.VisualStudio.FSharp.Editor open Microsoft.VisualStudio.FSharp.Editor.Hints @@ -43,8 +42,8 @@ module HintTestFramework = let getHints (document: Document) hintKinds = async { - let! sourceText = document.GetTextAsync CancellationToken.None |> Async.AwaitTask - let! hints = HintService.getHintsForDocument sourceText document hintKinds "test" CancellationToken.None + let! sourceText = document.GetTextAsync Async.DefaultCancellationToken |> Async.AwaitTask + let! hints = HintService.getHintsForDocument sourceText document hintKinds "test" Async.DefaultCancellationToken return hints |> Seq.map convert } |> Async.RunSynchronously From cf6865566956097d1ba6de7154274f7e0fc85468 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 11 Jan 2023 14:26:20 +0100 Subject: [PATCH 4/4] Update HintTestFramework.fs --- .../tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs index 70ab8900c05..a542e12d790 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs @@ -42,8 +42,9 @@ module HintTestFramework = let getHints (document: Document) hintKinds = async { - let! sourceText = document.GetTextAsync Async.DefaultCancellationToken |> Async.AwaitTask - let! hints = HintService.getHintsForDocument sourceText document hintKinds "test" Async.DefaultCancellationToken + 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