diff --git a/src/FsAutoComplete.Core/FileSystem.fs b/src/FsAutoComplete.Core/FileSystem.fs index 550dbe93b..b5997d0c2 100644 --- a/src/FsAutoComplete.Core/FileSystem.fs +++ b/src/FsAutoComplete.Core/FileSystem.fs @@ -140,7 +140,7 @@ type NamedText(fileName: string, str: string) = if pos.Column = 0 then return! None else let lineIndex = pos.Column - 1 - if lineText.Length < lineIndex + if lineText.Length <= lineIndex then return! None else diff --git a/src/FsAutoComplete/CodeFixes/AddExplicitTypeToParameter.fs b/src/FsAutoComplete/CodeFixes/AddExplicitTypeToParameter.fs index 3f6e41b1d..dbe6b363a 100644 --- a/src/FsAutoComplete/CodeFixes/AddExplicitTypeToParameter.fs +++ b/src/FsAutoComplete/CodeFixes/AddExplicitTypeToParameter.fs @@ -1,15 +1,52 @@ module FsAutoComplete.CodeFix.AddExplicitTypeToParameter open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix.Navigation open FsAutoComplete.CodeFix.Types open Ionide.LanguageServerProtocol.Types open FsAutoComplete open FsAutoComplete.LspHelpers open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Symbols -open FsAutoComplete.FCSPatches open FSharp.Compiler.Syntax +open FSharp.Compiler.Text.Range + +let private isPositionContainedInUntypedImplicitCtorParameter input pos = + let result = + SyntaxTraversal.Traverse(pos, input, { new SyntaxVisitorBase<_>() with + member _.VisitModuleDecl(_, defaultTraverse, decl) = + match decl with + | SynModuleDecl.Types(typeDefns = typeDefns) -> + maybe { + let! ctorArgs = + typeDefns + |> List.tryPick ( + function + | SynTypeDefn(implicitConstructor=Some(SynMemberDefn.ImplicitCtor(ctorArgs = args))) when rangeContainsPos args.Range pos -> + Some args + | _ -> None + ) + + match ctorArgs with + | SynSimplePats.SimplePats (pats=pats) -> + let! pat = + pats + |> List.tryFind (fun pat -> rangeContainsPos pat.Range pos) + let rec tryGetUntypedIdent = + function + | SynSimplePat.Id (ident=ident) when rangeContainsPos ident.idRange pos -> + Some ident + | SynSimplePat.Attrib (pat=pat) when rangeContainsPos pat.Range pos -> + tryGetUntypedIdent pat + | SynSimplePat.Typed _ + | _ -> + None + return! tryGetUntypedIdent pat + | _ -> return! None + } + |> Option.orElseWith (fun _ -> defaultTraverse decl) + | _ -> defaultTraverse decl + }) + result.IsSome let title = "Add explicit type annotation" let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix = @@ -31,12 +68,14 @@ let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix = funcOrValue.IsFunction && parseFileResults.IsBindingALambdaAtPosition symbolUse.Range.Start - let IsPositionContainedInACurriedParameter = parseFileResults.IsPositionContainedInACurriedParameter symbolUse.Range.End - let IsTypeAnnotationGivenAtPosition = parseFileResults.IsTypeAnnotationGivenAtPosition symbolUse.Range.End - (funcOrValue.IsValue || isLambdaIfFunction) && - IsPositionContainedInACurriedParameter && - not IsTypeAnnotationGivenAtPosition && + ( + ( + parseFileResults.IsPositionContainedInACurriedParameter symbolUse.Range.Start && + not (parseFileResults.IsTypeAnnotationGivenAtPosition symbolUse.Range.Start) + ) || + (isPositionContainedInUntypedImplicitCtorParameter parseFileResults.ParseTree symbolUse.Range.Start) + ) && not funcOrValue.IsMember && not funcOrValue.IsMemberThisValue && not funcOrValue.IsConstructorThisValue && @@ -50,15 +89,34 @@ let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix = let protocolSymbolRange = fcsRangeToLsp fcsSymbolRange let! symbolText = sourceText.GetText fcsSymbolRange - let alreadyWrappedInParens = - let hasLeftParen = Navigation.walkBackUntilConditionWithTerminal sourceText protocolSymbolRange.Start (fun c -> c = '(') System.Char.IsWhiteSpace - let hasRightParen = Navigation.walkForwardUntilConditionWithTerminal sourceText protocolSymbolRange.End (fun c -> c = ')') System.Char.IsWhiteSpace - hasLeftParen.IsSome && hasRightParen.IsSome + let requiresParens = + if isPositionContainedInUntypedImplicitCtorParameter parseFileResults.ParseTree symbolUse.Range.Start then + // no patterns in primary ctor allowed -> `type A((a))` is invalid + false + else + // `(a, b, c)` + // -> between `,` and parens (there might be spaces between) + let left = + sourceText.WalkBackwards(fcsSymbolRange.Start, (fun _ -> false), ((<>) ' ')) + |> Option.bind (sourceText.TryGetChar) + let right = + sourceText.NextPos fcsSymbolRange.End // end is on last char of identifier + |> Option.bind (fun pos -> sourceText.WalkForward(pos, (fun _ -> false), ((<>) ' '))) + |> Option.bind (sourceText.TryGetChar) + + match left, right with + | Some left, Some right -> + let isContained = + (left = '(' || left = ',') + && + (right = ',' || right = ')') + not isContained + | _, _ -> true let changedText, changedRange = - if alreadyWrappedInParens - then ": " + typeString, { Start = protocolSymbolRange.End; End = protocolSymbolRange.End } - else "(" + symbolText + ": " + typeString + ")", protocolSymbolRange + if requiresParens + then "(" + symbolText + ": " + typeString + ")", protocolSymbolRange + else ": " + typeString, { Start = protocolSymbolRange.End; End = protocolSymbolRange.End } return [ { Edits = [| { Range = changedRange; NewText = changedText } |] File = codeActionParams.TextDocument diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index f5d460afc..11093c719 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -43,6 +43,7 @@ module CodeFix = let private addExplicitTypeToParameterTests state = serverTestList (nameof AddExplicitTypeToParameter) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle AddExplicitTypeToParameter.title testCaseAsync "can suggest explicit parameter for record-typed function parameters" <| CodeFix.check server """ @@ -53,7 +54,7 @@ let private addExplicitTypeToParameterTests state = f.name """ (Diagnostics.acceptAll) - (CodeFix.withTitle AddExplicitTypeToParameter.title) + selectCodeFix """ type Foo = { name: string } @@ -61,6 +62,343 @@ let private addExplicitTypeToParameterTests state = let name (f: Foo) = f.name """ + testCaseAsync "can add type for int param" <| + CodeFix.check server + """ + let f ($0x) = x + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x: int) = x + 1 + """ + testCaseAsync "can add type for generic param" <| + CodeFix.check server + """ + let f ($0x) = () + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x: 'a) = () + """ + testCaseAsync "doesn't trigger when existing type" <| + CodeFix.checkNotApplicable server + """ + let f ($0x: int) = () + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to tuple item" <| + CodeFix.check server + """ + let f (a, $0b, c) = a + b + c + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (a, b: int, c) = a + b + c + 1 + """ + testCaseAsync "doesn't trigger in tuple when existing type" <| + CodeFix.checkNotApplicable server + """ + let f (a, $0b: int, c) = a + b + c + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to 2nd of 3 param" <| + CodeFix.check server + """ + let f a $0b c = a + b + c + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f a (b: int) c = a + b + c + 1 + """ + testCaseAsync "doesn't trigger on 2nd of 3 param when existing type" <| + CodeFix.checkNotApplicable server + """ + let f a ($0b: int) c = a + b + c + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to 2nd of 3 param when other params have types" <| + CodeFix.check server + """ + let f (a: int) $0b (c: int) = a + b + c + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (a: int) (b: int) (c: int) = a + b + c + 1 + """ + testCaseAsync "can add type to member param" <| + CodeFix.check server + """ + type A() = + member _.F($0a) = a + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + type A() = + member _.F(a: int) = a + 1 + """ + testCaseAsync "doesn't trigger for member param when existing type" <| + CodeFix.checkNotApplicable server + """ + type A() = + member _.F($0a: int) = a + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to ctor param" <| + CodeFix.check server + """ + type A($0a) = + member _.F() = a + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + type A(a: int) = + member _.F() = a + 1 + """ + testCaseAsync "doesn't trigger for ctor param when existing type" <| + CodeFix.checkNotApplicable server + """ + type A($0a: int) = + member _.F() = a + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to correct ctor param" <| + CodeFix.check server + """ + type A(str, $0n, b) = + member _.FString() = sprintf "str=%s" str + member _.FInt() = n + 1 + member _.FBool() = sprintf "b=%b" b + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + type A(str, n: int, b) = + member _.FString() = sprintf "str=%s" str + member _.FInt() = n + 1 + member _.FBool() = sprintf "b=%b" b + """ + testCaseAsync "doesn't trigger for ctor param when existing type and multiple params" <| + CodeFix.checkNotApplicable server + """ + type A(str, $0n: int, b) = + member _.FString() = sprintf "str=%s" str + member _.FInt() = a + 1 + member _.FBool() = sprintf "b=%b" b + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "can add type to secondary ctor param" <| + CodeFix.check server + """ + type A(a) = + new($0a, b) = A(a+b) + member _.F() = a + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + type A(a) = + new(a: int, b) = A(a+b) + member _.F() = a + 1 + """ + testList "parens" [ + testCaseAsync "single param without parens -> add parens" <| + CodeFix.check server + """ + let f $0x = x + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x: int) = x + 1 + """ + testCaseAsync "single param with parens -> keep parens" <| + CodeFix.check server + """ + let f ($0x) = x + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x: int) = x + 1 + """ + testCaseAsync "multi params without parens -> add parens" <| + CodeFix.check server + """ + let f a $0x y = x + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f a (x: int) y = x + 1 + """ + testCaseAsync "multi params with parens -> keep parens" <| + CodeFix.check server + """ + let f a ($0x) y = x + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f a (x: int) y = x + 1 + """ + testList "tuple params without parens -> no parens" [ + testCaseAsync "start" <| + CodeFix.check server + """ + let f ($0x, y, z) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x: int, y, z) = x + y + z + 1 + """ + testCaseAsync "center" <| + CodeFix.check server + """ + let f (x, $0y, z) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x, y: int, z) = x + y + z + 1 + """ + testCaseAsync "end" <| + CodeFix.check server + """ + let f (x, y, $0z) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x, y, z: int) = x + y + z + 1 + """ + ] + testList "tuple params with parens -> keep parens" [ + testCaseAsync "start" <| + CodeFix.check server + """ + let f (($0x), y, z) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ((x: int), y, z) = x + y + z + 1 + """ + testCaseAsync "center" <| + CodeFix.check server + """ + let f (x, ($0y), z) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x, (y: int), z) = x + y + z + 1 + """ + testCaseAsync "end" <| + CodeFix.check server + """ + let f (x, y, ($0z)) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f (x, y, (z: int)) = x + y + z + 1 + """ + ] + testList "tuple params without parens but spaces -> no parens" [ + testCaseAsync "start" <| + CodeFix.check server + """ + let f ( $0x , y , z ) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( x: int , y , z ) = x + y + z + 1 + """ + testCaseAsync "center" <| + CodeFix.check server + """ + let f ( x , $0y , z ) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( x , y: int , z ) = x + y + z + 1 + """ + testCaseAsync "end" <| + CodeFix.check server + """ + let f ( x , y , $0z ) = x + y + z + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( x , y , z: int ) = x + y + z + 1 + """ + ] + testList "long tuple params without parens but spaces -> no parens" [ + testCaseAsync "start" <| + CodeFix.check server + """ + let f ( xV$0alue , yAnotherValue , zFinalValue ) = xValue + yAnotherValue + zFinalValue + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( xValue: int , yAnotherValue , zFinalValue ) = xValue + yAnotherValue + zFinalValue + 1 + """ + testCaseAsync "center" <| + CodeFix.check server + """ + let f ( xValue , yAn$0otherValue , zFinalValue ) = xValue + yAnotherValue + zFinalValue + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( xValue , yAnotherValue: int , zFinalValue ) = xValue + yAnotherValue + zFinalValue + 1 + """ + testCaseAsync "end" <| + CodeFix.check server + """ + let f ( xValue , yAnotherValue , zFina$0lValue ) = xValue + yAnotherValue + zFinalValue + 1 + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + let f ( xValue , yAnotherValue , zFinalValue: int ) = xValue + yAnotherValue + zFinalValue + 1 + """ + ] + testCaseAsync "never add parens to primary ctor param" <| + CodeFix.check server + """ + type A ( + $0a + ) = + member _.F(b) = a + b + """ + (Diagnostics.acceptAll) + selectCodeFix + """ + type A ( + a: int + ) = + member _.F(b) = a + b + """ + ] ]) let private addMissingEqualsToTypeDefinitionTests state = diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index 5106903b7..305d6906e 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -42,7 +42,8 @@ let lspTests = [ for (name, workspaceLoaderFactory) in loaders do testList name - [ Templates.tests () + [ + Templates.tests () let testRunDir = Path.Combine(Path.GetTempPath(), "FsAutoComplete.Tests", Guid.NewGuid().ToString()) |> DirectoryInfo let state () = FsAutoComplete.State.Initial toolsPath testRunDir workspaceLoaderFactory