diff --git a/src/FsAutoComplete.Core/FsprojEdit.fs b/src/FsAutoComplete.Core/FsprojEdit.fs index 08e1b83eb..d863bfafa 100644 --- a/src/FsAutoComplete.Core/FsprojEdit.fs +++ b/src/FsAutoComplete.Core/FsprojEdit.fs @@ -46,22 +46,60 @@ module FsProjEditor = node.SetAttribute("Include", includePath) node + /// Find the previous sibling that is a `` Element node, skipping over whitespace-only + /// text nodes and non-`Compile` elements that are present when PreserveWhitespace is true. + let private previousSiblingElement (node: System.Xml.XmlNode) = + let mutable prev = node.PreviousSibling + + // Skip over non-element nodes (e.g., whitespace) and elements that are not ``. + while not (isNull prev) + && (prev.NodeType <> System.Xml.XmlNodeType.Element || prev.LocalName <> "Compile") do + prev <- prev.PreviousSibling + + prev + + /// Find the next sibling that is a `` Element node, skipping over whitespace-only + /// text nodes and non-`Compile` elements that are present when PreserveWhitespace is true. + let private nextSiblingElement (node: System.Xml.XmlNode) = + let mutable next = node.NextSibling + + // Skip over non-element nodes (e.g., whitespace) and elements that are not ``. + while not (isNull next) + && (next.NodeType <> System.Xml.XmlNodeType.Element || next.LocalName <> "Compile") do + next <- next.NextSibling + + next + let moveFileUp (fsprojPath: string) (file: string) = let xDoc = System.Xml.XmlDocument() xDoc.PreserveWhitespace <- true // to keep custom formatting if present xDoc.Load fsprojPath let xpath = sprintf "//Compile[@Include='%s']/.." file let itemGroup = xDoc.SelectSingleNode(xpath) - let childXPath = sprintf "//Compile[@Include='%s']" file - let node = itemGroup.SelectSingleNode(childXPath) - let upNode = node.PreviousSibling - if isNull upNode then + if isNull itemGroup then () else - itemGroup.RemoveChild node |> ignore - itemGroup.InsertBefore(node, upNode) |> ignore - xDoc.Save fsprojPath + let childXPath = sprintf "//Compile[@Include='%s']" file + let node = itemGroup.SelectSingleNode(childXPath) + + if isNull node then + () + else + let prevElement = previousSiblingElement node + + if isNull prevElement then + () + else + // Clone both elements, insert clones in swapped positions, then remove the originals. + // Whitespace text nodes (indentation) remain in place so vertical formatting is preserved. + let nodeClone = node.CloneNode(true) + let prevClone = prevElement.CloneNode(true) + itemGroup.InsertBefore(nodeClone, prevElement) |> ignore + itemGroup.InsertBefore(prevClone, node) |> ignore + itemGroup.RemoveChild(prevElement) |> ignore + itemGroup.RemoveChild(node) |> ignore + xDoc.Save fsprojPath let moveFileDown (fsprojPath: string) (file: string) = let xDoc = System.Xml.XmlDocument() @@ -69,16 +107,30 @@ module FsProjEditor = xDoc.Load fsprojPath let xpath = sprintf "//Compile[@Include='%s']/.." file let itemGroup = xDoc.SelectSingleNode(xpath) - let childXPath = sprintf "//Compile[@Include='%s']" file - let node = itemGroup.SelectSingleNode(childXPath) - let downNode = node.NextSibling - if isNull downNode then + if isNull itemGroup then () else - itemGroup.RemoveChild node |> ignore - itemGroup.InsertAfter(node, downNode) |> ignore - xDoc.Save fsprojPath + let childXPath = sprintf "//Compile[@Include='%s']" file + let node = itemGroup.SelectSingleNode(childXPath) + + if isNull node then + () + else + let nextElement = nextSiblingElement node + + if isNull nextElement then + () + else + // Clone both elements, insert clones in swapped positions, then remove the originals. + // Whitespace text nodes (indentation) remain in place so vertical formatting is preserved. + let nodeClone = node.CloneNode(true) + let nextClone = nextElement.CloneNode(true) + itemGroup.InsertBefore(nextClone, node) |> ignore + itemGroup.InsertBefore(nodeClone, nextElement) |> ignore + itemGroup.RemoveChild(node) |> ignore + itemGroup.RemoveChild(nextElement) |> ignore + xDoc.Save fsprojPath let addFileAbove (fsprojPath: string) (aboveFile: string) (newFileName: string) = let xDoc = System.Xml.XmlDocument() diff --git a/test/FsAutoComplete.Tests.Lsp/FsProjEditorTests.fs b/test/FsAutoComplete.Tests.Lsp/FsProjEditorTests.fs new file mode 100644 index 000000000..0f7310cb6 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/FsProjEditorTests.fs @@ -0,0 +1,251 @@ +module FsAutoComplete.Tests.FsProjEditorTests + +open Expecto +open FsAutoComplete +open System +open System.IO +open System.Xml + +/// Create a temporary .fsproj file whose ItemGroup contains the given files +/// (one per line, LF line endings). +/// Returns the path; caller must delete. +let private createTempFsproj (files: string list) = + let path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".fsproj") + + let filesXml = + files + |> List.map (fun f -> $" ") + |> String.concat "\n" + + let content = + $"\n \n net8.0\n \n \n{filesXml}\n \n" + + File.WriteAllText(path, content) + path + +/// Create a temporary .fsproj file with CRLF line endings. +let private createTempFsprojCrlf (files: string list) = + let path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".fsproj") + + let filesXml = + files + |> List.map (fun f -> $" ") + |> String.concat "\r\n" + + let content = + $"\r\n \r\n net8.0\r\n \r\n \r\n{filesXml}\r\n \r\n" + + File.WriteAllText(path, content) + path + +/// Return the ordered list of values from a .fsproj. +let private getCompileOrder (fsprojPath: string) = + let xDoc = XmlDocument() + xDoc.Load fsprojPath + + xDoc.SelectNodes("//Compile[@Include]") + |> Seq.cast + |> Seq.map (fun n -> n.Attributes.["Include"].InnerText) + |> Seq.toList + +/// Return true when every element appears on its own line +/// (i.e. no two Compile tags share the same line in the saved file). +let private eachCompileOnOwnLine (fsprojPath: string) = + let lines = File.ReadAllLines(fsprojPath) + + lines + |> Array.filter (fun l -> l.Contains(" Array.forall (fun l -> + // A line that contains exactly one 1 they are horizontal. + let occurrences = + let mutable count = 0 + let mutable idx = 0 + + while idx < l.Length do + let pos = l.IndexOf(" + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "B.fs" + Expect.equal (getCompileOrder path) [ "B.fs"; "A.fs"; "C.fs" ] "B.fs should precede A.fs" + finally + File.Delete path + + testCase "moves last file up" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "C.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "C.fs"; "B.fs" ] "C.fs should precede B.fs" + finally + File.Delete path + + testCase "first file is unchanged" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "A.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "B.fs"; "C.fs" ] "Order should be unchanged" + finally + File.Delete path + + testCase "moving up twice reaches top" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "C.fs" + FsProjEditor.moveFileUp path "C.fs" + Expect.equal (getCompileOrder path) [ "C.fs"; "A.fs"; "B.fs" ] "C.fs should be first" + finally + File.Delete path + + testCase "single press actually changes compile order (regression: required two presses)" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "C.fs" + let order = getCompileOrder path + Expect.notEqual order [ "A.fs"; "B.fs"; "C.fs" ] "Compile order must change on first call" + finally + File.Delete path + + testCase "preserves vertical formatting with LF line endings" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "B.fs" + Expect.isTrue (eachCompileOnOwnLine path) "Each Compile element must remain on its own line" + finally + File.Delete path + + testCase "preserves vertical formatting with CRLF line endings" + <| fun _ -> + let path = createTempFsprojCrlf [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "B.fs" + Expect.isTrue (eachCompileOnOwnLine path) "Each Compile element must remain on its own line (CRLF)" + finally + File.Delete path ] + + testList + "moveFileDown" + [ testCase "moves first file below second" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "A.fs" + Expect.equal (getCompileOrder path) [ "B.fs"; "A.fs"; "C.fs" ] "A.fs should follow B.fs" + finally + File.Delete path + + testCase "moves middle file down" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "B.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "C.fs"; "B.fs" ] "B.fs should follow C.fs" + finally + File.Delete path + + testCase "last file is unchanged" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "C.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "B.fs"; "C.fs" ] "Order should be unchanged" + finally + File.Delete path + + testCase "moving down twice reaches bottom" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "A.fs" + FsProjEditor.moveFileDown path "A.fs" + Expect.equal (getCompileOrder path) [ "B.fs"; "C.fs"; "A.fs" ] "A.fs should be last" + finally + File.Delete path + + testCase "single press actually changes compile order (regression: required two presses)" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "A.fs" + let order = getCompileOrder path + Expect.notEqual order [ "A.fs"; "B.fs"; "C.fs" ] "Compile order must change on first call" + finally + File.Delete path + + testCase "preserves vertical formatting with LF line endings" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "B.fs" + Expect.isTrue (eachCompileOnOwnLine path) "Each Compile element must remain on its own line" + finally + File.Delete path + + testCase "preserves vertical formatting with CRLF line endings" + <| fun _ -> + let path = createTempFsprojCrlf [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "B.fs" + Expect.isTrue (eachCompileOnOwnLine path) "Each Compile element must remain on its own line (CRLF)" + finally + File.Delete path ] + + testList + "moveFileUp and moveFileDown roundtrip" + [ testCase "move up then down restores original order" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileUp path "B.fs" + FsProjEditor.moveFileDown path "B.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "B.fs"; "C.fs" ] "Round-trip should restore original order" + finally + File.Delete path + + testCase "move down then up restores original order" + <| fun _ -> + let path = createTempFsproj [ "A.fs"; "B.fs"; "C.fs" ] + + try + FsProjEditor.moveFileDown path "B.fs" + FsProjEditor.moveFileUp path "B.fs" + Expect.equal (getCompileOrder path) [ "A.fs"; "B.fs"; "C.fs" ] "Round-trip should restore original order" + finally + File.Delete path ] ] diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index af3f294d5..d177fdf04 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -154,6 +154,7 @@ let generalTests = LspHelpersTests.allTests TipFormatterTests.allTests FcsInvariantTests.tests + FsProjEditorTests.allTests decompilerTests ] []