From 0f7d51980beaac692d68446b4816651eef304b87 Mon Sep 17 00:00:00 2001 From: cartermp Date: Tue, 26 Jan 2021 17:59:19 -0800 Subject: [PATCH 1/2] Fix issue where sighelp triggered in lambda gave outer signature help --- src/fsharp/service/ServiceUntypedParse.fs | 36 +++++++++++++++---- tests/service/ServiceUntypedParseTests.fs | 35 ++++++++++++++++++ .../FSharp.Editor/Completion/SignatureHelp.fs | 2 ++ 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/src/fsharp/service/ServiceUntypedParse.fs b/src/fsharp/service/ServiceUntypedParse.fs index 9a9d83f059a..a1139b58c03 100755 --- a/src/fsharp/service/ServiceUntypedParse.fs +++ b/src/fsharp/service/ServiceUntypedParse.fs @@ -170,14 +170,18 @@ type FSharpParseFileResults(errors: FSharpDiagnostic[], input: ParsedInput optio member _.VisitExpr(_, _, defaultTraverse, expr) = match expr with | SynExpr.App (_, _, SynExpr.App(_, true, SynExpr.Ident ident, _, _), argExpr, _) when rangeContainsPos argExpr.Range pos -> - if ident.idText = "op_PipeRight" then - Some (ident, 1) - elif ident.idText = "op_PipeRight2" then - Some (ident, 2) - elif ident.idText = "op_PipeRight3" then - Some (ident, 3) - else + match argExpr with + | SynExpr.App(_, _, _, SynExpr.Paren(expr, _, _, _), _) when rangeContainsPos expr.Range pos -> None + | _ -> + if ident.idText = "op_PipeRight" then + Some (ident, 1) + elif ident.idText = "op_PipeRight2" then + Some (ident, 2) + elif ident.idText = "op_PipeRight3" then + Some (ident, 3) + else + None | _ -> defaultTraverse expr }) | None -> None @@ -216,6 +220,13 @@ type FSharpParseFileResults(errors: FSharpDiagnostic[], input: ParsedInput optio match argExpr with | SynExpr.App (_, _, _, _, range) when rangeContainsPos range pos -> getIdentRangeForFuncExprInApp traverseSynExpr argExpr pos + + | SynExpr.Paren(SynExpr.Lambda(_, _, _args, body, _, _), _, _, _) when rangeContainsPos body.Range pos -> + getIdentRangeForFuncExprInApp traverseSynExpr body pos + + // Figure out a way to generalize this around parens? + //| SynExpr.Paren(expr, _, _, _) when rangeContainsPos expr.Range pos -> + // getIdentRangeForFuncExprInApp traverseSynExpr expr pos | _ -> match funcExpr with | SynExpr.App (_, true, _, _, _) when rangeContainsPos argExpr.Range pos -> @@ -227,6 +238,17 @@ type FSharpParseFileResults(errors: FSharpDiagnostic[], input: ParsedInput optio // Generally, we want to dive into the func expr to get the range // of the identifier of the function we're after getIdentRangeForFuncExprInApp traverseSynExpr funcExpr pos + + | SynExpr.LetOrUse(_, _, bindings, body, range) when rangeContainsPos range pos -> + let binding = + bindings + |> List.tryFind (fun x -> rangeContainsPos x.RangeOfBindingAndRhs pos) + match binding with + | Some(SynBinding.Binding(_, _, _, _, _, _, _, _, _, expr, _, _)) -> + getIdentRangeForFuncExprInApp traverseSynExpr expr pos + | None -> + getIdentRangeForFuncExprInApp traverseSynExpr body pos + | expr -> traverseSynExpr expr |> Option.map (fun expr -> expr) diff --git a/tests/service/ServiceUntypedParseTests.fs b/tests/service/ServiceUntypedParseTests.fs index 3c87e8f7973..5d00a4f4400 100644 --- a/tests/service/ServiceUntypedParseTests.fs +++ b/tests/service/ServiceUntypedParseTests.fs @@ -835,6 +835,26 @@ async { |> tups |> shouldEqual ((4, 11), (4, 16)) + [] + let ``TryRangeOfFunctionOrMethodBeingApplied - inside lambda``() = + let source = """ +let add n1 n2 = n1 + n2 +let lst = [1; 2; 3] +let mapped = + lst |> List.map (fun n -> + let sum = add + n.ToString() + ) +""" + let parseFileResults, _ = getParseAndCheckResults source + let res = parseFileResults.TryRangeOfFunctionOrMethodBeingApplied (mkPos 6 21) + match res with + | None -> Assert.Fail("Expected 'add' but got nothing") + | Some range -> + range + |> tups + |> shouldEqual ((6, 18), (6, 21)) + module PipelinesAndArgs = [] let ``TryIdentOfPipelineContainingPosAndNumArgsApplied - No pipeline, no infix app``() = @@ -897,6 +917,21 @@ let square x = x * | None -> Assert.Fail("No pipeline found") + [] + let ``TryIdentOfPipelineContainingPosAndNumArgsApplied - none when inside lambda``() = + let source = """ +let add n1 n2 = n1 + n2 +let lst = [1; 2; 3] +let mapped = + lst |> List.map (fun n -> + let sum = add 1 + n.ToString() + ) + """ + let parseFileResults, _ = getParseAndCheckResults source + let res = parseFileResults.TryIdentOfPipelineContainingPosAndNumArgsApplied (mkPos 6 22) + Assert.IsTrue(res.IsNone, "Inside a lambda but counted the pipeline outside of that lambda.") + [] let ``TryRangeOfExprInYieldOrReturn - not contained``() = let source = """ diff --git a/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs b/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs index 650d4ab25e4..b7cc5ac5595 100644 --- a/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs +++ b/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs @@ -251,6 +251,8 @@ type internal FSharpSignatureHelpProvider let! symbolUse = checkFileResults.GetSymbolUseAtLocation(fcsTextLineNumber, lexerSymbol.Ident.idRange.EndColumn, textLineText, lexerSymbol.FullIsland) let isValid (mfv: FSharpMemberOrFunctionOrValue) = + // TODO check if it is a keyword - nested inside parens case! + // TODO check classification data similar to completion provider not (PrettyNaming.IsOperatorName mfv.DisplayName) && not mfv.IsProperty && mfv.CurriedParameterGroups.Count > 0 From 6442b405ffa796a1de646b970d30ab82da80b6c0 Mon Sep 17 00:00:00 2001 From: cartermp Date: Tue, 26 Jan 2021 20:45:13 -0800 Subject: [PATCH 2/2] Updates --- src/fsharp/service/ServiceUntypedParse.fs | 3 --- vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/fsharp/service/ServiceUntypedParse.fs b/src/fsharp/service/ServiceUntypedParse.fs index a1139b58c03..626898a9db2 100755 --- a/src/fsharp/service/ServiceUntypedParse.fs +++ b/src/fsharp/service/ServiceUntypedParse.fs @@ -224,9 +224,6 @@ type FSharpParseFileResults(errors: FSharpDiagnostic[], input: ParsedInput optio | SynExpr.Paren(SynExpr.Lambda(_, _, _args, body, _, _), _, _, _) when rangeContainsPos body.Range pos -> getIdentRangeForFuncExprInApp traverseSynExpr body pos - // Figure out a way to generalize this around parens? - //| SynExpr.Paren(expr, _, _, _) when rangeContainsPos expr.Range pos -> - // getIdentRangeForFuncExprInApp traverseSynExpr expr pos | _ -> match funcExpr with | SynExpr.App (_, true, _, _, _) when rangeContainsPos argExpr.Range pos -> diff --git a/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs b/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs index b7cc5ac5595..650d4ab25e4 100644 --- a/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs +++ b/vsintegration/src/FSharp.Editor/Completion/SignatureHelp.fs @@ -251,8 +251,6 @@ type internal FSharpSignatureHelpProvider let! symbolUse = checkFileResults.GetSymbolUseAtLocation(fcsTextLineNumber, lexerSymbol.Ident.idRange.EndColumn, textLineText, lexerSymbol.FullIsland) let isValid (mfv: FSharpMemberOrFunctionOrValue) = - // TODO check if it is a keyword - nested inside parens case! - // TODO check classification data similar to completion provider not (PrettyNaming.IsOperatorName mfv.DisplayName) && not mfv.IsProperty && mfv.CurriedParameterGroups.Count > 0