From ea1d5773791109f43a97055c13e711c5bd84f4b1 Mon Sep 17 00:00:00 2001 From: Petr Date: Mon, 18 Sep 2023 22:54:52 +0200 Subject: [PATCH 1/2] Fix a bug in an Add Open code fix --- .../CodeFixes/AddOpenCodeFixProvider.fs | 26 ++++++++++------- .../CodeFixes/AddOpenOnTopOffTests.fs | 29 ++++++++++++++++++- .../CodeFixes/AddOpenOnTopOnTests.fs | 29 ++++++++++++++++++- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index b6b13608cb1..af426cd4b5b 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -55,21 +55,27 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr // attribute, shouldn't be here | line when line.StartsWith "[<" && line.EndsWith ">]" -> - let moduleDeclLineNumber = + let moduleDeclLineNumberOpt = sourceText.Lines |> Seq.skip insertionLineNumber - |> Seq.findIndex (fun line -> line.ToString().Contains "module") - // add back the skipped lines - |> fun i -> insertionLineNumber + i - - let moduleDeclLineText = sourceText.Lines[ moduleDeclLineNumber ].ToString().Trim() + |> Seq.tryFindIndex (fun line -> line.ToString().Contains "module") - if moduleDeclLineText.EndsWith "=" then + match moduleDeclLineNumberOpt with + // implicit top level module + | None -> insertionLineNumber, $"{margin}open {ns}{br}{br}" - else - moduleDeclLineNumber + 2, $"{margin}open {ns}{br}{br}" + // explicit top level module + | Some number -> + // add back the skipped lines + let moduleDeclLineNumber = insertionLineNumber + number + let moduleDeclLineText = sourceText.Lines[moduleDeclLineNumber].ToString().Trim() + + if moduleDeclLineText.EndsWith "=" then + insertionLineNumber, $"{margin}open {ns}{br}{br}" + else + moduleDeclLineNumber + 2, $"{margin}open {ns}{br}{br}" - // something else, shot in the dark + // implicit top level module | _ -> insertionLineNumber, $"{margin}open {ns}{br}{br}" | ScopeKind.Namespace -> insertionLineNumber + 3, $"{margin}open {ns}{br}{br}" diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs index 297554f3784..d3897828292 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -159,7 +159,7 @@ let ``Fixes FS0039 for missing opens - nested module`` () = Assert.Equal(expected, actual) [] -let ``Fixes FS0039 for missing opens - module has attributes`` () = +let ``Fixes FS0039 for missing opens - explicit module has attributes`` () = let code = """ [] @@ -187,6 +187,33 @@ Console.WriteLine 42 Assert.Equal(expected, actual) +[] +let ``Fixes FS0039 for missing opens - implicit module has attributes`` () = + let code = + """ +[] +type MyType() = + let now = DateTime.Now +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +[] +type MyType() = + let now = DateTime.Now +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + // TODO: the open statement should actually be within the module [] let ``Fixes FS0039 for missing opens - nested module has attributes`` () = diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs index ac95de440d4..185fd8390d1 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -153,7 +153,7 @@ module Module1 = Assert.Equal(expected, actual) [] -let ``Fixes FS0039 for missing opens - module has attributes`` () = +let ``Fixes FS0039 for missing opens - explicit module has attributes`` () = let code = """ [] @@ -181,6 +181,33 @@ Console.WriteLine 42 Assert.Equal(expected, actual) +[] +let ``Fixes FS0039 for missing opens - implicit module has attributes`` () = + let code = + """ +[] +type MyType() = + let now = DateTime.Now +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +[] +type MyType() = + let now = DateTime.Now +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + [] let ``Fixes FS0039 for missing opens - nested module has attributes`` () = let code = From 92a4d2604710b16c56e484a72d7a708efcc144f3 Mon Sep 17 00:00:00 2001 From: Petr Date: Mon, 18 Sep 2023 22:55:16 +0200 Subject: [PATCH 2/2] fantomas --- .../src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index af426cd4b5b..ac0b952f65f 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -62,13 +62,12 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr match moduleDeclLineNumberOpt with // implicit top level module - | None -> - insertionLineNumber, $"{margin}open {ns}{br}{br}" + | None -> insertionLineNumber, $"{margin}open {ns}{br}{br}" // explicit top level module | Some number -> // add back the skipped lines let moduleDeclLineNumber = insertionLineNumber + number - let moduleDeclLineText = sourceText.Lines[moduleDeclLineNumber].ToString().Trim() + let moduleDeclLineText = sourceText.Lines[ moduleDeclLineNumber ].ToString().Trim() if moduleDeclLineText.EndsWith "=" then insertionLineNumber, $"{margin}open {ns}{br}{br}"