From 2b8f3dd9dcd4f12da7f9153226057ea07ac1932a Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Thu, 7 Apr 2022 21:40:35 +0200 Subject: [PATCH 01/29] Sort Code Fix Tests alphabetically --- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 416 +++++++++--------- 1 file changed, 209 insertions(+), 207 deletions(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index ef6b6efcc..585bc1554 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -40,6 +40,139 @@ module CodeFix = ) codeActions + +let private addExplicitTypeToParameterTests state = + serverTestList (nameof AddExplicitTypeToParameter) state defaultConfigDto None (fun server -> [ + testCaseAsync "can suggest explicit parameter for record-typed function parameters" <| + CodeFix.check server + """ + type Foo = + { name: string } + + let name $0f = + f.name + """ + (Diagnostics.acceptAll) + (CodeFix.withTitle AddExplicitTypeToParameter.title) + """ + type Foo = + { name: string } + + let name (f: Foo) = + f.name + """ + ]) + +let private addMissingFunKeywordTests state = + serverTestList (nameof AddMissingFunKeyword) state defaultConfigDto None (fun server -> [ + testCaseAsync "can generate the fun keyword when error 10 is raised" <| + CodeFix.check server + """ + let doThing = x $0-> printfn "%s" x + """ + (Diagnostics.expectCode "10") + (CodeFix.ofKind "quickfix" >> CodeFix.withTitle AddMissingFunKeyword.title) + """ + let doThing = fun x -> printfn "%s" x + """ + ]) + +let private addMissingInstanceMemberTests state = + serverTestList (nameof AddMissingInstanceMember) state defaultConfigDto None (fun server -> [ + testCaseAsync "can add this member prefix" <| + CodeFix.check server + """ + type C () = + member $0Foo() = () + """ + (Diagnostics.expectCode "673") + (CodeFix.ofKind "quickfix" >> CodeFix.withTitle AddMissingInstanceMember.title) + """ + type C () = + member x.Foo() = () + """ + ]) + +let private changeTypeOfNameToNameOfTests state = + serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ + testCaseAsync "can suggest fix" <| + CodeFix.check server + """ + let x = $0typeof>.Name + """ + (Diagnostics.acceptAll) + (CodeFix.ofKind "refactor" >> CodeFix.withTitle ChangeTypeOfNameToNameOf.title) + """ + let x = nameof(Async) + """ + ]) + +let private convertPositionalDUToNamedTests state = + serverTestList (nameof ConvertPositionalDUToNamed) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ConvertPositionalDUToNamed.title + testCaseAsync "in parenthesized let binding" <| + CodeFix.check server + """ + type A = A of a: int * b: bool + + let (A(a$0, b)) = A(1, true) + """ + Diagnostics.acceptAll + selectCodeFix + """ + type A = A of a: int * b: bool + + let (A(a = a; b = b;)) = A(1, true) + """ + testCaseAsync "in simple match" <| + CodeFix.check server + """ + type A = A of a: int * b: bool + + match A(1, true) with + | A(a$0, b) -> () + """ + Diagnostics.acceptAll + selectCodeFix + """ + type A = A of a: int * b: bool + + match A(1, true) with + | A(a = a; b = b;) -> () + """ + testCaseAsync "in parenthesized match" <| + CodeFix.check server + """ + type A = A of a: int * b: bool + + match A(1, true) with + | (A(a$0, b)) -> () + """ + Diagnostics.acceptAll + selectCodeFix + """ + type A = A of a: int * b: bool + + match A(1, true) with + | (A(a = a; b = b;)) -> () + """ + testCaseAsync "when there are new fields on the DU" <| + //ENHANCEMENT: add space before wildcard case + CodeFix.check server + """ + type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char + + let (ThirdFieldWasJustAdded($0a, b)) = ThirdFieldWasJustAdded(1, true, 'c') + """ + Diagnostics.acceptAll + selectCodeFix + """ + type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char + + let (ThirdFieldWasJustAdded(a = a; b = b;c = _;)) = ThirdFieldWasJustAdded(1, true, 'c') + """ + ]) + let private generateAbstractClassStubTests state = let config = { defaultConfigDto with AbstractClassStubGeneration = Some true } // issue: returns same fix twice: @@ -66,6 +199,30 @@ let private generateAbstractClassStubTests state = selectCodeFix ]) +let private generateRecordStubTests state = + let config = + { defaultConfigDto with + RecordStubGeneration = Some true + RecordStubGenerationBody = Some "failwith \"---\"" + } + serverTestList (nameof GenerateRecordStub) state config None (fun server -> [ + CodeFix.testAllPositions "can generate record stubs for every pos in the record as soon as one field is known" + server + """ + type R = { a: string; b: int } + + let a = $0{ $0a = $0"";$0 }$0 + """ + (Diagnostics.expectCode "764") + (CodeFix.withTitle GenerateRecordStub.title) + """ + type R = { a: string; b: int } + + let a = { a = ""; + b = failwith "---" } + """ + ]) + let private generateUnionCasesTests state = let config = { defaultConfigDto with @@ -98,44 +255,6 @@ let private generateUnionCasesTests state = """ ]) -let private generateRecordStubTests state = - let config = - { defaultConfigDto with - RecordStubGeneration = Some true - RecordStubGenerationBody = Some "failwith \"---\"" - } - serverTestList (nameof GenerateRecordStub) state config None (fun server -> [ - CodeFix.testAllPositions "can generate record stubs for every pos in the record as soon as one field is known" - server - """ - type R = { a: string; b: int } - - let a = $0{ $0a = $0"";$0 }$0 - """ - (Diagnostics.expectCode "764") - (CodeFix.withTitle GenerateRecordStub.title) - """ - type R = { a: string; b: int } - - let a = { a = ""; - b = failwith "---" } - """ - ]) - -let private addMissingFunKeywordTests state = - serverTestList (nameof AddMissingFunKeyword) state defaultConfigDto None (fun server -> [ - testCaseAsync "can generate the fun keyword when error 10 is raised" <| - CodeFix.check server - """ - let doThing = x $0-> printfn "%s" x - """ - (Diagnostics.expectCode "10") - (CodeFix.ofKind "quickfix" >> CodeFix.withTitle AddMissingFunKeyword.title) - """ - let doThing = fun x -> printfn "%s" x - """ - ]) - let private makeOuterBindingRecursiveTests state = serverTestList (nameof MakeOuterBindingRecursive) state defaultConfigDto None (fun server -> [ testCaseAsync "can make the outer binding recursive when self-referential" <| @@ -158,93 +277,21 @@ let private makeOuterBindingRecursiveTests state = """ ]) -let private changeTypeOfNameToNameOfTests state = - serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ - testCaseAsync "can suggest fix" <| - CodeFix.check server - """ - let x = $0typeof>.Name - """ - (Diagnostics.acceptAll) - (CodeFix.ofKind "refactor" >> CodeFix.withTitle ChangeTypeOfNameToNameOf.title) - """ - let x = nameof(Async) - """ - ]) - -let private addMissingInstanceMemberTests state = - serverTestList (nameof AddMissingInstanceMember) state defaultConfigDto None (fun server -> [ - testCaseAsync "can add this member prefix" <| - CodeFix.check server - """ - type C () = - member $0Foo() = () - """ - (Diagnostics.expectCode "673") - (CodeFix.ofKind "quickfix" >> CodeFix.withTitle AddMissingInstanceMember.title) - """ - type C () = - member x.Foo() = () - """ - ]) - -let private unusedValueTests state = - let config = { defaultConfigDto with UnusedDeclarationsAnalyzer = Some true } - serverTestList (nameof UnusedValue) state config None (fun server -> [ - let selectReplace = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titleReplace - let selectPrefix = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titlePrefix - - testCaseAsync "can replace unused self-reference" <| - CodeFix.check server - """ - type MyClass() = - member $0this.DoAThing() = () - """ - (Diagnostics.acceptAll) - selectReplace - """ - type MyClass() = - member _.DoAThing() = () - """ - testCaseAsync "can replace unused binding" <| - CodeFix.check server - """ - let $0six = 6 - """ - (Diagnostics.acceptAll) - selectReplace - """ - let _ = 6 - """ - testCaseAsync "can prefix unused binding" <| - CodeFix.check server - """ - let $0six = 6 - """ - (Diagnostics.acceptAll) - selectPrefix - """ - let _six = 6 - """ - testCaseAsync "can replace unused parameter" <| - CodeFix.check server - """ - let add one two $0three = one + two - """ - (Diagnostics.acceptAll) - selectReplace - """ - let add one two _ = one + two - """ - testCaseAsync "can prefix unused parameter" <| +let private negationToSubtractionTests state = + serverTestList (nameof NegationToSubtraction) state defaultConfigDto None (fun server -> [ + testCaseAsync "converts negation to subtraction" <| CodeFix.check server """ - let add one two $0three = one + two + let getListWithoutFirstAndLastElement list = + let l = List.length list + list[ 1 .. $0l -1 ] """ - (Diagnostics.log >> Diagnostics.acceptAll) - (CodeFix.log >> selectPrefix) + (Diagnostics.expectCode "3") + (CodeFix.ofKind "quickfix" >> CodeFix.withTitle NegationToSubtraction.title) """ - let add one two _three = one + two + let getListWithoutFirstAndLastElement list = + let l = List.length list + list[ 1 .. l - 1 ] """ ]) @@ -292,109 +339,63 @@ let private removeUnusedBindingTests state = """ ]) -let private addExplicitTypeToParameterTests state = - serverTestList (nameof AddExplicitTypeToParameter) state defaultConfigDto None (fun server -> [ - testCaseAsync "can suggest explicit parameter for record-typed function parameters" <| - CodeFix.check server - """ - type Foo = - { name: string } - - let name $0f = - f.name - """ - (Diagnostics.acceptAll) - (CodeFix.withTitle AddExplicitTypeToParameter.title) - """ - type Foo = - { name: string } - - let name (f: Foo) = - f.name - """ - ]) +let private unusedValueTests state = + let config = { defaultConfigDto with UnusedDeclarationsAnalyzer = Some true } + serverTestList (nameof UnusedValue) state config None (fun server -> [ + let selectReplace = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titleReplace + let selectPrefix = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titlePrefix -let private negationToSubtractionTests state = - serverTestList (nameof NegationToSubtraction) state defaultConfigDto None (fun server -> [ - testCaseAsync "converts negation to subtraction" <| + testCaseAsync "can replace unused self-reference" <| CodeFix.check server """ - let getListWithoutFirstAndLastElement list = - let l = List.length list - list[ 1 .. $0l -1 ] + type MyClass() = + member $0this.DoAThing() = () """ - (Diagnostics.expectCode "3") - (CodeFix.ofKind "quickfix" >> CodeFix.withTitle NegationToSubtraction.title) + (Diagnostics.acceptAll) + selectReplace """ - let getListWithoutFirstAndLastElement list = - let l = List.length list - list[ 1 .. l - 1 ] + type MyClass() = + member _.DoAThing() = () """ - ]) - -let private convertPositionalDUToNamedTests state = - serverTestList (nameof ConvertPositionalDUToNamed) state defaultConfigDto None (fun server -> [ - let selectCodeFix = CodeFix.withTitle ConvertPositionalDUToNamed.title - testCaseAsync "in parenthesized let binding" <| + testCaseAsync "can replace unused binding" <| CodeFix.check server """ - type A = A of a: int * b: bool - - let (A(a$0, b)) = A(1, true) + let $0six = 6 """ - Diagnostics.acceptAll - selectCodeFix + (Diagnostics.acceptAll) + selectReplace """ - type A = A of a: int * b: bool - - let (A(a = a; b = b;)) = A(1, true) + let _ = 6 """ - testCaseAsync "in simple match" <| + testCaseAsync "can prefix unused binding" <| CodeFix.check server """ - type A = A of a: int * b: bool - - match A(1, true) with - | A(a$0, b) -> () + let $0six = 6 """ - Diagnostics.acceptAll - selectCodeFix + (Diagnostics.acceptAll) + selectPrefix """ - type A = A of a: int * b: bool - - match A(1, true) with - | A(a = a; b = b;) -> () + let _six = 6 """ - testCaseAsync "in parenthesized match" <| + testCaseAsync "can replace unused parameter" <| CodeFix.check server """ - type A = A of a: int * b: bool - - match A(1, true) with - | (A(a$0, b)) -> () + let add one two $0three = one + two """ - Diagnostics.acceptAll - selectCodeFix + (Diagnostics.acceptAll) + selectReplace """ - type A = A of a: int * b: bool - - match A(1, true) with - | (A(a = a; b = b;)) -> () + let add one two _ = one + two """ - testCaseAsync "when there are new fields on the DU" <| - //ENHANCEMENT: add space before wildcard case + testCaseAsync "can prefix unused parameter" <| CodeFix.check server """ - type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char - - let (ThirdFieldWasJustAdded($0a, b)) = ThirdFieldWasJustAdded(1, true, 'c') + let add one two $0three = one + two """ - Diagnostics.acceptAll - selectCodeFix + (Diagnostics.log >> Diagnostics.acceptAll) + (CodeFix.log >> selectPrefix) """ - type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char - - let (ThirdFieldWasJustAdded(a = a; b = b;c = _;)) = ThirdFieldWasJustAdded(1, true, 'c') + let add one two _three = one + two """ ]) @@ -413,18 +414,19 @@ let private useTripleQuotedInterpolationTests state = " ]) + let tests state = testList "CodeFix tests" [ + addExplicitTypeToParameterTests state + addMissingFunKeywordTests state + addMissingInstanceMemberTests state + changeTypeOfNameToNameOfTests state + convertPositionalDUToNamedTests state generateAbstractClassStubTests state - generateUnionCasesTests state generateRecordStubTests state - addMissingFunKeywordTests state + generateUnionCasesTests state makeOuterBindingRecursiveTests state - changeTypeOfNameToNameOfTests state - addMissingInstanceMemberTests state - unusedValueTests state - removeUnusedBindingTests state - addExplicitTypeToParameterTests state negationToSubtractionTests state - convertPositionalDUToNamedTests state + removeUnusedBindingTests state + unusedValueTests state useTripleQuotedInterpolationTests state ] From 47b2e4d87edaeb234c09c2c054d0b4be5fe2104b Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 11:01:46 +0200 Subject: [PATCH 02/29] Fix: `GenerateAbstractClassStub` triggers twice Triggered for two error codes in same location: * Once for error 54 (`This type is 'abstract' since some abstract members have not been given an implementation. If this is intentional then add the '[]' attribute to your type.`) * And once for error 365 (`No implementation was given for those members [...]`) Solution: Trigger only for Error 365 I *think* both errors always occur together -> triggering just for 365 shouldn't change behaviour Note: `GenerateAbstractClassStub` still has some issues: (-> pending tests) * Inserts text in incorrect location: Seems to inserts members always 1 line down and column with alignment -> might be out of text or step over some other code * Reason: Location is currently where text should end up -- not where text should be inserted and without leading indentation & new line * Adds always all overrides instead of just missing ones -> Same or similar issues (and similar code) as `GenerateInterfaceStub` -> Delay fixing --- .../CodeFixes/GenerateAbstractClassStub.fs | 15 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 168 +++++++++++++++++- 2 files changed, 167 insertions(+), 16 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/GenerateAbstractClassStub.fs b/src/FsAutoComplete/CodeFixes/GenerateAbstractClassStub.fs index f3ff5d07f..6f9873b8c 100644 --- a/src/FsAutoComplete/CodeFixes/GenerateAbstractClassStub.fs +++ b/src/FsAutoComplete/CodeFixes/GenerateAbstractClassStub.fs @@ -15,23 +15,14 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (getTextReplacements: unit -> Map) : CodeFix = Run.ifDiagnosticByCode - (Set.ofList [ "365"; "54" ]) + (Set.ofList [ "365" ]) (fun diagnostic codeActionParams -> asyncResult { let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath - let interestingRange = - (match diagnostic.Code with - | Some "365" -> - // the object expression diagnostic covers the entire interesting range - diagnostic.Range - | Some "54" -> - // the full-class range is on the typename, which should be enough to enable traversal - diagnostic.Range - | _ -> - // everything else is a best guess - codeActionParams.Range) + // the object expression diagnostic covers the entire interesting range + let interestingRange = diagnostic.Range let fcsRange = interestingRange |> protocolRangeToRange (UMX.untag fileName) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 585bc1554..5dd07ef1a 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -175,10 +175,7 @@ let private convertPositionalDUToNamedTests state = let private generateAbstractClassStubTests state = let config = { defaultConfigDto with AbstractClassStubGeneration = Some true } - // issue: returns same fix twice: - // Once for error 54 (`This type is 'abstract' since some abstract members have not been given an implementation.`) - // And once for error 365 (`No implementation was given for those members [...]`) - pserverTestList (nameof GenerateAbstractClassStub) state config None (fun server -> [ + serverTestList (nameof GenerateAbstractClassStub) state config None (fun server -> [ let selectCodeFix = CodeFix.withTitle GenerateAbstractClassStub.title testCaseAsync "can generate a derivative of a long ident - System.IO.Stream" <| CodeFix.checkApplicable server @@ -197,6 +194,169 @@ let private generateAbstractClassStubTests state = """ (Diagnostics.expectCode "365") selectCodeFix + ptestCaseAsync "can generate abstract class stub" <| + // issue: Wants to insert text in line 13, column 12. + // But Line 13 (line with `"""`) is empty -> no column 12 + CodeFix.check server + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type $0Square(x,y, sideLength) = + inherit Shape(x,y) + """ + (Diagnostics.expectCode "365") + selectCodeFix + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type Square(x,y, sideLength) = + inherit Shape(x,y) + + override this.Area: float = + failwith "Not Implemented" + override this.Name: string = + failwith "Not Implemented" + """ + ptestCaseAsync "can generate abstract class stub without trailing nl" <| + // issue: Wants to insert text in line 13, column 12. + // But there's no line 13 (last line is line 12) + CodeFix.check server + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type $0Square(x,y, sideLength) = + inherit Shape(x,y)""" + (Diagnostics.expectCode "365") + selectCodeFix + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type Square(x,y, sideLength) = + inherit Shape(x,y) + + override this.Area: float = + failwith "Not Implemented" + override this.Name: string = + failwith "Not Implemented" + """ + ptestCaseAsync "inserts override in correct place" <| + // issue: inserts overrides after `let a = ...`, not before + CodeFix.check server + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type $0Square(x,y, sideLength) = + inherit Shape(x,y) + let a = 0 + """ + (Diagnostics.expectCode "365") + selectCodeFix + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type Square(x,y, sideLength) = + inherit Shape(x,y) + + override this.Area: float = + failwith "Not Implemented" + override this.Name: string = + failwith "Not Implemented" + let a = 0 + """ + ptestCaseAsync "can generate abstract class stub with existing override" <| + // issue: Generates override for already existing member + CodeFix.check server + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type $0Square(x,y, sideLength) = + inherit Shape(x,y) + """ + (Diagnostics.expectCode "365") + selectCodeFix + """ + [] + type Shape(x0: float, y0: float) = + let mutable x, y = x0, y0 + + abstract Name : string with get + abstract Area : float with get + + member _.Move dx dy = + x <- x + dx + y <- y + dy + + type Square(x,y, sideLength) = + inherit Shape(x,y) + + override this.Name = "Circle" + + override this.Area: float = + failwith "Not Implemented" + """ ]) let private generateRecordStubTests state = From d1e77e696df0223f8520019beb9484741520e673 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 11:46:21 +0200 Subject: [PATCH 03/29] Fix: `AddMissingRecKeyword` uses wrong function name in title Add Test for `AddMissingRecKeyword` --- .../CodeFixes/AddMissingRecKeyword.fs | 5 +++-- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs b/src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs index 7f93d7cf5..393e66007 100644 --- a/src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs +++ b/src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs @@ -8,6 +8,7 @@ open FsAutoComplete open FsAutoComplete.LspHelpers open FSharp.UMX +let title symbolName = $"Make '{symbolName}' recursive" /// a codefix that adds the 'rec' modifier to a binding in a mutually-recursive loop let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix = Run.ifDiagnosticByCode @@ -60,10 +61,10 @@ let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix = let protocolRange = fcsRangeToLsp (FSharp.Compiler.Text.Range.mkRange (UMX.untag fileName) fcsStartPos fcsEndPos) - let symbolName = getLineText lines protocolRange + let! symbolName = getLineText lines protocolRange return - [ { Title = $"Make '{symbolName}' recursive" + [ { Title = title symbolName File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic Edits = diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 5dd07ef1a..1ffda62cb 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -93,6 +93,23 @@ let private addMissingInstanceMemberTests state = """ ]) +let private addMissingRecKeywordTests state = + serverTestList (nameof AddMissingRecKeyword) state defaultConfigDto None (fun server -> [ + // `rec` in single function is handled in `MakeOuterBindingRecursive` + testCaseAsync "can add rec to mutual recursive function" <| + CodeFix.check server + """ + $0let a x = x + and b x = x + """ + (Diagnostics.expectCode "576") + (CodeFix.withTitle (AddMissingRecKeyword.title "a")) + """ + let rec a x = x + and b x = x + """ + ]) + let private changeTypeOfNameToNameOfTests state = serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ testCaseAsync "can suggest fix" <| @@ -579,6 +596,7 @@ let tests state = testList "CodeFix tests" [ addExplicitTypeToParameterTests state addMissingFunKeywordTests state addMissingInstanceMemberTests state + addMissingRecKeywordTests state changeTypeOfNameToNameOfTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state From ddf44ece5371b5e7f18eca1f028cebf80bdf7cc6 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 12:35:23 +0200 Subject: [PATCH 04/29] Fix: `AddTypeToIndeterminateValue` doesn't trigger Issue was: incorrect file name Add tests for AddTypeToIndeterminateValue Examples based on: https://github.com/dotnet/fsharp/pull/11236 & https://gist.github.com/cartermp/995584694fb6f2f7b228fa8e939795c9 --- .../CodeFixes/AddTypeToIndeterminateValue.fs | 13 ++--- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 47 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/AddTypeToIndeterminateValue.fs b/src/FsAutoComplete/CodeFixes/AddTypeToIndeterminateValue.fs index 1c300da35..a232e09b8 100644 --- a/src/FsAutoComplete/CodeFixes/AddTypeToIndeterminateValue.fs +++ b/src/FsAutoComplete/CodeFixes/AddTypeToIndeterminateValue.fs @@ -7,7 +7,9 @@ open FsAutoComplete open FsAutoComplete.LspHelpers open FSharp.Compiler.EditorServices open FSharp.Compiler.Symbols +open FSharp.UMX +let title = "Add explicit type annotation" /// fix inderminate type errors by adding an explicit type to a value let fix (getParseResultsForFile: GetParseResultsForFile) @@ -17,14 +19,13 @@ let fix (Set.ofList ["72"; "3245"]) (fun diagnostic codeActionParams -> asyncResult { - let filename = codeActionParams.TextDocument.GetFilePath () - let typedFileName = filename |> Utils.normalizePath + let fileName = codeActionParams.TextDocument.GetFilePath () |> Utils.normalizePath let fcsRange = protocolRangeToRange (codeActionParams.TextDocument.GetFilePath()) diagnostic.Range - let! (tyRes, line, lines) = getParseResultsForFile typedFileName fcsRange.Start + let! (tyRes, line, lines) = getParseResultsForFile fileName fcsRange.Start let! (endColumn, identIslands) = Lexer.findLongIdents(fcsRange.Start.Column, line) |> Result.ofOption (fun _ -> "No long ident at position") match tyRes.GetCheckResults.GetDeclarationLocation(fcsRange.Start.Line, endColumn, line, List.ofArray identIslands) with - | FindDeclResult.DeclFound declRange when declRange.FileName = filename -> - let! projectOptions = getProjectOptionsForFile typedFileName + | FindDeclResult.DeclFound declRange when declRange.FileName = UMX.untag fileName -> + let! projectOptions = getProjectOptionsForFile fileName let protocolDeclRange = fcsRangeToLsp declRange let! declText = lines.GetText declRange let! declTextLine = lines.GetLine declRange.Start |> Result.ofOption (fun _ -> "No line found at pos") @@ -48,7 +49,7 @@ let fix else "(" + declText + ": " + typeString + ")", protocolDeclRange return [{ - Title = "Add explicit type annotation" + Title = title File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic Kind = FixKind.Fix diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 1ffda62cb..fe24c3e33 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -110,6 +110,52 @@ let private addMissingRecKeywordTests state = """ ]) +let private addTypeToIndeterminateValueTests state = + serverTestList (nameof AddTypeToIndeterminateValue) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle AddTypeToIndeterminateValue.title + testCaseAsync "can add type annotation to error 72 ('Lookup on object of indeterminate type')" <| + CodeFix.check server + """ + let data = [ + {| Name = "foo"; Value = 42 |} + {| Name = "bar"; Value = 13 |} + ] + let res = List.filter (fun d -> $0d.Value > 20) data + """ + (Diagnostics.expectCode "72") + selectCodeFix + """ + let data = [ + {| Name = "foo"; Value = 42 |} + {| Name = "bar"; Value = 13 |} + ] + let res = List.filter (fun (d: {| Name: string; Value: int |}) -> d.Value > 20) data + """ + testCaseAsync "can add type annotation to error 3245 ('The input to a copy-and-update expression that creates an anonymous record must be either an anonymous record or a record')" <| + CodeFix.check server + """ + [1..5] + |> List.fold + (fun s i -> + match i % 2 with + | 0 -> {| $0s with Evens = s.Evens + 1 |} + | _ -> s + ) + {| Evens = 0 |} + """ + (Diagnostics.expectCode "3245") + selectCodeFix + """ + [1..5] + |> List.fold + (fun (s: {| Evens: int |}) i -> + match i % 2 with + | 0 -> {| s with Evens = s.Evens + 1 |} + | _ -> s + ) + {| Evens = 0 |} + """ + ]) let private changeTypeOfNameToNameOfTests state = serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ testCaseAsync "can suggest fix" <| @@ -597,6 +643,7 @@ let tests state = testList "CodeFix tests" [ addMissingFunKeywordTests state addMissingInstanceMemberTests state addMissingRecKeywordTests state + addTypeToIndeterminateValueTests state changeTypeOfNameToNameOfTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state From 0505dd64a93935e45c7ee9c7962716c784ddf89c Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 14:44:37 +0200 Subject: [PATCH 05/29] Fix: `UseMutationWhenValueIsMutable` adds `<-` after `=` instead of replacing it Rename: `ChangeComparisonToMutableAssignment` to `UseMutationWhenValueIsMutable` -> Align with CodeFix in `dotnet/fsharp` (VS) Add tests for `UseMutationWhenValueIsMutable` --- ...nt.fs => UseMutationWhenValueIsMutable.fs} | 11 ++-- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 57 +++++++++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) rename src/FsAutoComplete/CodeFixes/{ChangeComparisonToMutableAssignment.fs => UseMutationWhenValueIsMutable.fs} (85%) diff --git a/src/FsAutoComplete/CodeFixes/ChangeComparisonToMutableAssignment.fs b/src/FsAutoComplete/CodeFixes/UseMutationWhenValueIsMutable.fs similarity index 85% rename from src/FsAutoComplete/CodeFixes/ChangeComparisonToMutableAssignment.fs rename to src/FsAutoComplete/CodeFixes/UseMutationWhenValueIsMutable.fs index aee5896a3..893512d96 100644 --- a/src/FsAutoComplete/CodeFixes/ChangeComparisonToMutableAssignment.fs +++ b/src/FsAutoComplete/CodeFixes/UseMutationWhenValueIsMutable.fs @@ -1,4 +1,4 @@ -module FsAutoComplete.CodeFix.ChangeComparisonToMutableAssignment +module FsAutoComplete.CodeFix.UseMutationWhenValueIsMutable open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types @@ -8,6 +8,7 @@ open FsAutoComplete.CodeFix.Navigation open FsAutoComplete.LspHelpers open FSharp.Compiler.Symbols +let title = "Use '<-' to mutate value" /// a codefix that changes equality checking to mutable assignment when the compiler thinks it's relevant let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = Run.ifDiagnosticByCode @@ -38,15 +39,15 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = match walkForwardUntilCondition lines endOfMutableValue (fun c -> c = '=') with | Some equalsPos -> - let! nextPos = inc lines equalsPos |> Result.ofOption (fun _ -> "next position wasn't valid") + let! prevPos = dec lines equalsPos |> Result.ofOption (fun _ -> "prev position wasn't valid") return [ { File = codeActionParams.TextDocument - Title = "Use '<-' to mutate value" + Title = title SourceDiagnostic = Some diagnostic Edits = [| { Range = - { Start = equalsPos - End = nextPos } + { Start = prevPos + End = equalsPos } NewText = "<-" } |] Kind = FixKind.Refactor } ] | None -> return [] diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index d4b065c71..9e51334b1 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -880,7 +880,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS RefCellAccesToNot.fix tryGetParseResultsForFile UseSafeCastInsteadOfUnsafe.fix getRangeText MakeDeclarationMutable.fix tryGetParseResultsForFile tryGetProjectOptions - ChangeComparisonToMutableAssignment.fix tryGetParseResultsForFile + UseMutationWhenValueIsMutable.fix tryGetParseResultsForFile ConvertInvalidRecordToAnonRecord.fix tryGetParseResultsForFile RemoveUnnecessaryReturnOrYield.fix tryGetParseResultsForFile getLineText ChangeCSharpLambdaToFSharp.fix tryGetParseResultsForFile getLineText diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index fe24c3e33..74825ab9b 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -637,6 +637,62 @@ let private useTripleQuotedInterpolationTests state = " ]) +let private useMutationWhenValueIsMutableTests state = + serverTestList (nameof UseMutationWhenValueIsMutable) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle UseMutationWhenValueIsMutable.title + testCaseAsync "can replace = with <- when cursor on =" <| + CodeFix.check server + """ + let _ = + let mutable v = 42 + v $0= 5 + v + """ + (Diagnostics.expectCode "20") + selectCodeFix + """ + let _ = + let mutable v = 42 + v <- 5 + v + """ + testCaseAsync "can replace = with <- when cursor on variable" <| + CodeFix.check server + """ + let _ = + let mutable v = 42 + $0v = 5 + v + """ + (Diagnostics.expectCode "20") + selectCodeFix + """ + let _ = + let mutable v = 42 + v <- 5 + v + """ + testCaseAsync "doesn't suggest fix when = is comparison" <| + CodeFix.checkNotApplicable server + """ + let _ = + let mutable v = 42 + v $0= 5 + """ + Diagnostics.acceptAll + selectCodeFix + testCaseAsync "doesn't suggest fix when variable is not mutable" <| + CodeFix.checkNotApplicable server + """ + let _ = + let v = 42 + v $0= 5 + v + """ + Diagnostics.acceptAll + selectCodeFix + ]) + let tests state = testList "CodeFix tests" [ addExplicitTypeToParameterTests state @@ -654,4 +710,5 @@ let tests state = testList "CodeFix tests" [ removeUnusedBindingTests state unusedValueTests state useTripleQuotedInterpolationTests state + useMutationWhenValueIsMutableTests state ] From 0f36396b9228b10342516fe47e91bb34e67abed1 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 14:46:44 +0200 Subject: [PATCH 06/29] Fix: `ConvertCSharpLambdaToFSharpLambda` joins multi-line body to single-line body Rename `ChangeCSharpLambdaToFSharp` to `ConvertCSharpLambdaToFSharpLambda` -> align with `dotnet/fsharp` (VS) Add tests for `ConvertCSharpLambdaToFSharpLambda` --- .../CodeFixes/ChangeCSharpLambdaToFSharp.fs | 44 --------- .../ConvertCSharpLambdaToFSharpLambda.fs | 76 ++++++++++++++++ src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 89 +++++++++++++++++++ 4 files changed, 166 insertions(+), 45 deletions(-) delete mode 100644 src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs create mode 100644 src/FsAutoComplete/CodeFixes/ConvertCSharpLambdaToFSharpLambda.fs diff --git a/src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs b/src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs deleted file mode 100644 index a0b6b64e2..000000000 --- a/src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs +++ /dev/null @@ -1,44 +0,0 @@ -/// a codefix that rewrites C#-style '=>' lambdas to F#-style 'fun _ -> _' lambdas -module FsAutoComplete.CodeFix.ChangeCSharpLambdaToFSharp - -open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix.Types -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText) - : CodeFix = - Run.ifDiagnosticByCode - (Set.ofList [ "39" // undefined value - "43" ]) // operator not defined - (fun diagnostic codeActionParams -> - asyncResult { - let fileName = - codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath - - let fcsPos = protocolPosToPos diagnostic.Range.Start - let! (tyRes, _, lines) = getParseResultsForFile fileName fcsPos - - match tyRes.GetParseResults.TryRangeOfParenEnclosingOpEqualsGreaterUsage fcsPos with - | Some (fullParenRange, lambdaArgRange, lambdaBodyRange) -> - let! argExprText = - getLineText lines (fcsRangeToLsp lambdaArgRange) - - let! bodyExprText = - getLineText lines (fcsRangeToLsp lambdaBodyRange) - - let replacementText = $"fun {argExprText} -> {bodyExprText}" - let replacementRange = fcsRangeToLsp fullParenRange - - return - [ { Title = "Replace C#-style lambda with F# lambda" - File = codeActionParams.TextDocument - SourceDiagnostic = Some diagnostic - Edits = - [| { Range = replacementRange - NewText = replacementText } |] - Kind = FixKind.Refactor } ] - | None -> return [] - } - ) diff --git a/src/FsAutoComplete/CodeFixes/ConvertCSharpLambdaToFSharpLambda.fs b/src/FsAutoComplete/CodeFixes/ConvertCSharpLambdaToFSharpLambda.fs new file mode 100644 index 000000000..f014e6e73 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/ConvertCSharpLambdaToFSharpLambda.fs @@ -0,0 +1,76 @@ +/// a codefix that rewrites C#-style '=>' lambdas to F#-style 'fun _ -> _' lambdas +module FsAutoComplete.CodeFix.ConvertCSharpLambdaToFSharpLambda + +open FsToolkit.ErrorHandling +open FsAutoComplete.CodeFix.Types +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text + +let title = "Replace C#-style lambda with F# lambda" +// adopted from `FSharp.Compiler.CodeAnalysis.FSharpParseFileResults.TryRangeOfParenEnclosingOpEqualsGreaterUsage` +let private tryRangeOfParenEnclosingOpEqualsGreaterUsage input pos = + let (|Ident|_|) ofName = + function | SynExpr.Ident ident when ident.idText = ofName -> Some () + | _ -> None + let (|InfixAppOfOpEqualsGreater|_|) = + function | SynExpr.App(ExprAtomicFlag.NonAtomic, false, SynExpr.App(ExprAtomicFlag.NonAtomic, true, Ident "op_EqualsGreater", actualParamListExpr, range), actualLambdaBodyExpr, _) -> + let opEnd = range.End + let opStart = Position.mkPos (range.End.Line) (range.End.Column - 2) + let opRange = Range.mkRange range.FileName opStart opEnd + + let argsRange = actualParamListExpr.Range + + Some (argsRange, opRange) + | _ -> None + + SyntaxTraversal.Traverse(pos, input, { new SyntaxVisitorBase<_>() with + member _.VisitExpr(_, _, defaultTraverse, expr) = + match expr with + | SynExpr.Paren(InfixAppOfOpEqualsGreater(argsRange, opRange), _, _, _) -> + Some (argsRange, opRange) + | _ -> defaultTraverse expr + + member _.VisitBinding(_path, defaultTraverse, binding) = + match binding with + | SynBinding(kind=SynBindingKind.Normal; expr=InfixAppOfOpEqualsGreater(argsRange, opRange)) -> + Some (argsRange, opRange) + | _ -> defaultTraverse binding }) +let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText) + : CodeFix = + Run.ifDiagnosticByCode + (Set.ofList [ "39" // undefined value + "43" ]) // operator not defined + (fun diagnostic codeActionParams -> + asyncResult { + let fileName = + codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + + let fcsPos = protocolPosToPos diagnostic.Range.Start + let! (tyRes, _, lines) = getParseResultsForFile fileName fcsPos + + match tryRangeOfParenEnclosingOpEqualsGreaterUsage tyRes.GetAST fcsPos with + | Some (argsRange, opRange) -> + return + [ { Title = title + File = codeActionParams.TextDocument + SourceDiagnostic = Some diagnostic + Edits = + [| + // add `fun ` in front of args + { + Range = { Start = fcsPosToLsp argsRange.Start; End = fcsPosToLsp argsRange.Start } + NewText = "fun " + } + // replace `=>` with `->` + { + Range = fcsRangeToLsp opRange + NewText = "->" + } + |] + Kind = FixKind.Refactor } ] + | None -> return [] + } + ) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 9e51334b1..4e4e6324f 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -883,7 +883,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS UseMutationWhenValueIsMutable.fix tryGetParseResultsForFile ConvertInvalidRecordToAnonRecord.fix tryGetParseResultsForFile RemoveUnnecessaryReturnOrYield.fix tryGetParseResultsForFile getLineText - ChangeCSharpLambdaToFSharp.fix tryGetParseResultsForFile getLineText + ConvertCSharpLambdaToFSharpLambda.fix tryGetParseResultsForFile getLineText AddMissingFunKeyword.fix getFileLines getLineText MakeOuterBindingRecursive.fix tryGetParseResultsForFile getLineText AddMissingRecKeyword.fix getFileLines getLineText diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 74825ab9b..3019fa37e 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -170,6 +170,94 @@ let private changeTypeOfNameToNameOfTests state = """ ]) +let private convertCSharpLambdaToFSharpTests state = + serverTestList (nameof ConvertCSharpLambdaToFSharpLambda) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ConvertCSharpLambdaToFSharpLambda.title + testCaseAsync "can convert csharp lambda in variable assignment with cursor on input" <| + CodeFix.check server + """ + let x = $0y => 1 + y + """ + Diagnostics.acceptAll + selectCodeFix + """ + let x = fun y -> 1 + y + """ + testCaseAsync "can convert csharp lambda in variable assignment with cursor on usage" <| + CodeFix.check server + """ + let x = y => 1 + $0y + """ + Diagnostics.acceptAll + selectCodeFix + """ + let x = fun y -> 1 + y + """ + //ENHANCEMENT: trigger on `=>` + // testCaseAsync "can convert csharp lambda in variable assignment with cursor on =>" <| + // CodeFix.check server + // """ + // let x = y $0=> 1 + y + // """ + // Diagnostics.acceptAll + // selectReplaceCSharpLambdaWithFSharp + // """ + // let x = fun y -> 1 + y + // """ + testCaseAsync "can convert csharp lambda in lambda with parens with cursor on input" <| + CodeFix.check server + """ + [1..10] |> List.map ($0x => 1 + x) + """ + Diagnostics.acceptAll + selectCodeFix + """ + [1..10] |> List.map (fun x -> 1 + x) + """ + testCaseAsync "can convert csharp lambda in lambda with parens with cursor on usage" <| + CodeFix.check server + """ + [1..10] |> List.map (x => 1 + $0x) + """ + Diagnostics.acceptAll + selectCodeFix + """ + [1..10] |> List.map (fun x -> 1 + x) + """ + testCaseAsync "keep multi-line lambda intact - cursor on input" <| + CodeFix.check server + """ + let x = + $0y => + let a = 1 + y + a + """ + Diagnostics.acceptAll + selectCodeFix + """ + let x = + fun y -> + let a = 1 + y + a + """ + testCaseAsync "keep multi-line lambda intact - cursor on usage" <| + CodeFix.check server + """ + let x = + y => + let a = 1 + $0y + a + """ + Diagnostics.acceptAll + selectCodeFix + """ + let x = + fun y -> + let a = 1 + y + a + """ + ]) + let private convertPositionalDUToNamedTests state = serverTestList (nameof ConvertPositionalDUToNamed) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ConvertPositionalDUToNamed.title @@ -701,6 +789,7 @@ let tests state = testList "CodeFix tests" [ addMissingRecKeywordTests state addTypeToIndeterminateValueTests state changeTypeOfNameToNameOfTests state + convertCSharpLambdaToFSharpTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state generateRecordStubTests state From 842c24a88dd2246613aead5be587581f13546fc3 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 14:46:02 +0200 Subject: [PATCH 07/29] Rename `ColonInFieldType` to `ChangeEqualsInFieldTypeToColon` Add tests for `ChangeEqualsInFieldTypeToColon` --- ...e.fs => ChangeEqualsInFieldTypeToColon.fs} | 5 +-- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 33 +++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) rename src/FsAutoComplete/CodeFixes/{ColonInFieldType.fs => ChangeEqualsInFieldTypeToColon.fs} (83%) diff --git a/src/FsAutoComplete/CodeFixes/ColonInFieldType.fs b/src/FsAutoComplete/CodeFixes/ChangeEqualsInFieldTypeToColon.fs similarity index 83% rename from src/FsAutoComplete/CodeFixes/ColonInFieldType.fs rename to src/FsAutoComplete/CodeFixes/ChangeEqualsInFieldTypeToColon.fs index ab051801e..21b36ec3b 100644 --- a/src/FsAutoComplete/CodeFixes/ColonInFieldType.fs +++ b/src/FsAutoComplete/CodeFixes/ChangeEqualsInFieldTypeToColon.fs @@ -1,9 +1,10 @@ -module FsAutoComplete.CodeFix.ColonInFieldType +module FsAutoComplete.CodeFix.ChangeEqualsInFieldTypeToColon open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types open FsAutoComplete +let title = "Use ':' for type in field declaration" /// a codefix that fixes a malformed record type annotation to use colon instead of equals let fix: CodeFix = Run.ifDiagnosticByCode @@ -11,7 +12,7 @@ let fix: CodeFix = (fun diagnostic codeActionParams -> if diagnostic.Message = "Unexpected symbol '=' in field declaration. Expected ':' or other token." then AsyncResult.retn [ { File = codeActionParams.TextDocument - Title = "Use ':' for type in field declaration" + Title = title SourceDiagnostic = Some diagnostic Edits = [| { Range = diagnostic.Range diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 4e4e6324f..5f3a1f674 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -875,7 +875,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS MissingEquals.fix getFileLines NegationToSubtraction.fix getFileLines DoubleEqualsToSingleEquals.fix getRangeText - ColonInFieldType.fix + ChangeEqualsInFieldTypeToColon.fix ParenthesizeExpression.fix getRangeText RefCellAccesToNot.fix tryGetParseResultsForFile UseSafeCastInsteadOfUnsafe.fix getRangeText diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 3019fa37e..bf1116df7 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -156,6 +156,38 @@ let private addTypeToIndeterminateValueTests state = {| Evens = 0 |} """ ]) + +let private changeEqualsInFieldTypeToColonTests state = + serverTestList (nameof ChangeEqualsInFieldTypeToColon) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ChangeEqualsInFieldTypeToColon.title + testCaseAsync "can change = to : in single line" <| + CodeFix.check server + """ + type A = { Name : string; Key $0= int } + """ + (Diagnostics.expectCode "10") + selectCodeFix + """ + type A = { Name : string; Key : int } + """ + testCaseAsync "can change = to : in multi line" <| + CodeFix.check server + """ + type A = { + Name : string + Key $0= int + } + """ + (Diagnostics.expectCode "10") + selectCodeFix + """ + type A = { + Name : string + Key : int + } + """ + ]) + let private changeTypeOfNameToNameOfTests state = serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ testCaseAsync "can suggest fix" <| @@ -788,6 +820,7 @@ let tests state = testList "CodeFix tests" [ addMissingInstanceMemberTests state addMissingRecKeywordTests state addTypeToIndeterminateValueTests state + changeEqualsInFieldTypeToColonTests state changeTypeOfNameToNameOfTests state convertCSharpLambdaToFSharpTests state convertPositionalDUToNamedTests state From 678295e9114490f9ab15daa0ff013df39319ce49 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 14:43:22 +0200 Subject: [PATCH 08/29] Add test for `ConvertBangEqualsToInequality` Note: It's called `ConvertToNotEqualsEqualityExpression` in `dotnet/fsharp` (VS). Unlike other CodeFixes this wasn't renamed because `ConvertBangEqualsToInequality` is more descriptive. --- .../CodeFixes/ConvertBangEqualsToInequality.fs | 3 ++- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/FsAutoComplete/CodeFixes/ConvertBangEqualsToInequality.fs b/src/FsAutoComplete/CodeFixes/ConvertBangEqualsToInequality.fs index 957591a56..9d2c8b640 100644 --- a/src/FsAutoComplete/CodeFixes/ConvertBangEqualsToInequality.fs +++ b/src/FsAutoComplete/CodeFixes/ConvertBangEqualsToInequality.fs @@ -6,6 +6,7 @@ open FsAutoComplete.CodeFix.Types open FsAutoComplete open FsAutoComplete.LspHelpers +let title = "Use <> for inequality check" let fix (getRangeText: GetRangeText): CodeFix = Run.ifDiagnosticByCode (Set.ofList ["43"]) @@ -14,7 +15,7 @@ let fix (getRangeText: GetRangeText): CodeFix = let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath let! errorText = getRangeText fileName diag.Range do! Result.guard (fun () -> errorText = "!=") "Not an != equality usage" - return [{ Title = "Use <> for inequality check" + return [{ Title = title File = codeActionParams.TextDocument SourceDiagnostic = Some diag Kind = FixKind.Fix diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index bf1116df7..7cf969554 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -202,6 +202,21 @@ let private changeTypeOfNameToNameOfTests state = """ ]) +let private convertBangEqualsToInequalityTests state = + serverTestList (nameof ConvertBangEqualsToInequality) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ConvertBangEqualsToInequality.title + testCaseAsync "can change != to <>" <| + CodeFix.check server + """ + 1 $0!= 2 + """ + (Diagnostics.expectCode "43") + selectCodeFix + """ + 1 <> 2 + """ + ]) + let private convertCSharpLambdaToFSharpTests state = serverTestList (nameof ConvertCSharpLambdaToFSharpLambda) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ConvertCSharpLambdaToFSharpLambda.title @@ -822,6 +837,7 @@ let tests state = testList "CodeFix tests" [ addTypeToIndeterminateValueTests state changeEqualsInFieldTypeToColonTests state changeTypeOfNameToNameOfTests state + convertBangEqualsToInequalityTests state convertCSharpLambdaToFSharpTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state From 00098f208411511a247148689df1068d5d0d119d Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 15:01:18 +0200 Subject: [PATCH 09/29] Add tests for `ConvertInvalidRecordToAnonRecord` Note: Not renamed to match `dotnet/fsharp` (VS; `ConvertToAnonymousRecord`): Current name is more descriptive. --- .../ConvertInvalidRecordToAnonRecord.fs | 3 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 58 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/FsAutoComplete/CodeFixes/ConvertInvalidRecordToAnonRecord.fs b/src/FsAutoComplete/CodeFixes/ConvertInvalidRecordToAnonRecord.fs index a79e4c27d..5cdac49f8 100644 --- a/src/FsAutoComplete/CodeFixes/ConvertInvalidRecordToAnonRecord.fs +++ b/src/FsAutoComplete/CodeFixes/ConvertInvalidRecordToAnonRecord.fs @@ -7,6 +7,7 @@ open FsAutoComplete open FsAutoComplete.CodeFix.Navigation open FsAutoComplete.LspHelpers +let title = "Convert to anonymous record" /// a codefix that converts unknown/partial record expressions to anonymous records let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = Run.ifDiagnosticByCode @@ -34,7 +35,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = |> Result.ofOption (fun _ -> "No end insert range") return - [ { Title = "Convert to anonymous record" + [ { Title = title File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic Edits = diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 7cf969554..bea7f1c3b 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -305,6 +305,63 @@ let private convertCSharpLambdaToFSharpTests state = """ ]) + +let private convertInvalidRecordToAnonRecordTests state = + serverTestList (nameof ConvertInvalidRecordToAnonRecord) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ConvertInvalidRecordToAnonRecord.title + testCaseAsync "can convert single-line record with single field" <| + CodeFix.check server + """ + let v = { $0Name = "foo" } + """ + (Diagnostics.expectCode "39") + selectCodeFix + """ + let v = {| Name = "foo" |} + """ + testCaseAsync "can convert single-line record with two fields" <| + CodeFix.check server + """ + let v = { $0Name = "foo"; Value = 42 } + """ + (Diagnostics.expectCode "39") + selectCodeFix + """ + let v = {| Name = "foo"; Value = 42 |} + """ + testCaseAsync "can convert multi-line record with two fields" <| + CodeFix.check server + """ + let v = { + $0Name = "foo" + Value = 42 + } + """ + (Diagnostics.expectCode "39") + selectCodeFix + """ + let v = {| + Name = "foo" + Value = 42 + |} + """ + testCaseAsync "doesn't trigger for existing record" <| + CodeFix.checkNotApplicable server + """ + type V = { Name: string; Value: int } + let v = { $0Name = "foo"; Value = 42 } + """ + (Diagnostics.acceptAll) + selectCodeFix + testCaseAsync "doesn't trigger for anon record" <| + CodeFix.checkNotApplicable server + """ + let v = {| $0Name = "foo"; Value = 42 |} + """ + (Diagnostics.acceptAll) + selectCodeFix + ]) + let private convertPositionalDUToNamedTests state = serverTestList (nameof ConvertPositionalDUToNamed) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ConvertPositionalDUToNamed.title @@ -839,6 +896,7 @@ let tests state = testList "CodeFix tests" [ changeTypeOfNameToNameOfTests state convertBangEqualsToInequalityTests state convertCSharpLambdaToFSharpTests state + convertInvalidRecordToAnonRecordTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state generateRecordStubTests state From a72ea781f18c2643f5ccd7332d930484506d058b Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 20:15:48 +0200 Subject: [PATCH 10/29] Add tests for `ConvertDoubleEqualsToSingleEquals` Rename `DoubleEqualsToSingleEquals` to `ConvertDoubleEqualsToSingleEquals` * Note: Name in `dotnet/fsharp` (VS): `ConvertToSingleEqualsEqualityExpression` --- ...s => ConvertDoubleEqualsToSingleEquals.fs} | 5 ++-- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) rename src/FsAutoComplete/CodeFixes/{DoubleEqualsToSingleEquals.fs => ConvertDoubleEqualsToSingleEquals.fs} (86%) diff --git a/src/FsAutoComplete/CodeFixes/DoubleEqualsToSingleEquals.fs b/src/FsAutoComplete/CodeFixes/ConvertDoubleEqualsToSingleEquals.fs similarity index 86% rename from src/FsAutoComplete/CodeFixes/DoubleEqualsToSingleEquals.fs rename to src/FsAutoComplete/CodeFixes/ConvertDoubleEqualsToSingleEquals.fs index 82d3fcd2a..654a0ab51 100644 --- a/src/FsAutoComplete/CodeFixes/DoubleEqualsToSingleEquals.fs +++ b/src/FsAutoComplete/CodeFixes/ConvertDoubleEqualsToSingleEquals.fs @@ -1,10 +1,11 @@ -module FsAutoComplete.CodeFix.DoubleEqualsToSingleEquals +module FsAutoComplete.CodeFix.ConvertDoubleEqualsToSingleEquals open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types open FsAutoComplete open FsAutoComplete.LspHelpers +let title = "Use '=' for equality check" /// a codefix that corrects == equality to = equality let fix (getRangeText: GetRangeText) : CodeFix = Run.ifDiagnosticByCode @@ -16,7 +17,7 @@ let fix (getRangeText: GetRangeText) : CodeFix = match errorText with | "==" -> return - [ { Title = "Use '=' for equality check" + [ { Title = title File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic Edits = diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 5f3a1f674..0c67bfa70 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -874,7 +874,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS getAbstractClassStubReplacements) MissingEquals.fix getFileLines NegationToSubtraction.fix getFileLines - DoubleEqualsToSingleEquals.fix getRangeText + ConvertDoubleEqualsToSingleEquals.fix getRangeText ChangeEqualsInFieldTypeToColon.fix ParenthesizeExpression.fix getRangeText RefCellAccesToNot.fix tryGetParseResultsForFile diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index bea7f1c3b..0c1931878 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -305,6 +305,28 @@ let private convertCSharpLambdaToFSharpTests state = """ ]) +let private convertDoubleEqualsToSingleEqualsTests state = + serverTestList (nameof ConvertDoubleEqualsToSingleEquals) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ConvertDoubleEqualsToSingleEquals.title + testCaseAsync "can replace == with =" <| + CodeFix.check server + """ + 1 $0== 1 + """ + (Diagnostics.expectCode "43") + selectCodeFix + """ + 1 = 1 + """ + testCaseAsync "doesn't replace existing operator == with =" <| + CodeFix.checkNotApplicable server + """ + let (==) a b = a = b + 1 $0== 1 + """ + Diagnostics.acceptAll + selectCodeFix + ]) let private convertInvalidRecordToAnonRecordTests state = serverTestList (nameof ConvertInvalidRecordToAnonRecord) state defaultConfigDto None (fun server -> [ @@ -896,6 +918,7 @@ let tests state = testList "CodeFix tests" [ changeTypeOfNameToNameOfTests state convertBangEqualsToInequalityTests state convertCSharpLambdaToFSharpTests state + convertDoubleEqualsToSingleEqualsTests state convertInvalidRecordToAnonRecordTests state convertPositionalDUToNamedTests state generateAbstractClassStubTests state From 1a1eb459a63b24959670524bdb14f3d226000f5f Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 8 Apr 2022 20:57:06 +0200 Subject: [PATCH 11/29] Fix: Goto Declaration fails when declaration is in untitled file * Reason: `ParseAndCheckResults.TryFindIdentifierDeclaration` fails when declaration is in untitled file (-> `File.Exists` is false) * Solution: Succeed when decl is in same file as lookup pos Fix: `MakeDeclarationMutable` doesn't trigger in untitled file Add tests for `MakeDeclarationMutable` --- .../ParseAndCheckResults.fs | 11 ++- .../CodeFixes/MakeDeclarationMutable.fs | 3 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 78 +++++++++++++++++++ test/FsAutoComplete.Tests.Lsp/GoToTests.fs | 77 +++++++++++++++++- 4 files changed, 164 insertions(+), 5 deletions(-) diff --git a/src/FsAutoComplete.Core/ParseAndCheckResults.fs b/src/FsAutoComplete.Core/ParseAndCheckResults.fs index 9c0e68bca..69bd6da20 100644 --- a/src/FsAutoComplete.Core/ParseAndCheckResults.fs +++ b/src/FsAutoComplete.Core/ParseAndCheckResults.fs @@ -74,7 +74,7 @@ type ParseAndCheckResults | None -> return Error "load directive not recognized" } - member __.TryFindIdentifierDeclaration (pos: Position) (lineStr: LineStr) = + member x.TryFindIdentifierDeclaration (pos: Position) (lineStr: LineStr) = match Lexer.findLongIdents (pos.Column, lineStr) with | None -> async.Return(ResultOrString.Error "Could not find ident at this location") | Some (col, identIsland) -> @@ -151,6 +151,15 @@ type ParseAndCheckResults return ResultOrString.Error(sprintf "Could not find declaration. %s" elaboration) | FindDeclResult.DeclFound range when range.FileName.EndsWith(Range.rangeStartup.FileName) -> return ResultOrString.Error "Could not find declaration" + | FindDeclResult.DeclFound range when range.FileName = UMX.untag x.FileName -> + // decl in same file + // necessary to get decl in untitled file (-> `File.Exists range.FileName` is false) + logger.info ( + Log.setMessage "Got a declresult of {range} in same file" + >> Log.addContextDestructured "range" range + ) + + return Ok(FindDeclarationResult.Range range) | FindDeclResult.DeclFound range when System.IO.File.Exists range.FileName -> let rangeStr = range.ToString() diff --git a/src/FsAutoComplete/CodeFixes/MakeDeclarationMutable.fs b/src/FsAutoComplete/CodeFixes/MakeDeclarationMutable.fs index 3c1b5d482..0a698906d 100644 --- a/src/FsAutoComplete/CodeFixes/MakeDeclarationMutable.fs +++ b/src/FsAutoComplete/CodeFixes/MakeDeclarationMutable.fs @@ -7,6 +7,7 @@ open FsAutoComplete open FsAutoComplete.LspHelpers open FSharp.UMX +let title = "Make declaration 'mutable'" /// a codefix that makes a binding mutable when a user attempts to mutably set it let fix (getParseResultsForFile: GetParseResultsForFile) (getProjectOptionsForFile: GetProjectOptionsForFile) @@ -34,7 +35,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) return [ { File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic - Title = "Make declaration 'mutable'" + Title = title Edits = [| { Range = { Start = lspRange.Start diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 0c1931878..c03aa806a 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -692,6 +692,83 @@ let private generateUnionCasesTests state = """ ]) +let private makeDeclarationMutableTests state = + serverTestList (nameof MakeDeclarationMutable) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle MakeDeclarationMutable.title + testCaseAsync "can make decl mutable in top level assignment" <| + CodeFix.check server + """ + let x = 0 + x $0<- 1 + """ + (Diagnostics.expectCode "27") + selectCodeFix + """ + let mutable x = 0 + x <- 1 + """ + testCaseAsync "can make decl mutable in nested assignment" <| + CodeFix.check server + """ + let x = 0 + let _ = + x $0<- 1 + () + """ + (Diagnostics.expectCode "27") + selectCodeFix + """ + let mutable x = 0 + let _ = + x <- 1 + () + """ + testCaseAsync "can make decl mutable in function" <| + CodeFix.check server + """ + let count xs = + let counter = 0 + for x in xs do + counter $0<- counter + 1 + counter + """ + (Diagnostics.expectCode "27") + selectCodeFix + """ + let count xs = + let mutable counter = 0 + for x in xs do + counter <- counter + 1 + counter + """ + testCaseAsync "doesn't trigger for already mutable variable" <| + CodeFix.checkNotApplicable server + """ + let mutable x = 0 + x $0<- 1 + """ + Diagnostics.acceptAll + selectCodeFix + testCaseAsync "doesn't trigger for immutable parameter" <| + CodeFix.checkNotApplicable server + """ + let f (v: int) = + v $0<- 1 + v + """ + Diagnostics.acceptAll + selectCodeFix + testCaseAsync "doesn't trigger for immutable member parameter" <| + CodeFix.checkNotApplicable server + """ + type C() = + member _.M(v: int) + v $0<- 1 + """ + Diagnostics.acceptAll + selectCodeFix + ]) + let private makeOuterBindingRecursiveTests state = serverTestList (nameof MakeOuterBindingRecursive) state defaultConfigDto None (fun server -> [ testCaseAsync "can make the outer binding recursive when self-referential" <| @@ -924,6 +1001,7 @@ let tests state = testList "CodeFix tests" [ generateAbstractClassStubTests state generateRecordStubTests state generateUnionCasesTests state + makeDeclarationMutableTests state makeOuterBindingRecursiveTests state negationToSubtractionTests state removeUnusedBindingTests state diff --git a/test/FsAutoComplete.Tests.Lsp/GoToTests.fs b/test/FsAutoComplete.Tests.Lsp/GoToTests.fs index f9f326129..810a236ee 100644 --- a/test/FsAutoComplete.Tests.Lsp/GoToTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/GoToTests.fs @@ -7,6 +7,10 @@ open Helpers open Ionide.LanguageServerProtocol.Types open FsToolkit.ErrorHandling open FsAutoComplete +open Utils.ServerTests +open Utils.Server +open Utils.Utils +open Utils.TextEdit ///GoTo tests let private gotoTest state = @@ -542,9 +546,76 @@ let private scriptGotoTests state = "should point to the range of the definition of `testFunction`" }) ] +let private untitledGotoTests state = + serverTestList "Untitled GoTo Tests" state defaultConfigDto None (fun server -> [ + testCaseAsync "can go to variable declaration" <| async { + let (usagePos, declRange, text) = + """ + let $0x$0 = 1 + let _ = () + let a = $0x + """ + |> Text.trimTripleQuotation + |> Cursor.assertExtractRange + |> fun (decl, text) -> + let (pos, text) = + text + |> Cursor.assertExtractPosition + (pos, decl, text) + let! (doc, diags) = server |> Server.createUntitledDocument text + use doc = doc + + let p : TextDocumentPositionParams = { + TextDocument = doc.TextDocumentIdentifier + Position = usagePos + } + let! res = doc.Server.Server.TextDocumentDefinition p + match res with + | Error e -> failtestf "Request failed: %A" e + | Ok None -> failtest "Request none" + | Ok (Some (GotoResult.Multiple _)) -> failtest "Should only get one location" + | Ok (Some (GotoResult.Single r)) -> + Expect.stringEnds r.Uri doc.Uri "should navigate to source file" + Expect.equal r.Range declRange "should point to the range of variable declaration" + } + testCaseAsync "can go to function declaration" <| async { + let (usagePos, declRange, text) = + """ + let $0myFun$0 a b = a + b + let _ = () + let a = my$0Fun 1 1 + """ + |> Text.trimTripleQuotation + |> Cursor.assertExtractRange + |> fun (decl, text) -> + let (pos, text) = + text + |> Cursor.assertExtractPosition + (pos, decl, text) + let! (doc, diags) = server |> Server.createUntitledDocument text + use doc = doc + + let p : TextDocumentPositionParams = { + TextDocument = doc.TextDocumentIdentifier + Position = usagePos + } + let! res = doc.Server.Server.TextDocumentDefinition p + match res with + | Error e -> failtestf "Request failed: %A" e + | Ok None -> failtest "Request none" + | Ok (Some (GotoResult.Multiple _)) -> failtest "Should only get one location" + | Ok (Some (GotoResult.Single r)) -> + Expect.stringEnds r.Uri doc.Uri "should navigate to source file" + Expect.equal r.Range declRange "should point to the range of function declaration" + } + ]) + let tests state = testSequenced <| testList - "Go to definition tests" - [ gotoTest state - scriptGotoTests state ] + "Go to definition tests" + [ + gotoTest state + scriptGotoTests state + untitledGotoTests state + ] From 6e75b7fb9ae76e75a31b6002cc9f2896dc7883d0 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 12:57:36 +0200 Subject: [PATCH 12/29] Fix: `AddMissingEqualsToTypeDefinition` doesn't trigger * Reason: Checking for incorrect diagnostics messages * Note: CodeFix in `dotnet/fsharp` (VS) only checks for error `3360`, while previous CodeFix in FSAC did additional check for error `10` and additional matching diagnostics messages (which were wrong/outdated?) -> CodeFix now checks just for error `3360` like `dotnet/fsharp` (VS) Fix: `AddMissingEqualsToTypeDefinition` adds space on wrong side of `=` * Note: Was valid, just looked odd * Note: Wasn't visible because Fix didn't trigger Rename `MissingEquals` to `AddMissingEqualsToTypeDefinition` to align with `dotnet/fsharp` (VS) Add tests for `AddMissingEqualsToTypeDefinition` --- .../AddMissingEqualsToTypeDefinition.fs | 36 +++++++++++++++++ src/FsAutoComplete/CodeFixes/MissingEquals.fs | 39 ------------------- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 26 +++++++++++++ 4 files changed, 63 insertions(+), 40 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/AddMissingEqualsToTypeDefinition.fs delete mode 100644 src/FsAutoComplete/CodeFixes/MissingEquals.fs diff --git a/src/FsAutoComplete/CodeFixes/AddMissingEqualsToTypeDefinition.fs b/src/FsAutoComplete/CodeFixes/AddMissingEqualsToTypeDefinition.fs new file mode 100644 index 000000000..a501908e0 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddMissingEqualsToTypeDefinition.fs @@ -0,0 +1,36 @@ +module FsAutoComplete.CodeFix.AddMissingEqualsToTypeDefinition + +open FsToolkit.ErrorHandling +open FsAutoComplete.CodeFix.Navigation +open FsAutoComplete.CodeFix.Types +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +let title = "Add missing '=' to type definition" +/// a codefix that adds in missing '=' characters in type declarations +let fix (getFileLines: GetFileLines) = + Run.ifDiagnosticByCode + (Set.ofList [ "3360" ]) + (fun diagnostic codeActionParams -> + asyncResult { + let fileName = + codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + + let! lines = getFileLines fileName + let! walkPos = dec lines diagnostic.Range.Start |> Result.ofOption (fun _ -> "No walk pos") + match walkBackUntilCondition lines walkPos (System.Char.IsWhiteSpace >> not) with + | Some firstNonWhitespaceChar -> + let! insertPos = inc lines firstNonWhitespaceChar |> Result.ofOption (fun _ -> "No insert pos") + + return + [ { SourceDiagnostic = Some diagnostic + Title = title + File = codeActionParams.TextDocument + Edits = + [| { Range = { Start = insertPos; End = insertPos } + NewText = "= " } |] + Kind = FixKind.Fix } ] + | None -> return [] + } + ) diff --git a/src/FsAutoComplete/CodeFixes/MissingEquals.fs b/src/FsAutoComplete/CodeFixes/MissingEquals.fs deleted file mode 100644 index 9bbed1e91..000000000 --- a/src/FsAutoComplete/CodeFixes/MissingEquals.fs +++ /dev/null @@ -1,39 +0,0 @@ -module FsAutoComplete.CodeFix.MissingEquals - -open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix.Navigation -open FsAutoComplete.CodeFix.Types -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -/// a codefix that adds in missing '=' characters in type declarations -let fix (getFileLines: GetFileLines) = - Run.ifDiagnosticByCode - (Set.ofList [ "10"; "3360" ]) - (fun diagnostic codeActionParams -> - asyncResult { - if diagnostic.Message.Contains "Unexpected symbol '{' in type definition" - || diagnostic.Message.Contains "Unexpected keyword 'member' in type definition" then - let fileName = - codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath - - let! lines = getFileLines fileName - let! walkPos = dec lines diagnostic.Range.Start |> Result.ofOption (fun _ -> "No walk pos") - match walkBackUntilCondition lines walkPos (System.Char.IsWhiteSpace >> not) with - | Some firstNonWhitespaceChar -> - let! insertPos = inc lines firstNonWhitespaceChar |> Result.ofOption (fun _ -> "No insert pos") - - return - [ { SourceDiagnostic = Some diagnostic - Title = "Add missing '=' to type definition" - File = codeActionParams.TextDocument - Edits = - [| { Range = { Start = insertPos; End = insertPos } - NewText = " =" } |] - Kind = FixKind.Fix } ] - | None -> return [] - else - return [] - } - ) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 0c67bfa70..19b23a995 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -872,7 +872,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS tryGetParseResultsForFile commands.GetAbstractClassStub getAbstractClassStubReplacements) - MissingEquals.fix getFileLines + AddMissingEqualsToTypeDefinition.fix getFileLines NegationToSubtraction.fix getFileLines ConvertDoubleEqualsToSingleEquals.fix getRangeText ChangeEqualsInFieldTypeToColon.fix diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index c03aa806a..1175742ea 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -63,6 +63,31 @@ let private addExplicitTypeToParameterTests state = """ ]) +let private addMissingEqualsToTypeDefinitionTests state = + serverTestList (nameof AddMissingEqualsToTypeDefinition) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle AddMissingEqualsToTypeDefinition.title + testCaseAsync "can add = to record def" <| + CodeFix.check server + """ + type Person $0{ Name : string; Age : int; City : string } + """ + (Diagnostics.expectCode "3360") + selectCodeFix + """ + type Person = { Name : string; Age : int; City : string } + """ + testCaseAsync "can add = to union def" <| + CodeFix.check server + """ + type Name $0Name of string + """ + (Diagnostics.expectCode "3360") + selectCodeFix + """ + type Name = Name of string + """ + ]) + let private addMissingFunKeywordTests state = serverTestList (nameof AddMissingFunKeyword) state defaultConfigDto None (fun server -> [ testCaseAsync "can generate the fun keyword when error 10 is raised" <| @@ -987,6 +1012,7 @@ let private useMutationWhenValueIsMutableTests state = let tests state = testList "CodeFix tests" [ addExplicitTypeToParameterTests state + addMissingEqualsToTypeDefinitionTests state addMissingFunKeywordTests state addMissingInstanceMemberTests state addMissingRecKeywordTests state From 4942871e4a1b5048390ab47a3e65665c99c25ea8 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 13:06:33 +0200 Subject: [PATCH 13/29] Rename `NegationToSubtraction` to `ChangePrefixNegationToInfixSubtraction` -> align with `dotnet/fsharp` (VS) --- ...ChangePrefixNegationToInfixSubtraction.fs} | 2 +- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 38 +++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) rename src/FsAutoComplete/CodeFixes/{NegationToSubtraction.fs => ChangePrefixNegationToInfixSubtraction.fs} (94%) diff --git a/src/FsAutoComplete/CodeFixes/NegationToSubtraction.fs b/src/FsAutoComplete/CodeFixes/ChangePrefixNegationToInfixSubtraction.fs similarity index 94% rename from src/FsAutoComplete/CodeFixes/NegationToSubtraction.fs rename to src/FsAutoComplete/CodeFixes/ChangePrefixNegationToInfixSubtraction.fs index b3dea2537..0716a5242 100644 --- a/src/FsAutoComplete/CodeFixes/NegationToSubtraction.fs +++ b/src/FsAutoComplete/CodeFixes/ChangePrefixNegationToInfixSubtraction.fs @@ -1,4 +1,4 @@ -module FsAutoComplete.CodeFix.NegationToSubtraction +module FsAutoComplete.CodeFix.ChangePrefixNegationToInfixSubtraction open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Navigation diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 19b23a995..583798146 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -873,7 +873,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS commands.GetAbstractClassStub getAbstractClassStubReplacements) AddMissingEqualsToTypeDefinition.fix getFileLines - NegationToSubtraction.fix getFileLines + ChangePrefixNegationToInfixSubtraction.fix getFileLines ConvertDoubleEqualsToSingleEquals.fix getRangeText ChangeEqualsInFieldTypeToColon.fix ParenthesizeExpression.fix getRangeText diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 1175742ea..5a12ca15e 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -213,6 +213,24 @@ let private changeEqualsInFieldTypeToColonTests state = """ ]) +let private changePrefixNegationToInfixSubtractionTests state = + serverTestList (nameof ChangePrefixNegationToInfixSubtraction) state defaultConfigDto None (fun server -> [ + testCaseAsync "converts negation to subtraction" <| + CodeFix.check server + """ + let getListWithoutFirstAndLastElement list = + let l = List.length list + list[ 1 .. $0l -1 ] + """ + (Diagnostics.expectCode "3") + (CodeFix.ofKind "quickfix" >> CodeFix.withTitle ChangePrefixNegationToInfixSubtraction.title) + """ + let getListWithoutFirstAndLastElement list = + let l = List.length list + list[ 1 .. l - 1 ] + """ + ]) + let private changeTypeOfNameToNameOfTests state = serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ testCaseAsync "can suggest fix" <| @@ -816,24 +834,6 @@ let private makeOuterBindingRecursiveTests state = """ ]) -let private negationToSubtractionTests state = - serverTestList (nameof NegationToSubtraction) state defaultConfigDto None (fun server -> [ - testCaseAsync "converts negation to subtraction" <| - CodeFix.check server - """ - let getListWithoutFirstAndLastElement list = - let l = List.length list - list[ 1 .. $0l -1 ] - """ - (Diagnostics.expectCode "3") - (CodeFix.ofKind "quickfix" >> CodeFix.withTitle NegationToSubtraction.title) - """ - let getListWithoutFirstAndLastElement list = - let l = List.length list - list[ 1 .. l - 1 ] - """ - ]) - let private removeUnusedBindingTests state = let config = { defaultConfigDto with FSIExtraParameters = Some [| "--warnon:1182" |] } serverTestList (nameof RemoveUnusedBinding) state config None (fun server -> [ @@ -1018,6 +1018,7 @@ let tests state = testList "CodeFix tests" [ addMissingRecKeywordTests state addTypeToIndeterminateValueTests state changeEqualsInFieldTypeToColonTests state + changePrefixNegationToInfixSubtractionTests state changeTypeOfNameToNameOfTests state convertBangEqualsToInequalityTests state convertCSharpLambdaToFSharpTests state @@ -1029,7 +1030,6 @@ let tests state = testList "CodeFix tests" [ generateUnionCasesTests state makeDeclarationMutableTests state makeOuterBindingRecursiveTests state - negationToSubtractionTests state removeUnusedBindingTests state unusedValueTests state useTripleQuotedInterpolationTests state From 99591d59bf5b22970ef61c07e48bc5a4cb3be0a9 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 13:27:28 +0200 Subject: [PATCH 14/29] Rename `NewWithDisposables` to `AddNewKeywordToDisposableConstructorInvocation` -> align with `dotnet/fsharp` (VS) Simplify `AddNewKeywordToDisposableConstructorInvocation`: (-> align with `dotnet/fsharp`) * Trigger on diag code, instead of diag message * Just add `new ` instead of replacing disposable constructor Add tests for `AddNewKeywordToDisposableConstructorInvocation` --- ...eywordToDisposableConstructorInvocation.fs | 23 ++++++++++++ .../CodeFixes/NewWithDisposables.fs | 24 ------------- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 35 +++++++++++++++++++ 4 files changed, 59 insertions(+), 25 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs delete mode 100644 src/FsAutoComplete/CodeFixes/NewWithDisposables.fs diff --git a/src/FsAutoComplete/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs b/src/FsAutoComplete/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs new file mode 100644 index 000000000..454a1e593 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs @@ -0,0 +1,23 @@ +module FsAutoComplete.CodeFix.AddNewKeywordToDisposableConstructorInvocation + +open FsToolkit.ErrorHandling +open FsAutoComplete.CodeFix +open FsAutoComplete.CodeFix.Types +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +let title = "Add 'new'" +/// a codefix that suggests using the 'new' keyword on IDisposables +let fix (getRangeText: GetRangeText) = + Run.ifDiagnosticByCode + (Set.ofList [ "760" ]) + (fun diagnostic codeActionParams -> + AsyncResult.retn [ { SourceDiagnostic = Some diagnostic + File = codeActionParams.TextDocument + Title = title + Edits = + [| { Range = { Start = diagnostic.Range.Start; End = diagnostic.Range.Start } + NewText = $"new " } |] + Kind = FixKind.Refactor } ] + ) diff --git a/src/FsAutoComplete/CodeFixes/NewWithDisposables.fs b/src/FsAutoComplete/CodeFixes/NewWithDisposables.fs deleted file mode 100644 index e9016a21c..000000000 --- a/src/FsAutoComplete/CodeFixes/NewWithDisposables.fs +++ /dev/null @@ -1,24 +0,0 @@ -module FsAutoComplete.CodeFix.NewWithDisposables - -open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix -open FsAutoComplete.CodeFix.Types -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -/// a codefix that suggests using the 'new' keyword on IDisposables -let fix (getRangeText: GetRangeText) = - Run.ifDiagnosticByMessage - "It is recommended that objects supporting the IDisposable interface are created using the syntax" - (fun diagnostic codeActionParams -> - match getRangeText (codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath) diagnostic.Range with - | Ok errorText -> - AsyncResult.retn [ { SourceDiagnostic = Some diagnostic - File = codeActionParams.TextDocument - Title = "Add new" - Edits = - [| { Range = diagnostic.Range - NewText = $"new %s{errorText}" } |] - Kind = FixKind.Refactor } ] - | Error _ -> AsyncResult.retn []) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 583798146..ab6ce1027 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -850,7 +850,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS SuggestedIdentifier.fix RedundantQualifier.fix Run.ifEnabled (fun _ -> config.UnusedDeclarationsAnalyzer) (UnusedValue.fix getRangeText) - NewWithDisposables.fix getRangeText + AddNewKeywordToDisposableConstructorInvocation.fix getRangeText Run.ifEnabled (fun _ -> config.UnionCaseStubGeneration) (GenerateUnionCases.fix diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 5a12ca15e..2f01d874a 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -135,6 +135,40 @@ let private addMissingRecKeywordTests state = """ ]) +let private addNewKeywordToDisposableConstructorInvocationTests state = + serverTestList (nameof AddNewKeywordToDisposableConstructorInvocation) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle AddNewKeywordToDisposableConstructorInvocation.title + testCaseAsync "can add new to Disposable" <| + CodeFix.check server + """ + open System.Threading.Tasks + let _ = $0Task(fun _ -> 1) + """ + (Diagnostics.expectCode "760") + selectCodeFix + """ + open System.Threading.Tasks + let _ = new Task(fun _ -> 1) + """ + testCaseAsync "can add new to Disposable with namespace" <| + CodeFix.check server + """ + let _ = System.Threading.Tasks.$0Task(fun _ -> 1) + """ + (Diagnostics.expectCode "760") + selectCodeFix + """ + let _ = new System.Threading.Tasks.Task(fun _ -> 1) + """ + testCaseAsync "doesn't trigger for not Disposable" <| + CodeFix.checkNotApplicable server + """ + let _ = System.$0String('.', 3) + """ + Diagnostics.acceptAll + selectCodeFix + ]) + let private addTypeToIndeterminateValueTests state = serverTestList (nameof AddTypeToIndeterminateValue) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle AddTypeToIndeterminateValue.title @@ -1016,6 +1050,7 @@ let tests state = testList "CodeFix tests" [ addMissingFunKeywordTests state addMissingInstanceMemberTests state addMissingRecKeywordTests state + addNewKeywordToDisposableConstructorInvocationTests state addTypeToIndeterminateValueTests state changeEqualsInFieldTypeToColonTests state changePrefixNegationToInfixSubtractionTests state From dd350c85c0859ee618eef50103454fe283fcefc5 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 13:44:11 +0200 Subject: [PATCH 15/29] Fix: `ConvertPositionalDUToNamed` doesn't put space before wildcard Note: Was valid, just looked odd --- .../CodeFixes/ConvertPositionalDUToNamed.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/ConvertPositionalDUToNamed.fs b/src/FsAutoComplete/CodeFixes/ConvertPositionalDUToNamed.fs index a7111aac3..6b425be6a 100644 --- a/src/FsAutoComplete/CodeFixes/ConvertPositionalDUToNamed.fs +++ b/src/FsAutoComplete/CodeFixes/ConvertPositionalDUToNamed.fs @@ -98,7 +98,7 @@ let private createEdit (astField: SynPat, duField: string) : TextEdit list = { NewText = suffix; Range = endRange } ] let private createWildCard endRange (duField: string) : TextEdit = - let wildcard = $"{duField} = _;" + let wildcard = $" {duField} = _;" let range = endRange { NewText = wildcard; Range = range } diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 2f01d874a..b746ef8de 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -510,8 +510,7 @@ let private convertPositionalDUToNamedTests state = match A(1, true) with | (A(a = a; b = b;)) -> () """ - testCaseAsync "when there are new fields on the DU" <| - //ENHANCEMENT: add space before wildcard case + testCaseAsync "when there is one new field on the DU" <| CodeFix.check server """ type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char @@ -523,7 +522,19 @@ let private convertPositionalDUToNamedTests state = """ type ThirdFieldWasJustAdded = ThirdFieldWasJustAdded of a: int * b: bool * c: char - let (ThirdFieldWasJustAdded(a = a; b = b;c = _;)) = ThirdFieldWasJustAdded(1, true, 'c') + let (ThirdFieldWasJustAdded(a = a; b = b; c = _;)) = ThirdFieldWasJustAdded(1, true, 'c') + """ + testCaseAsync "when there are multiple new fields on the DU" <| + CodeFix.check server + """ + type U = U of aValue: int * boolean: int * char: char * dec: decimal * element: int + let (U($0a, b)) = failwith "..." + """ + Diagnostics.acceptAll + selectCodeFix + """ + type U = U of aValue: int * boolean: int * char: char * dec: decimal * element: int + let (U(aValue = a; boolean = b; char = _; dec = _; element = _;)) = failwith "..." """ ]) From 82fa65a83cadcee36b1aba308944d14eff02f947 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 13:59:22 +0200 Subject: [PATCH 16/29] Rename `ParenthesizeEpression` to `WrapExpressionInParentheses` -> align with `dotnet/fsharp` (VS) Simplify `WrapExpressionInParentheses`: don't replace text, but instead just add parens Add tests for `WrapExpressionInParentheses` --- .../CodeFixes/ParenthesizeExpression.fs | 23 ---------------- .../CodeFixes/WrapExpressionInParentheses.fs | 27 +++++++++++++++++++ src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 22 +++++++++++++++ 4 files changed, 50 insertions(+), 24 deletions(-) delete mode 100644 src/FsAutoComplete/CodeFixes/ParenthesizeExpression.fs create mode 100644 src/FsAutoComplete/CodeFixes/WrapExpressionInParentheses.fs diff --git a/src/FsAutoComplete/CodeFixes/ParenthesizeExpression.fs b/src/FsAutoComplete/CodeFixes/ParenthesizeExpression.fs deleted file mode 100644 index ecc095a04..000000000 --- a/src/FsAutoComplete/CodeFixes/ParenthesizeExpression.fs +++ /dev/null @@ -1,23 +0,0 @@ -module FsAutoComplete.CodeFix.ParenthesizeExpression - -open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix.Types -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -/// a codefix that parenthesizes a member expression that needs it -let fix (getRangeText: GetRangeText): CodeFix = - Run.ifDiagnosticByCode - (Set.ofList [ "597" ]) - (fun diagnostic codeActionParams -> - match getRangeText (codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath) diagnostic.Range with - | Ok erroringExpression -> - AsyncResult.retn [ { Title = "Wrap expression in parentheses" - File = codeActionParams.TextDocument - SourceDiagnostic = Some diagnostic - Edits = - [| { Range = diagnostic.Range - NewText = $"(%s{erroringExpression})" } |] - Kind = FixKind.Fix } ] - | Error _ -> AsyncResult.retn []) diff --git a/src/FsAutoComplete/CodeFixes/WrapExpressionInParentheses.fs b/src/FsAutoComplete/CodeFixes/WrapExpressionInParentheses.fs new file mode 100644 index 000000000..674a223c4 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/WrapExpressionInParentheses.fs @@ -0,0 +1,27 @@ +module FsAutoComplete.CodeFix.WrapExpressionInParentheses + +open FsToolkit.ErrorHandling +open FsAutoComplete.CodeFix.Types +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete + +let title = "Wrap expression in parentheses" +/// a codefix that parenthesizes a member expression that needs it +let fix (getRangeText: GetRangeText): CodeFix = + Run.ifDiagnosticByCode + (Set.ofList [ "597" ]) + (fun diagnostic codeActionParams -> + AsyncResult.retn [{ + Title = title + File = codeActionParams.TextDocument + SourceDiagnostic = Some diagnostic + Edits = + [| + { Range = { Start = diagnostic.Range.Start; End = diagnostic.Range.Start } + NewText = "(" } + { Range = { Start = diagnostic.Range.End; End = diagnostic.Range.End } + NewText = ")" } + |] + Kind = FixKind.Fix + }] + ) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index ab6ce1027..749df5259 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -876,7 +876,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS ChangePrefixNegationToInfixSubtraction.fix getFileLines ConvertDoubleEqualsToSingleEquals.fix getRangeText ChangeEqualsInFieldTypeToColon.fix - ParenthesizeExpression.fix getRangeText + WrapExpressionInParentheses.fix getRangeText RefCellAccesToNot.fix tryGetParseResultsForFile UseSafeCastInsteadOfUnsafe.fix getRangeText MakeDeclarationMutable.fix tryGetParseResultsForFile tryGetProjectOptions diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index b746ef8de..448394dde 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1054,6 +1054,27 @@ let private useMutationWhenValueIsMutableTests state = selectCodeFix ]) +let private wrapExpressionInParenthesesTests state = + serverTestList (nameof WrapExpressionInParentheses) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle WrapExpressionInParentheses.title + testCaseAsync "can add parenthesize expression" <| + CodeFix.check server + """ + printfn "%b" System.String.$0IsNullOrWhiteSpace("foo") + """ + (Diagnostics.expectCode "597") + selectCodeFix + """ + printfn "%b" (System.String.IsNullOrWhiteSpace("foo")) + """ + testCaseAsync "doesn't trigger for expression in parens" <| + CodeFix.checkNotApplicable server + """ + printfn "%b" (System.String.$0IsNullOrWhiteSpace("foo")) + """ + Diagnostics.acceptAll + selectCodeFix + ]) let tests state = testList "CodeFix tests" [ addExplicitTypeToParameterTests state @@ -1080,4 +1101,5 @@ let tests state = testList "CodeFix tests" [ unusedValueTests state useTripleQuotedInterpolationTests state useMutationWhenValueIsMutableTests state + wrapExpressionInParenthesesTests state ] From 0d8239e784605565c6fe4a12c765a12e53bf86b0 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 18:42:52 +0200 Subject: [PATCH 17/29] Rename `RedundantQualifier` to `RemoveRedundantQualifier` Note: called `SimplifyName` in `dotnet/fsharp` (VS). But root diag message isn't part of normal F# compiler diags, but instead requires custom analyzer. Is called `SimplifyNameAnalyzer` in FSAC, but returns different message than `dotnet/fsharp` ('This qualifier is redundant' vs. 'Name can be simplified'). CodeFix name and title reflect that difference ('Remove redundant qualifier' vs. 'Simplify name `xxx`') -> Might be reasonable to align, but for now: Is kept the way it is Add tests for `RedundantQualifier` --- ...alifier.fs => RemoveRedundantQualifier.fs} | 5 ++-- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) rename src/FsAutoComplete/CodeFixes/{RedundantQualifier.fs => RemoveRedundantQualifier.fs} (82%) diff --git a/src/FsAutoComplete/CodeFixes/RedundantQualifier.fs b/src/FsAutoComplete/CodeFixes/RemoveRedundantQualifier.fs similarity index 82% rename from src/FsAutoComplete/CodeFixes/RedundantQualifier.fs rename to src/FsAutoComplete/CodeFixes/RemoveRedundantQualifier.fs index 7d68a84dc..284ff29df 100644 --- a/src/FsAutoComplete/CodeFixes/RedundantQualifier.fs +++ b/src/FsAutoComplete/CodeFixes/RemoveRedundantQualifier.fs @@ -1,10 +1,11 @@ -module FsAutoComplete.CodeFix.RedundantQualifier +module FsAutoComplete.CodeFix.RemoveRedundantQualifier open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix open FsAutoComplete.CodeFix.Types open Ionide.LanguageServerProtocol.Types +let title = "Remove redundant qualifier" /// a codefix that removes unnecessary qualifiers from an identifier let fix = Run.ifDiagnosticByMessage @@ -14,6 +15,6 @@ let fix = [| { Range = diagnostic.Range NewText = "" } |] File = codeActionParams.TextDocument - Title = "Remove redundant qualifier" + Title = title SourceDiagnostic = Some diagnostic Kind = FixKind.Refactor } ]) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 749df5259..9f6026f1f 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -848,7 +848,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS (fun _ -> config.ResolveNamespaces) (ResolveNamespace.fix tryGetParseResultsForFile commands.GetNamespaceSuggestions) SuggestedIdentifier.fix - RedundantQualifier.fix + RemoveRedundantQualifier.fix Run.ifEnabled (fun _ -> config.UnusedDeclarationsAnalyzer) (UnusedValue.fix getRangeText) AddNewKeywordToDisposableConstructorInvocation.fix getRangeText Run.ifEnabled diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 448394dde..bb8e92039 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -879,6 +879,31 @@ let private makeOuterBindingRecursiveTests state = """ ]) +let private removeRedundantQualifierTests state = + let config = { defaultConfigDto with SimplifyNameAnalyzer = Some true } + serverTestList (nameof RemoveRedundantQualifier) state config None (fun server -> [ + let selectCodeFix = CodeFix.withTitle RemoveRedundantQualifier.title + testCaseAsync "can remove redundant namespace" <| + CodeFix.check server + """ + open System + let _ = $0System.String.IsNullOrWhiteSpace "foo" + """ + Diagnostics.acceptAll + selectCodeFix + """ + open System + let _ = String.IsNullOrWhiteSpace "foo" + """ + testCaseAsync "doesn't remove necessary namespace" <| + CodeFix.checkNotApplicable server + """ + let _ = $0System.String.IsNullOrWhiteSpace "foo" + """ + Diagnostics.acceptAll + selectCodeFix + ]) + let private removeUnusedBindingTests state = let config = { defaultConfigDto with FSIExtraParameters = Some [| "--warnon:1182" |] } serverTestList (nameof RemoveUnusedBinding) state config None (fun server -> [ @@ -1097,6 +1122,7 @@ let tests state = testList "CodeFix tests" [ generateUnionCasesTests state makeDeclarationMutableTests state makeOuterBindingRecursiveTests state + removeRedundantQualifierTests state removeUnusedBindingTests state unusedValueTests state useTripleQuotedInterpolationTests state From 68dfb1914b3d863070f562a75c25bb48979f4825 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 18:54:10 +0200 Subject: [PATCH 18/29] Rename `RefCellAccesToNot` to `ChangeRefCellDerefToNot` -> ~ align with `dotnet/fsharp` (VS) (`ChangeRefCellDerefToNotExpression`) Add tests for `ChangeRefCellDerefToNot` --- ...essToNot.fs => ChangeRefCellDerefToNot.fs} | 5 ++- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 42 +++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) rename src/FsAutoComplete/CodeFixes/{RefCellAccessToNot.fs => ChangeRefCellDerefToNot.fs} (89%) diff --git a/src/FsAutoComplete/CodeFixes/RefCellAccessToNot.fs b/src/FsAutoComplete/CodeFixes/ChangeRefCellDerefToNot.fs similarity index 89% rename from src/FsAutoComplete/CodeFixes/RefCellAccessToNot.fs rename to src/FsAutoComplete/CodeFixes/ChangeRefCellDerefToNot.fs index 5fb38bb30..017955784 100644 --- a/src/FsAutoComplete/CodeFixes/RefCellAccessToNot.fs +++ b/src/FsAutoComplete/CodeFixes/ChangeRefCellDerefToNot.fs @@ -1,4 +1,4 @@ -module FsAutoComplete.CodeFix.RefCellAccesToNot +module FsAutoComplete.CodeFix.ChangeRefCellDerefToNot open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types @@ -6,6 +6,7 @@ open Ionide.LanguageServerProtocol.Types open FsAutoComplete open FsAutoComplete.LspHelpers +let title = "Use 'not' to negate expression" /// a codefix that changes a ref cell deref (!) to a call to 'not' let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix = Run.ifDiagnosticByCode @@ -22,7 +23,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix = | Some derefRange -> return [ { SourceDiagnostic = Some diagnostic - Title = "Use 'not' to negate expression" + Title = title File = codeActionParams.TextDocument Edits = [| { Range = fcsRangeToLsp derefRange diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 9f6026f1f..d50d20ab7 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -877,7 +877,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS ConvertDoubleEqualsToSingleEquals.fix getRangeText ChangeEqualsInFieldTypeToColon.fix WrapExpressionInParentheses.fix getRangeText - RefCellAccesToNot.fix tryGetParseResultsForFile + ChangeRefCellDerefToNot.fix tryGetParseResultsForFile UseSafeCastInsteadOfUnsafe.fix getRangeText MakeDeclarationMutable.fix tryGetParseResultsForFile tryGetProjectOptions UseMutationWhenValueIsMutable.fix tryGetParseResultsForFile diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index bb8e92039..3e86d5413 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -265,6 +265,47 @@ let private changePrefixNegationToInfixSubtractionTests state = """ ]) +let private changeRefCellDerefToNotTests state = + serverTestList (nameof ChangeRefCellDerefToNot) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ChangeRefCellDerefToNot.title + testCaseAsync "can change simple deref to not" <| + CodeFix.check server + """ + let x = 1 + !$0x + """ + (Diagnostics.expectCode "1") + selectCodeFix + """ + let x = 1 + not x + """ + testCaseAsync "can change simple deref with parens to not" <| + CodeFix.check server + """ + let x = 1 + !($0x) + """ + (Diagnostics.expectCode "1") + selectCodeFix + """ + let x = 1 + not (x) + """ + testCaseAsync "can change deref of binary expr to not" <| + CodeFix.check server + """ + let x = 1 + !($0x = false) + """ + (Diagnostics.expectCode "1") + selectCodeFix + """ + let x = 1 + not (x = false) + """ + ]) + let private changeTypeOfNameToNameOfTests state = serverTestList (nameof ChangeTypeOfNameToNameOf) state defaultConfigDto None (fun server -> [ testCaseAsync "can suggest fix" <| @@ -1111,6 +1152,7 @@ let tests state = testList "CodeFix tests" [ addTypeToIndeterminateValueTests state changeEqualsInFieldTypeToColonTests state changePrefixNegationToInfixSubtractionTests state + changeRefCellDerefToNotTests state changeTypeOfNameToNameOfTests state convertBangEqualsToInequalityTests state convertCSharpLambdaToFSharpTests state From 8c13b8e043a8850be60fce72ebce34ddcd75ea6a Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 19:13:18 +0200 Subject: [PATCH 19/29] Add tests for `RemoveUnnecessaryReturnOrYield` Note: `RemoveReturnOrYield` in `dotnet/fsharp` (VS) --- .../RemoveUnnecessaryReturnOrYield.fs | 10 +-- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 62 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/RemoveUnnecessaryReturnOrYield.fs b/src/FsAutoComplete/CodeFixes/RemoveUnnecessaryReturnOrYield.fs index 3237cafde..a877e5e14 100644 --- a/src/FsAutoComplete/CodeFixes/RemoveUnnecessaryReturnOrYield.fs +++ b/src/FsAutoComplete/CodeFixes/RemoveUnnecessaryReturnOrYield.fs @@ -4,9 +4,9 @@ open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types open Ionide.LanguageServerProtocol.Types open FsAutoComplete -open FsAutoComplete.CodeFix.Navigation open FsAutoComplete.LspHelpers +let title keyword = $"Remove '%s{keyword}'" /// a codefix that removes 'return' or 'yield' (or bang-variants) when the compiler says they're not necessary let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText): CodeFix = Run.ifDiagnosticByCode @@ -29,13 +29,13 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineTe let! title = if errorText.StartsWith "return!" - then Ok "Remove 'return!'" + then Ok (title "return!") elif errorText.StartsWith "yield!" - then Ok "Remove 'yield!'" + then Ok (title "yield!") elif errorText.StartsWith "return" - then Ok "Remove 'return'" + then Ok (title "return") elif errorText.StartsWith "yield" - then Ok "Remove 'yield'" + then Ok (title "yield") else Error "unknown start token for remove return or yield codefix" return diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 3e86d5413..368280e1f 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -945,6 +945,67 @@ let private removeRedundantQualifierTests state = selectCodeFix ]) +let private removeUnnecessaryReturnOrYieldTests state = + serverTestList (nameof RemoveUnnecessaryReturnOrYield) state defaultConfigDto None (fun server -> [ + testCaseAsync "can remove return" <| + CodeFix.check server + """ + let f x = + $0return x + """ + (Diagnostics.expectCode "748") + (CodeFix.withTitle (RemoveUnnecessaryReturnOrYield.title "return")) + """ + let f x = + x + """ + testCaseAsync "can remove return!" <| + CodeFix.check server + """ + let f x = + $0return! x + """ + (Diagnostics.expectCode "748") + (CodeFix.withTitle (RemoveUnnecessaryReturnOrYield.title "return!")) + """ + let f x = + x + """ + testCaseAsync "can remove yield" <| + CodeFix.check server + """ + let f x = + $0yield x + """ + (Diagnostics.expectCode "747") + (CodeFix.withTitle (RemoveUnnecessaryReturnOrYield.title "yield")) + """ + let f x = + x + """ + testCaseAsync "can remove yield!" <| + CodeFix.check server + """ + let f x = + $0yield! x + """ + (Diagnostics.expectCode "747") + (CodeFix.withTitle (RemoveUnnecessaryReturnOrYield.title "yield!")) + """ + let f x = + x + """ + testCaseAsync "doesn't trigger in seq" <| + CodeFix.checkNotApplicable server + """ + let f x = seq { + $0yield x + } + """ + (Diagnostics.acceptAll) + (CodeFix.withTitle (RemoveUnnecessaryReturnOrYield.title "yield")) + ]) + let private removeUnusedBindingTests state = let config = { defaultConfigDto with FSIExtraParameters = Some [| "--warnon:1182" |] } serverTestList (nameof RemoveUnusedBinding) state config None (fun server -> [ @@ -1165,6 +1226,7 @@ let tests state = testList "CodeFix tests" [ makeDeclarationMutableTests state makeOuterBindingRecursiveTests state removeRedundantQualifierTests state + removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state unusedValueTests state useTripleQuotedInterpolationTests state From d82e22a59758f9eb88e6812edfdcd1fdc7c74ad2 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 19:41:49 +0200 Subject: [PATCH 20/29] Change: Trigger `ChangeDerefBangToValue` on Diag `3370` * prev: always check: cursor on ref expression that's accessed with `!` * now: trigger on diag `3370` -> simpler (and most likely faster) * But change in behaviour: * prev: trigger on ref expression * now: trigger on bang (`!` -> location of diag) Example: ```fsharp let fr a = ref a let v = !(fr 5) // ^^^^^^ prev trigger range // ^ new trigger range ``` I think that's ok: bang gets replaced -> trigger ON bang Rename `ReplaceBangWithValueFunction` to `ChangeDerefBangToValue` * combination of old name and `dotnet/fsharp` (VS) (`ChangeDerefToValueRefactoring`) Add tests for `ChangeDerefBangToValue` --- .../CodeFixes/ChangeDerefBangToValue.fs | 82 +++++++++++++++++++ .../CodeFixes/ReplaceBangWithValueFunction.fs | 29 ------- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 66 +++++++++++++++ 4 files changed, 149 insertions(+), 30 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs delete mode 100644 src/FsAutoComplete/CodeFixes/ReplaceBangWithValueFunction.fs diff --git a/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs b/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs new file mode 100644 index 000000000..86b7937b7 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs @@ -0,0 +1,82 @@ +/// replace use of ! operator on ref cells with calls to .Value +module FsAutoComplete.CodeFix.ChangeDerefBangToValue + +open FsToolkit.ErrorHandling +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text.Range +open FSharp.UMX + +// let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText): CodeFix = +// fun codeActionParams -> +// asyncResult { +// let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath +// let selectionRange = protocolRangeToRange (codeActionParams.TextDocument.GetFilePath()) codeActionParams.Range +// let! parseResults, line, lines = getParseResultsForFile fileName selectionRange.Start +// let! derefRange = parseResults.GetParseResults.TryRangeOfRefCellDereferenceContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No deref found at that pos") +// let! exprRange = parseResults.GetParseResults.TryRangeOfExpressionBeingDereferencedContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No expr found at that pos") +// let combinedRange = FSharp.Compiler.Text.Range.unionRanges derefRange exprRange +// let protocolRange = fcsRangeToLsp combinedRange +// let! badString = getLineText lines protocolRange +// let replacementString = badString.[1..] + ".Value" +// return [ +// { Title = title +// File = codeActionParams.TextDocument +// SourceDiagnostic = None +// Kind = FixKind.Refactor +// Edits = [| { Range = protocolRange +// NewText = replacementString } |] } +// ] +// } + +/// adopted from `dotnet/fsharp` -> `FSharp.Compiler.CodeAnalysis.FSharpParseFileResults.TryRangeOfExpressionBeingDereferencedContainingPos` +let private tryGetRangeOfDeref input derefPos = + SyntaxTraversal.Traverse(derefPos, input, { new SyntaxVisitorBase<_>() with + member _.VisitExpr(_, _, defaultTraverse, expr) = + match expr with + | SynExpr.App(_, false, SynExpr.Ident funcIdent, expr, _) -> + if funcIdent.idText = "op_Dereference" && rangeContainsPos funcIdent.idRange derefPos then + Some (funcIdent.idRange, expr.Range) + else + None + | _ -> defaultTraverse expr }) + +let title = "Use `.Value` instead of dereference operator" +let fix + (getParseResultsForFile: GetParseResultsForFile) + (getLineText: GetLineText) + : CodeFix = + Run.ifDiagnosticByCode + (Set.ofList ["3370"]) + (fun diagnostic codeActionParams -> asyncResult { + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + + let derefOpRange = protocolRangeToRange (UMX.untag fileName) diagnostic.Range + let! parseResults, _, _ = getParseResultsForFile fileName derefOpRange.Start + + let! (derefOpRange', exprRange) = + tryGetRangeOfDeref parseResults.GetParseResults.ParseTree derefOpRange.End + |> Result.ofOption (fun _ -> "No deref found at that pos") + assert(derefOpRange = derefOpRange') + + return [ + { + Title = title + File = codeActionParams.TextDocument + SourceDiagnostic = None + Kind = FixKind.Refactor + Edits = [| + // remove leading `!` (and whitespaces after `!`) + { Range = { Start = fcsPosToLsp derefOpRange'.Start; End = fcsPosToLsp exprRange.Start } + NewText = "" } + // Append trailing `.Value` + { Range = + let lspPos = fcsPosToLsp exprRange.End + { Start = lspPos; End = lspPos } + NewText = ".Value" } + |] + } + ] + }) diff --git a/src/FsAutoComplete/CodeFixes/ReplaceBangWithValueFunction.fs b/src/FsAutoComplete/CodeFixes/ReplaceBangWithValueFunction.fs deleted file mode 100644 index a77d648e6..000000000 --- a/src/FsAutoComplete/CodeFixes/ReplaceBangWithValueFunction.fs +++ /dev/null @@ -1,29 +0,0 @@ -/// replace use of ! operator on ref cells with calls to .Value -module FsAutoComplete.CodeFix.ReplaceBangWithValueFunction - -open FsToolkit.ErrorHandling -open FsAutoComplete.CodeFix.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText): CodeFix = - fun codeActionParams -> - asyncResult { - let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath - let selectionRange = protocolRangeToRange (codeActionParams.TextDocument.GetFilePath()) codeActionParams.Range - let! parseResults, line, lines = getParseResultsForFile fileName selectionRange.Start - let! derefRange = parseResults.GetParseResults.TryRangeOfRefCellDereferenceContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No deref found at that pos") - let! exprRange = parseResults.GetParseResults.TryRangeOfExpressionBeingDereferencedContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No expr found at that pos") - let combinedRange = FSharp.Compiler.Text.Range.unionRanges derefRange exprRange - let protocolRange = fcsRangeToLsp combinedRange - let! badString = getLineText lines protocolRange - let replacementString = badString.[1..] + ".Value" - return [ - { Title = "Use `.Value` instead of dereference operator" - File = codeActionParams.TextDocument - SourceDiagnostic = None - Kind = FixKind.Refactor - Edits = [| { Range = protocolRange - NewText = replacementString } |] } - ] - } diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index d50d20ab7..95aeaf2ee 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -888,7 +888,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS MakeOuterBindingRecursive.fix tryGetParseResultsForFile getLineText AddMissingRecKeyword.fix getFileLines getLineText ConvertBangEqualsToInequality.fix getRangeText - ReplaceBangWithValueFunction.fix tryGetParseResultsForFile getLineText + ChangeDerefBangToValue.fix tryGetParseResultsForFile getLineText RemoveUnusedBinding.fix tryGetParseResultsForFile AddTypeToIndeterminateValue.fix tryGetParseResultsForFile tryGetProjectOptions ChangeTypeOfNameToNameOf.fix tryGetParseResultsForFile diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 368280e1f..9d19aba5d 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -216,6 +216,71 @@ let private addTypeToIndeterminateValueTests state = """ ]) +let private changeDerefBangToValueTests state = + serverTestList (nameof ChangeDerefBangToValue) state defaultConfigDto None (fun server -> [ + let selectCodeFix = CodeFix.withTitle ChangeDerefBangToValue.title + testCaseAsync "can replace ! with .Value" <| + CodeFix.check server + """ + let rv = ref 5 + let v = $0!rv + """ + (Diagnostics.expectCode "3370") + selectCodeFix + """ + let rv = ref 5 + let v = rv.Value + """ + testCaseAsync "can replace ! with .Value when parens" <| + CodeFix.check server + """ + let rv = ref 5 + let v = $0!(rv) + """ + (Diagnostics.expectCode "3370") + selectCodeFix + """ + let rv = ref 5 + let v = (rv).Value + """ + testCaseAsync "can replace ! with .Value when function in parens" <| + CodeFix.check server + """ + let fr a = ref a + let v = $0!(fr 5) + """ + (Diagnostics.expectCode "3370") + selectCodeFix + """ + let fr a = ref a + let v = (fr 5).Value + """ + testCaseAsync "can replace ! with .Value when space between ! and variable" <| + CodeFix.check server + """ + let rv = ref 5 + let v = $0! rv + """ + (Diagnostics.expectCode "3370") + selectCodeFix + """ + let rv = ref 5 + let v = rv.Value + """ + testCaseAsync "can replace ! with .Value when when parens and space between ! and variable" <| + CodeFix.check server + """ + let rv = ref 5 + let v = $0! (rv) + """ + (Diagnostics.expectCode "3370") + selectCodeFix + """ + let rv = ref 5 + let v = (rv).Value + """ + ]) + let private changeEqualsInFieldTypeToColonTests state = serverTestList (nameof ChangeEqualsInFieldTypeToColon) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ChangeEqualsInFieldTypeToColon.title @@ -1211,6 +1276,7 @@ let tests state = testList "CodeFix tests" [ addMissingRecKeywordTests state addNewKeywordToDisposableConstructorInvocationTests state addTypeToIndeterminateValueTests state + changeDerefBangToValueTests state changeEqualsInFieldTypeToColonTests state changePrefixNegationToInfixSubtractionTests state changeRefCellDerefToNotTests state From c6012ee63039185b7ed1320829be85bc158c7992 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 20:51:03 +0200 Subject: [PATCH 21/29] Rename `UseSafeCastInsteadOfUnsafe` to `ChangeDowncastToUpcast` in `dotnet/fsharp` (VS): `ChangeToUpcast` Add tests for `ChangeDowncastToUpcast` --- ...dOfUnsafe.fs => ChangeDowncastToUpcast.fs} | 8 ++-- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 40 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) rename src/FsAutoComplete/CodeFixes/{UseSafeCastInsteadOfUnsafe.fs => ChangeDowncastToUpcast.fs} (86%) diff --git a/src/FsAutoComplete/CodeFixes/UseSafeCastInsteadOfUnsafe.fs b/src/FsAutoComplete/CodeFixes/ChangeDowncastToUpcast.fs similarity index 86% rename from src/FsAutoComplete/CodeFixes/UseSafeCastInsteadOfUnsafe.fs rename to src/FsAutoComplete/CodeFixes/ChangeDowncastToUpcast.fs index db61a88ee..e7718d3fa 100644 --- a/src/FsAutoComplete/CodeFixes/UseSafeCastInsteadOfUnsafe.fs +++ b/src/FsAutoComplete/CodeFixes/ChangeDowncastToUpcast.fs @@ -1,4 +1,4 @@ -module FsAutoComplete.CodeFix.UseSafeCastInsteadOfUnsafe +module FsAutoComplete.CodeFix.ChangeDowncastToUpcast open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix.Types @@ -6,6 +6,8 @@ open Ionide.LanguageServerProtocol.Types open FsAutoComplete open FsAutoComplete.LspHelpers +let titleUpcastOperator = "Use ':>' operator" +let titleUpcastFunction = "Use 'upcast' function" /// a codefix that replaces unsafe casts with safe casts let fix (getRangeText: GetRangeText): CodeFix = Run.ifDiagnosticByCode @@ -23,7 +25,7 @@ let fix (getRangeText: GetRangeText): CodeFix = | true, false -> AsyncResult.retn [ { File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic - Title = "Use ':>' operator" + Title = titleUpcastOperator Edits = [| { Range = diagnostic.Range NewText = expressionText.Replace(":?>", ":>") } |] @@ -31,7 +33,7 @@ let fix (getRangeText: GetRangeText): CodeFix = | false, true -> AsyncResult.retn [ { File = codeActionParams.TextDocument SourceDiagnostic = Some diagnostic - Title = "Use 'upcast' function" + Title = titleUpcastFunction Edits = [| { Range = diagnostic.Range NewText = expressionText.Replace("downcast", "upcast") } |] diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 95aeaf2ee..3cbdf1c05 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -878,7 +878,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS ChangeEqualsInFieldTypeToColon.fix WrapExpressionInParentheses.fix getRangeText ChangeRefCellDerefToNot.fix tryGetParseResultsForFile - UseSafeCastInsteadOfUnsafe.fix getRangeText + ChangeDowncastToUpcast.fix getRangeText MakeDeclarationMutable.fix tryGetParseResultsForFile tryGetProjectOptions UseMutationWhenValueIsMutable.fix tryGetParseResultsForFile ConvertInvalidRecordToAnonRecord.fix tryGetParseResultsForFile diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 9d19aba5d..00eb2e040 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -281,6 +281,45 @@ let private changeDerefBangToValueTests state = """ ]) +let private changeDowncastToUpcastTests state = + serverTestList (nameof ChangeDowncastToUpcast) state defaultConfigDto None (fun server -> [ + let selectOperatorCodeFix = CodeFix.withTitle ChangeDowncastToUpcast.titleUpcastOperator + let selectFunctionCodeFix = CodeFix.withTitle ChangeDowncastToUpcast.titleUpcastFunction + testCaseAsync "can change :?> to :>" <| + CodeFix.check server + """ + type I = interface end + type C() = interface I + + let v: I = C() $0:?> I + """ + (Diagnostics.expectCode "3198") + selectOperatorCodeFix + """ + type I = interface end + type C() = interface I + + let v: I = C() :> I + """ + testCaseAsync "can change downcast to upcast" <| + CodeFix.check server + """ + type I = interface end + type C() = interface I + + let v: I = $0downcast C() + """ + (Diagnostics.expectCode "3198") + selectFunctionCodeFix + """ + type I = interface end + type C() = interface I + + let v: I = upcast C() + """ + () + ]) + let private changeEqualsInFieldTypeToColonTests state = serverTestList (nameof ChangeEqualsInFieldTypeToColon) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ChangeEqualsInFieldTypeToColon.title @@ -1277,6 +1316,7 @@ let tests state = testList "CodeFix tests" [ addNewKeywordToDisposableConstructorInvocationTests state addTypeToIndeterminateValueTests state changeDerefBangToValueTests state + changeDowncastToUpcastTests state changeEqualsInFieldTypeToColonTests state changePrefixNegationToInfixSubtractionTests state changeRefCellDerefToNotTests state From e5108026447979887a2a650f14573c313d125682 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 20:58:25 +0200 Subject: [PATCH 22/29] Rename test to match CodeFix --- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 00eb2e040..1f8e520ed 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -439,7 +439,7 @@ let private convertBangEqualsToInequalityTests state = """ ]) -let private convertCSharpLambdaToFSharpTests state = +let private ConvertCSharpLambdaToFSharpLambdaTests state = serverTestList (nameof ConvertCSharpLambdaToFSharpLambda) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle ConvertCSharpLambdaToFSharpLambda.title testCaseAsync "can convert csharp lambda in variable assignment with cursor on input" <| @@ -1322,7 +1322,7 @@ let tests state = testList "CodeFix tests" [ changeRefCellDerefToNotTests state changeTypeOfNameToNameOfTests state convertBangEqualsToInequalityTests state - convertCSharpLambdaToFSharpTests state + ConvertCSharpLambdaToFSharpLambdaTests state convertDoubleEqualsToSingleEqualsTests state convertInvalidRecordToAnonRecordTests state convertPositionalDUToNamedTests state From d9fdfe456c22d69a2bf886e48b3345bfc1bccc98 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 21:00:39 +0200 Subject: [PATCH 23/29] Fix: Incorrect sorted tests --- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 1f8e520ed..185ebefc5 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1214,21 +1214,6 @@ let private unusedValueTests state = """ ]) -let private useTripleQuotedInterpolationTests state = - serverTestList (nameof UseTripleQuotedInterpolation) state defaultConfigDto None (fun server -> [ - testCaseAsync "converts erroring single-quoted interpolation to triple-quoted" <| - CodeFix.check server - """ - let a = $":^) {if true then $0"y" else "n"} d" - """ - (Diagnostics.expectCode "3373") - (CodeFix.ofKind "quickfix" >> CodeFix.withTitle UseTripleQuotedInterpolation.title) - // cannot use triple quotes string here: ends with `"""` -> cannot use in string - @" - let a = $"""""":^) {if true then ""y"" else ""n""} d"""""" - " - ]) - let private useMutationWhenValueIsMutableTests state = serverTestList (nameof UseMutationWhenValueIsMutable) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle UseMutationWhenValueIsMutable.title @@ -1285,6 +1270,21 @@ let private useMutationWhenValueIsMutableTests state = selectCodeFix ]) +let private useTripleQuotedInterpolationTests state = + serverTestList (nameof UseTripleQuotedInterpolation) state defaultConfigDto None (fun server -> [ + testCaseAsync "converts erroring single-quoted interpolation to triple-quoted" <| + CodeFix.check server + """ + let a = $":^) {if true then $0"y" else "n"} d" + """ + (Diagnostics.expectCode "3373") + (CodeFix.ofKind "quickfix" >> CodeFix.withTitle UseTripleQuotedInterpolation.title) + // cannot use triple quotes string here: ends with `"""` -> cannot use in string + @" + let a = $"""""":^) {if true then ""y"" else ""n""} d"""""" + " + ]) + let private wrapExpressionInParenthesesTests state = serverTestList (nameof WrapExpressionInParentheses) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle WrapExpressionInParentheses.title @@ -1335,7 +1335,7 @@ let tests state = testList "CodeFix tests" [ removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state unusedValueTests state - useTripleQuotedInterpolationTests state useMutationWhenValueIsMutableTests state + useTripleQuotedInterpolationTests state wrapExpressionInParenthesesTests state ] From 972fe48fdf7b684ca5fa1ace893fda3d324dff57 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 21:24:37 +0200 Subject: [PATCH 24/29] Fix: `ResolveNamespace` doesn't trigger when target is in last line `let x = Min(1,2)` (NO newline at end) -> Cursor on `M` -> `ResolveNamespace` throws exception. Also prevents other CodeFixes from running (for example: above doesn't produce Fix 'Replace with min') No other tests: * Tests for `ResolveNamespace` are a bit complex because of nested modules (-> where to `open`) * Additional: Best to unify with `AutoOpen` (Tests in `CompletionTests.fs` -> `autoOpenTests`). But `ResolveNamespace` and `AutoOpen` currently use different implementations and `open` in different locations. -> postpone unification and tests --- .../CodeFixes/ResolveNamespace.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/FsAutoComplete/CodeFixes/ResolveNamespace.fs b/src/FsAutoComplete/CodeFixes/ResolveNamespace.fs index 3d2012add..70694d5cd 100644 --- a/src/FsAutoComplete/CodeFixes/ResolveNamespace.fs +++ b/src/FsAutoComplete/CodeFixes/ResolveNamespace.fs @@ -78,7 +78,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (getNamespaceSuggestion let edits = [| yield insertLine docLine lineStr - if text.GetLineString(docLine + 1).Trim() <> "" then yield insertLine (docLine + 1) "" + if text.GetLineCount() < docLine + 1 && text.GetLineString(docLine + 1).Trim() <> "" then yield insertLine (docLine + 1) "" if (ctx.Pos.Column = 0 || ctx.ScopeKind = ScopeKind.Namespace) && docLine > 0 && not (text.GetLineString(docLine - 1).StartsWith "open") then diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 185ebefc5..0adf17eff 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1154,6 +1154,28 @@ let private removeUnusedBindingTests state = """ ]) +let private resolveNamespaceTests state = + let config = { defaultConfigDto with ResolveNamespaces = Some true } + serverTestList (nameof ResolveNamespace) state config None (fun server -> [ + testCaseAsync "doesn't fail when target not in last line" <| + CodeFix.checkApplicable server + """ + let x = $0Min(2.0, 1.0) + """ // Note: new line at end! + (Diagnostics.log >> Diagnostics.acceptAll) + (CodeFix.log >> CodeFix.matching (fun ca -> ca.Title.StartsWith "open") >> Array.take 1) + testCaseAsync "doesn't fail when target in last line" <| + CodeFix.checkApplicable server + "let x = $0Min(2.0, 1.0)" // Note: No new line at end! + (Diagnostics.log >> Diagnostics.acceptAll) + (CodeFix.log >> CodeFix.matching (fun ca -> ca.Title.StartsWith "open") >> Array.take 1) + + //TODO: Implement & unify with `Completion.AutoOpen` (`CompletionTests.fs`) + // Issues: + // * Complex because of nesting modules (-> where to open) + // * Different open locations of CodeFix and AutoOpen + ]) + let private unusedValueTests state = let config = { defaultConfigDto with UnusedDeclarationsAnalyzer = Some true } serverTestList (nameof UnusedValue) state config None (fun server -> [ @@ -1334,6 +1356,7 @@ let tests state = testList "CodeFix tests" [ removeRedundantQualifierTests state removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state + resolveNamespaceTests state unusedValueTests state useMutationWhenValueIsMutableTests state useTripleQuotedInterpolationTests state From 2b43d6121a67146ca50a3292894052c8c9dd1b74 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sat, 9 Apr 2022 21:33:23 +0200 Subject: [PATCH 25/29] Fix: `ReplaceWithSuggestion` surrounds identifiers in double-backticks in additional double-backticks (-> 4 backticks) Rename `SuggestedIdentifier` to `ReplaceWithSuggestion` -> align with `dotnet/fsharp` (VS) Note: `dotnet/fsharp` checks for Diags `"FS0039"; "FS1129"; "FS0495"`, while FSAC checks for `"Maybe you want one of the following:"`. (Test for message is kept: is more general) Add tests for `ReplaceWithSuggestion` --- .../CodeFixes/ReplaceWithSuggestion.fs | 30 ++++++ .../CodeFixes/SuggestedIdentifier.fs | 31 ------ src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 94 +++++++++++++++++++ 4 files changed, 125 insertions(+), 32 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/ReplaceWithSuggestion.fs delete mode 100644 src/FsAutoComplete/CodeFixes/SuggestedIdentifier.fs diff --git a/src/FsAutoComplete/CodeFixes/ReplaceWithSuggestion.fs b/src/FsAutoComplete/CodeFixes/ReplaceWithSuggestion.fs new file mode 100644 index 000000000..86322b9e7 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/ReplaceWithSuggestion.fs @@ -0,0 +1,30 @@ +module FsAutoComplete.CodeFix.ReplaceWithSuggestion + +open FsAutoComplete.CodeFix +open FsAutoComplete.CodeFix.Types +open FsToolkit.ErrorHandling +open FsAutoComplete +open FSharp.Compiler.Syntax + +let title suggestion = $"Replace with '%s{suggestion}'" +/// a codefix that replaces the use of an unknown identifier with a suggested identifier +let fix = + Run.ifDiagnosticByMessage + "Maybe you want one of the following:" + (fun diagnostic codeActionParams -> + diagnostic.Message.Split('\n').[1..] + |> Array.map + (fun suggestion -> + let suggestion = + suggestion.Trim() + |> PrettyNaming.AddBackticksToIdentifierIfNeeded + + { Edits = + [| { Range = diagnostic.Range + NewText = suggestion } |] + Title = title suggestion + File = codeActionParams.TextDocument + SourceDiagnostic = Some diagnostic + Kind = FixKind.Fix }) + |> Array.toList + |> AsyncResult.retn) diff --git a/src/FsAutoComplete/CodeFixes/SuggestedIdentifier.fs b/src/FsAutoComplete/CodeFixes/SuggestedIdentifier.fs deleted file mode 100644 index 896e191a4..000000000 --- a/src/FsAutoComplete/CodeFixes/SuggestedIdentifier.fs +++ /dev/null @@ -1,31 +0,0 @@ -module FsAutoComplete.CodeFix.SuggestedIdentifier - -open FsAutoComplete.CodeFix -open FsAutoComplete.CodeFix.Types -open FsToolkit.ErrorHandling -open FsAutoComplete - -/// a codefix that replaces the use of an unknown identifier with a suggested identifier -let fix = - Run.ifDiagnosticByMessage - "Maybe you want one of the following:" - (fun diagnostic codeActionParams -> - diagnostic.Message.Split('\n').[1..] - |> Array.map - (fun suggestion -> - let suggestion = suggestion.Trim() - - let suggestion = - if System.Text.RegularExpressions.Regex.IsMatch(suggestion, """^[a-zA-Z][a-zA-Z0-9']+$""") - then suggestion - else $"``%s{suggestion}``" - - { Edits = - [| { Range = diagnostic.Range - NewText = suggestion } |] - Title = $"Replace with %s{suggestion}" - File = codeActionParams.TextDocument - SourceDiagnostic = Some diagnostic - Kind = FixKind.Fix }) - |> Array.toList - |> AsyncResult.retn) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 3cbdf1c05..e0a1af455 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -847,7 +847,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS Run.ifEnabled (fun _ -> config.ResolveNamespaces) (ResolveNamespace.fix tryGetParseResultsForFile commands.GetNamespaceSuggestions) - SuggestedIdentifier.fix + ReplaceWithSuggestion.fix RemoveRedundantQualifier.fix Run.ifEnabled (fun _ -> config.UnusedDeclarationsAnalyzer) (UnusedValue.fix getRangeText) AddNewKeywordToDisposableConstructorInvocation.fix getRangeText diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 0adf17eff..5a72a0495 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1154,6 +1154,99 @@ let private removeUnusedBindingTests state = """ ]) +let private replaceWithSuggestionTests state = + serverTestList (nameof ReplaceWithSuggestion) state defaultConfigDto None (fun server -> [ + let selectCodeFix replacement = CodeFix.withTitle (ReplaceWithSuggestion.title replacement) + testCaseAsync "can change Min to min" <| + CodeFix.check server + """ + let x = $0Min(2.0, 1.0) + """ + Diagnostics.acceptAll + (selectCodeFix "min") + """ + let x = min(2.0, 1.0) + """ + testSequenced <| testList "can get multiple suggestions for flout" [ + testCaseAsync "can change flout to float" <| + CodeFix.check server + """ + let x = $0flout 2 + """ + Diagnostics.acceptAll + (selectCodeFix "float") + """ + let x = float 2 + """ + testCaseAsync "can change flout to float32" <| + CodeFix.check server + """ + let x = $0flout 2 + """ + Diagnostics.acceptAll + (selectCodeFix "float32") + """ + let x = float32 2 + """ + ] + testCaseAsync "can change flout to float in var type" <| + CodeFix.check server + """ + let x: $0flout = 2.0 + """ + Diagnostics.acceptAll + (selectCodeFix "float") + """ + let x: float = 2.0 + """ + testCaseAsync "can change namespace in open" <| + CodeFix.check server + """ + open System.Text.$0RegularEcpressions + """ + Diagnostics.acceptAll + (selectCodeFix "RegularExpressions") + """ + open System.Text.RegularExpressions + """ + testCaseAsync "can change type in type constructor" <| + CodeFix.check server + """ + open System.Text.RegularExpressions + let x = $0Regec() + """ + Diagnostics.acceptAll + (selectCodeFix "Regex") + """ + open System.Text.RegularExpressions + let x = Regex() + """ + testCaseAsync "can replace identifier in double-backticks" <| + CodeFix.check server + """ + let ``hello world`` = 2 + let x = ``$0hello word`` + """ + Diagnostics.acceptAll + (selectCodeFix "``hello world``") + """ + let ``hello world`` = 2 + let x = ``hello world`` + """ + testCaseAsync "can add double-backticks" <| + CodeFix.check server + """ + let ``hello world`` = 2 + let x = $0helloword + """ + Diagnostics.acceptAll + (selectCodeFix "``hello world``") + """ + let ``hello world`` = 2 + let x = ``hello world`` + """ + ]) + let private resolveNamespaceTests state = let config = { defaultConfigDto with ResolveNamespaces = Some true } serverTestList (nameof ResolveNamespace) state config None (fun server -> [ @@ -1356,6 +1449,7 @@ let tests state = testList "CodeFix tests" [ removeRedundantQualifierTests state removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state + replaceWithSuggestionTests state resolveNamespaceTests state unusedValueTests state useMutationWhenValueIsMutableTests state From 60617222ac8664000042617657af8932d26ae3d2 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sun, 10 Apr 2022 17:43:52 +0200 Subject: [PATCH 26/29] Fix: `RemoveUnusedOpens` doesn't trigger in first line Rename `UnusedOpens` to `RemoveUnusedOpens` -> align with `dotnet/fsharp` (VS) Add `isFirstLine`, `isLastLine`, `afterLastCharacterPosition` helper functions for `SourceText` Note: `SourceText` treats empty string (`""`) as no source. `SourceText.WithEmptyHandling` contains functions that consider empty string (`""`) as valid line. Add `tryEndOfPrevLine`, `tryStartOfNextLine`, `rangeToDeleteFullLine` helper functions for CodeFixes Add tests for `tryEndOfPrevLine`, `tryStartOfNextLine`, `rangeToDeleteFullLine` --- src/FsAutoComplete/CodeFixes.fs | 117 ++++++++ .../CodeFixes/RemoveUnusedOpens.fs | 30 ++ src/FsAutoComplete/CodeFixes/UnusedOpens.fs | 28 -- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 4 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 279 ++++++++++++++++++ 5 files changed, 429 insertions(+), 29 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/RemoveUnusedOpens.fs delete mode 100644 src/FsAutoComplete/CodeFixes/UnusedOpens.fs diff --git a/src/FsAutoComplete/CodeFixes.fs b/src/FsAutoComplete/CodeFixes.fs index 1634f815b..3a331cab3 100644 --- a/src/FsAutoComplete/CodeFixes.fs +++ b/src/FsAutoComplete/CodeFixes.fs @@ -73,6 +73,78 @@ module Types = Command = None Data = None } +module SourceText = + let inline private assertLineIndex lineIndex (sourceText: ISourceText) = + assert(0 <= lineIndex && lineIndex < sourceText.GetLineCount()) + /// Note: this fails when `sourceText` is empty string (`""`) + /// -> No lines + /// Use `WithEmptyHandling.isFirstLine` to handle empty string + let isFirstLine lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + lineIndex = 0 + /// Note: this fails when `sourceText` is empty string (`""`) + /// -> No lines + /// Use `WithEmptyHandling.isLastLine` to handle empty string + let isLastLine lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + lineIndex = sourceText.GetLineCount() - 1 + + /// SourceText treats empty string as no source: + /// ```fsharp + /// let text = SourceText.ofString "" + /// assert(text.ToString() = "") + /// assert(text.GetLastCharacterPosition() = (0, 0)) // Note: first line is `1` + /// assert(text.GetLineCount() = 0) // Note: `(SourceText.ofString "\n").GetLineCount()` is `2` + /// assert(text.GetLineString 0 ) // System.IndexOutOfRangeException: Index was outside the bounds of the array. + /// ``` + /// -> Functions in here treat empty string as empty single line + /// + /// Note: There's always at least empty single line + /// -> source MUST at least be empty (cannot not exist) + module WithEmptyHandling = + let getLineCount (sourceText: ISourceText) = + match sourceText.GetLineCount () with + | 0 -> 1 + | c -> c + // or + // max 1 (sourceText.GetLineCount()) + + let inline private assertLineIndex lineIndex sourceText = + assert(0 <= lineIndex && lineIndex < getLineCount sourceText) + + let getLineString lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + if lineIndex = 0 && sourceText.GetLineCount() = 0 then + "" + else + sourceText.GetLineString lineIndex + + let isFirstLine lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + // No need to check for inside `getLineCount`: there's always at least one line (might be empty) + lineIndex = 0 + + let isLastLine lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + lineIndex = (getLineCount sourceText) - 1 + + /// Returns position after last character in specified line. + /// Same as line length. + /// + /// Example: + /// ```fsharp + /// let text = SourceText.ofString "let a = 2\nlet foo = 42\na + foo\n" + /// + /// assert(afterLastCharacterPosition 0 text = 9) + /// assert(afterLastCharacterPosition 1 text = 12) + /// assert(afterLastCharacterPosition 2 text = 7) + /// assert(afterLastCharacterPosition 2 text = 0) + /// ``` + let afterLastCharacterPosition lineIndex (sourceText: ISourceText) = + assertLineIndex lineIndex sourceText + let line = sourceText |> getLineString lineIndex + line.Length + /// helpers for iterating along text lines module Navigation = @@ -144,6 +216,51 @@ module Navigation = let walkForwardUntilCondition lines pos condition = walkForwardUntilConditionWithTerminal lines pos condition (fun _ -> false) + /// Tries to detect the last cursor position in line before `currentLine` (0-based). + /// + /// Returns `None` iff there's no prev line -> `currentLine` is first line + let tryEndOfPrevLine (lines: ISourceText) currentLine = + if SourceText.WithEmptyHandling.isFirstLine currentLine lines then + None + else + let prevLine = currentLine - 1 + { Line = prevLine; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition prevLine } + |> Some + /// Tries to detect the first cursor position in line after `currentLine` (0-based). + /// + /// Returns `None` iff there's no next line -> `currentLine` is last line + let tryStartOfNextLine (lines: ISourceText) currentLine = + if SourceText.WithEmptyHandling.isLastLine currentLine lines then + None + else + let nextLine = currentLine + 1 + { Line = nextLine; Character = 0 } + |> Some + + /// Gets the range to delete the complete line `lineIndex` (0-based). + /// Deleting the line includes a linebreak if possible + /// -> range starts either at end of previous line (-> includes leading linebreak) + /// or start of next line (-> includes trailing linebreak) + /// + /// Special case: there's just one line + /// -> delete text of (single) line + let rangeToDeleteFullLine lineIndex (lines: ISourceText) = + match tryEndOfPrevLine lines lineIndex with + | Some start -> + // delete leading linebreak + { Start = start; End = { Line = lineIndex; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition lineIndex } } + | None -> + match tryStartOfNextLine lines lineIndex with + | Some fin -> + // delete trailing linebreak + { Start = { Line = lineIndex; Character = 0 }; End = fin } + | None -> + // single line + // -> just delete all text in line + { Start = { Line = lineIndex; Character = 0 }; End = { Line = lineIndex; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition lineIndex } } + + + module Run = open Types diff --git a/src/FsAutoComplete/CodeFixes/RemoveUnusedOpens.fs b/src/FsAutoComplete/CodeFixes/RemoveUnusedOpens.fs new file mode 100644 index 000000000..b12b72b28 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/RemoveUnusedOpens.fs @@ -0,0 +1,30 @@ +module FsAutoComplete.CodeFix.RemoveUnusedOpens + +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix +open FsAutoComplete.CodeFix.Types +open FsToolkit.ErrorHandling +open FsAutoComplete +open FsAutoComplete.LspHelpers +open FsAutoComplete.CodeFix.Navigation + +let title = "Remove unused open" +/// a codefix that removes unused open statements from the source +let fix (getFileLines: GetFileLines) : CodeFix = + Run.ifDiagnosticByMessage + "Unused open statement" + (fun d codeActionParams -> asyncResult { + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + let! lines = getFileLines fileName + + let lineToRemove = d.Range.Start.Line + let range = lines |> rangeToDeleteFullLine lineToRemove + + return [ + { Edits = [| { Range = range; NewText = "" } |] + File = codeActionParams.TextDocument + Title = title + SourceDiagnostic = Some d + Kind = FixKind.Refactor } + ] + }) diff --git a/src/FsAutoComplete/CodeFixes/UnusedOpens.fs b/src/FsAutoComplete/CodeFixes/UnusedOpens.fs deleted file mode 100644 index c0f42db19..000000000 --- a/src/FsAutoComplete/CodeFixes/UnusedOpens.fs +++ /dev/null @@ -1,28 +0,0 @@ -module FsAutoComplete.CodeFix.UnusedOpens - -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete.CodeFix -open FsAutoComplete.CodeFix.Types -open FsToolkit.ErrorHandling - -/// a codefix that removes unused open statements from the source -let fix : CodeFix = - Run.ifDiagnosticByMessage - "Unused open statement" - (fun d codeActionParams -> - let range = - { Start = - { Line = d.Range.Start.Line - 1 - Character = 1000 } - End = - { Line = d.Range.End.Line - Character = d.Range.End.Character } } - - let fix = - { Edits = [| { Range = range; NewText = "" } |] - File = codeActionParams.TextDocument - Title = "Remove unused open" - SourceDiagnostic = Some d - Kind = FixKind.Refactor } - - AsyncResult.retn [ fix ]) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index e0a1af455..d6f3a43e5 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -843,7 +843,9 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS let getAbstractClassStubReplacements () = abstractClassStubReplacements () codeFixes <- - [| Run.ifEnabled (fun _ -> config.UnusedOpensAnalyzer) UnusedOpens.fix + [| Run.ifEnabled + (fun _ -> config.UnusedOpensAnalyzer) + (RemoveUnusedOpens.fix getFileLines) Run.ifEnabled (fun _ -> config.ResolveNamespaces) (ResolveNamespace.fix tryGetParseResultsForFile commands.GetNamespaceSuggestions) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 5a72a0495..3847a16f7 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1154,6 +1154,120 @@ let private removeUnusedBindingTests state = """ ]) +let private removeUnusedOpensTests state = + let config = { defaultConfigDto with UnusedOpensAnalyzer = Some true } + serverTestList (nameof RemoveUnusedOpens) state config None (fun server -> [ + let selectCodeFix = CodeFix.withTitle RemoveUnusedOpens.title + testCaseAsync "can remove single unused open" <| + CodeFix.check server + """ + open $0System + """ + Diagnostics.acceptAll + selectCodeFix + "" + testCaseAsync "removes just current unused open" <| + // unlike VS, `RemoveUnusedOpens` removes just current open (with cursor) and not all unused opens + CodeFix.check server + """ + open $0System + open System.Text + """ + Diagnostics.acceptAll + selectCodeFix + """ + open System.Text + """ + testCaseAsync "removes just current unused open 2" <| + CodeFix.check server + """ + open System + open $0System.Text + """ + Diagnostics.acceptAll + selectCodeFix + """ + open System + """ + testCaseAsync "doesn't remove used open" <| + CodeFix.checkNotApplicable server + """ + open $0System + + let _ = String.IsNullOrWhiteSpace "" + """ + Diagnostics.acceptAll + selectCodeFix + testCaseAsync "can remove open in nested module" <| + CodeFix.check server + """ + module A = + module B = + open $0System + () + () + """ + Diagnostics.acceptAll + selectCodeFix + """ + module A = + module B = + () + () + """ + testCaseAsync "can remove used open in nested module when outer scope opens same open" <| + CodeFix.check server + """ + open System + module A = + module B = + open $0System + let x = String.IsNullOrWhiteSpace "" + () + () + """ + Diagnostics.acceptAll + selectCodeFix + """ + open System + module A = + module B = + let x = String.IsNullOrWhiteSpace "" + () + () + """ + //ENHANCEMENT: detect open in outer scope as unused too + // testCaseAsync "can remove used open in outer scope when usage in nested scope has own open" <| + // CodeFix.check server + // """ + // open $0System + // module A = + // module B = + // open System + // let x = String.IsNullOrWhiteSpace "" + // () + // () + // """ + // Diagnostics.acceptAll + // selectCodeFix + // """ + // module A = + // module B = + // open System + // let x = String.IsNullOrWhiteSpace "" + // () + // () + // """ + testCaseAsync "doesn't trigger for used open" <| + CodeFix.checkNotApplicable server + """ + open $0System + let x = String.IsNullOrWhiteSpace "" + """ + Diagnostics.acceptAll + selectCodeFix + ]) + let private replaceWithSuggestionTests state = serverTestList (nameof ReplaceWithSuggestion) state defaultConfigDto None (fun server -> [ let selectCodeFix replacement = CodeFix.withTitle (ReplaceWithSuggestion.title replacement) @@ -1422,7 +1536,171 @@ let private wrapExpressionInParenthesesTests state = selectCodeFix ]) +/// Helper functions for CodeFixes +module private CodeFixHelpers = + // `src\FsAutoComplete\CodeFixes.fs` -> `FsAutoComplete.CodeFix` + open Navigation + open FSharp.Compiler.Text + open Utils.TextEdit + + let private navigationTests = + testList (nameof Navigation) [ + let extractTwoCursors text = + let (text, poss) = Cursors.extract text + let text = SourceText.ofString text + (text, (poss[0], poss[1])) + + testList (nameof tryEndOfPrevLine) [ + testCase "can get end of prev line when not border line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5$0 +let $0x = 5 +let y = 7 +let z = 4""" + let (text, (expected, current)) = text |> extractTwoCursors + let actual = tryEndOfPrevLine text current.Line + Expect.equal actual (Some expected) "Incorrect pos" + + testCase "can get end of prev line when last line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5 +let x = 5 +let y = 7$0 +let z$0 = 4""" + let (text, (expected, current)) = text |> extractTwoCursors + let actual = tryEndOfPrevLine text current.Line + Expect.equal actual (Some expected) "Incorrect pos" + + testCase "cannot get end of prev line when first line" <| fun _ -> + let text = """let $0foo$0 = 4 +let bar = 5 +let baz = 5 +let x = 5 +let y = 7 +let z = 4""" + let (text, (_, current)) = text |> extractTwoCursors + let actual = tryEndOfPrevLine text current.Line + Expect.isNone actual "No prev line in first line" + + testCase "cannot get end of prev line when single line" <| fun _ -> + let text = SourceText.ofString "let foo = 4" + let line = 0 + let actual = tryEndOfPrevLine text line + Expect.isNone actual "No prev line in first line" + ] + testList (nameof tryStartOfNextLine) [ + // this would be WAY easier by just using `{ Line = current.Line + 1; Character = 0 }`... + testCase "can get start of next line when not border line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5 +let $0x = 5 +$0let y = 7 +let z = 4""" + let (text, (current, expected)) = text |> extractTwoCursors + let actual = tryStartOfNextLine text current.Line + Expect.equal actual (Some expected) "Incorrect pos" + + testCase "can get start of next line when first line" <| fun _ -> + let text = """let $0foo = 4 +$0let bar = 5 +let baz = 5 +let x = 5 +let y = 7 +let z = 4""" + let (text, (current, expected)) = text |> extractTwoCursors + let actual = tryStartOfNextLine text current.Line + Expect.equal actual (Some expected) "Incorrect pos" + + testCase "cannot get start of next line when last line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5 +let x = 5 +let y = 7 +let $0z$0 = 4""" + let (text, (current, _)) = text |> extractTwoCursors + let actual = tryStartOfNextLine text current.Line + Expect.isNone actual "No next line in last line" + + testCase "cannot get start of next line when single line" <| fun _ -> + let text = SourceText.ofString "let foo = 4" + let line = 0 + let actual = tryStartOfNextLine text line + Expect.isNone actual "No next line in first line" + ] + testList (nameof rangeToDeleteFullLine) [ + testCase "can get all range for single line" <| fun _ -> + let text = "$0let foo = 4$0" + let (text, (start, fin)) = text |> extractTwoCursors + let expected = { Start = start; End = fin } + + let line = fin.Line + let actual = text |> rangeToDeleteFullLine line + Expect.equal actual expected "Incorrect range" + + testCase "can get line range with leading linebreak in not border line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5$0 +let x = 5$0 +let y = 7 +let z = 4""" + let (text, (start, fin)) = text |> extractTwoCursors + let expected = { Start = start; End = fin } + + let line = fin.Line + let actual = text |> rangeToDeleteFullLine line + Expect.equal actual expected "Incorrect range" + + testCase "can get line range with leading linebreak in last line" <| fun _ -> + let text = """let foo = 4 +let bar = 5 +let baz = 5 +let x = 5 +let y = 7$0 +let z = 4$0""" + let (text, (start, fin)) = text |> extractTwoCursors + let expected = { Start = start; End = fin } + + let line = fin.Line + let actual = text |> rangeToDeleteFullLine line + Expect.equal actual expected "Incorrect range" + + testCase "can get line range with trailing linebreak in first line" <| fun _ -> + let text = """$0let foo = 4 +$0let bar = 5 +let baz = 5 +let x = 5 +let y = 7 +let z = 4""" + let (text, (start, fin)) = text |> extractTwoCursors + let expected = { Start = start; End = fin } + + let line = start.Line + let actual = text |> rangeToDeleteFullLine line + Expect.equal actual expected "Incorrect range" + + testCase "can get all range for single empty line" <| fun _ -> + let text = SourceText.ofString "" + let pos = { Line = 0; Character = 0 } + let expected = { Start = pos; End = pos } + + let line = pos.Line + let actual = text |> rangeToDeleteFullLine line + Expect.equal actual expected "Incorrect range" + ] + ] + + let tests = testList ($"{nameof(FsAutoComplete)}.{nameof FsAutoComplete.CodeFix}") [ + navigationTests + ] + let tests state = testList "CodeFix tests" [ + CodeFixHelpers.tests + addExplicitTypeToParameterTests state addMissingEqualsToTypeDefinitionTests state addMissingFunKeywordTests state @@ -1449,6 +1727,7 @@ let tests state = testList "CodeFix tests" [ removeRedundantQualifierTests state removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state + removeUnusedOpensTests state replaceWithSuggestionTests state resolveNamespaceTests state unusedValueTests state From bacbb21e37a513439d8beba6809dd60220dabd1f Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Sun, 10 Apr 2022 21:31:58 +0200 Subject: [PATCH 27/29] Rename `UnusedValue` to `RenameUnusedValue` -> align with `dotnet/fsharp` (VS) --- .../CodeFixes/{UnusedValue.fs => RenameUnusedValue.fs} | 2 +- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 2 +- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/FsAutoComplete/CodeFixes/{UnusedValue.fs => RenameUnusedValue.fs} (97%) diff --git a/src/FsAutoComplete/CodeFixes/UnusedValue.fs b/src/FsAutoComplete/CodeFixes/RenameUnusedValue.fs similarity index 97% rename from src/FsAutoComplete/CodeFixes/UnusedValue.fs rename to src/FsAutoComplete/CodeFixes/RenameUnusedValue.fs index a5d93df70..5198b9719 100644 --- a/src/FsAutoComplete/CodeFixes/UnusedValue.fs +++ b/src/FsAutoComplete/CodeFixes/RenameUnusedValue.fs @@ -1,4 +1,4 @@ -module FsAutoComplete.CodeFix.UnusedValue +module FsAutoComplete.CodeFix.RenameUnusedValue open FsToolkit.ErrorHandling open FsAutoComplete.CodeFix diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index d6f3a43e5..6534d8d62 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -851,7 +851,7 @@ type FSharpLspServer(backgroundServiceEnabled: bool, state: State, lspClient: FS (ResolveNamespace.fix tryGetParseResultsForFile commands.GetNamespaceSuggestions) ReplaceWithSuggestion.fix RemoveRedundantQualifier.fix - Run.ifEnabled (fun _ -> config.UnusedDeclarationsAnalyzer) (UnusedValue.fix getRangeText) + Run.ifEnabled (fun _ -> config.UnusedDeclarationsAnalyzer) (RenameUnusedValue.fix getRangeText) AddNewKeywordToDisposableConstructorInvocation.fix getRangeText Run.ifEnabled (fun _ -> config.UnionCaseStubGeneration) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 3847a16f7..680ed9365 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1383,11 +1383,11 @@ let private resolveNamespaceTests state = // * Different open locations of CodeFix and AutoOpen ]) -let private unusedValueTests state = +let private renameUnusedValue state = let config = { defaultConfigDto with UnusedDeclarationsAnalyzer = Some true } - serverTestList (nameof UnusedValue) state config None (fun server -> [ - let selectReplace = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titleReplace - let selectPrefix = CodeFix.ofKind "refactor" >> CodeFix.withTitle UnusedValue.titlePrefix + serverTestList (nameof RenameUnusedValue) state config None (fun server -> [ + let selectReplace = CodeFix.ofKind "refactor" >> CodeFix.withTitle RenameUnusedValue.titleReplace + let selectPrefix = CodeFix.ofKind "refactor" >> CodeFix.withTitle RenameUnusedValue.titlePrefix testCaseAsync "can replace unused self-reference" <| CodeFix.check server @@ -1730,7 +1730,7 @@ let tests state = testList "CodeFix tests" [ removeUnusedOpensTests state replaceWithSuggestionTests state resolveNamespaceTests state - unusedValueTests state + renameUnusedValue state useMutationWhenValueIsMutableTests state useTripleQuotedInterpolationTests state wrapExpressionInParenthesesTests state From cc2a49f87da2f68b5eb19f17d8d924c3a0044b22 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Mon, 11 Apr 2022 15:19:47 +0200 Subject: [PATCH 28/29] Fix: Sort order of Tests --- test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs index 680ed9365..f5d460afc 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests.fs @@ -1728,9 +1728,9 @@ let tests state = testList "CodeFix tests" [ removeUnnecessaryReturnOrYieldTests state removeUnusedBindingTests state removeUnusedOpensTests state + renameUnusedValue state replaceWithSuggestionTests state resolveNamespaceTests state - renameUnusedValue state useMutationWhenValueIsMutableTests state useTripleQuotedInterpolationTests state wrapExpressionInParenthesesTests state From 1db5841f918d017216caead6628acaad32cc7065 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Mon, 11 Apr 2022 19:47:34 +0200 Subject: [PATCH 29/29] Remove uncommented code --- .../CodeFixes/ChangeDerefBangToValue.fs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs b/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs index 86b7937b7..1e5100401 100644 --- a/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs +++ b/src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs @@ -9,28 +9,6 @@ open FSharp.Compiler.Syntax open FSharp.Compiler.Text.Range open FSharp.UMX -// let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineText): CodeFix = -// fun codeActionParams -> -// asyncResult { -// let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath -// let selectionRange = protocolRangeToRange (codeActionParams.TextDocument.GetFilePath()) codeActionParams.Range -// let! parseResults, line, lines = getParseResultsForFile fileName selectionRange.Start -// let! derefRange = parseResults.GetParseResults.TryRangeOfRefCellDereferenceContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No deref found at that pos") -// let! exprRange = parseResults.GetParseResults.TryRangeOfExpressionBeingDereferencedContainingPos selectionRange.Start |> Result.ofOption (fun _ -> "No expr found at that pos") -// let combinedRange = FSharp.Compiler.Text.Range.unionRanges derefRange exprRange -// let protocolRange = fcsRangeToLsp combinedRange -// let! badString = getLineText lines protocolRange -// let replacementString = badString.[1..] + ".Value" -// return [ -// { Title = title -// File = codeActionParams.TextDocument -// SourceDiagnostic = None -// Kind = FixKind.Refactor -// Edits = [| { Range = protocolRange -// NewText = replacementString } |] } -// ] -// } - /// adopted from `dotnet/fsharp` -> `FSharp.Compiler.CodeAnalysis.FSharpParseFileResults.TryRangeOfExpressionBeingDereferencedContainingPos` let private tryGetRangeOfDeref input derefPos = SyntaxTraversal.Traverse(derefPos, input, { new SyntaxVisitorBase<_>() with