Fix various issues with Extensions.csproj handling and add unit testing for it

Bonsai.Configuration.Tests/AssemblyInfo.cs

@@ -0,0 +1,3 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+[assembly: Parallelize]

Bonsai.Configuration.Tests/Bonsai.Configuration.Tests.csproj

@@ -0,0 +1,20 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFrameworks>net472;net8.0</TargetFrameworks>
+  </PropertyGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="**\*.bonsai" />
+  </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
+    <PackageReference Include="MSTest.TestAdapter" Version="3.5.0" />
+    <PackageReference Include="MSTest.TestFramework" Version="3.5.0" />
+    <PackageReference Include="coverlet.collector" Version="6.0.2">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\Bonsai.Configuration\Bonsai.Configuration.csproj" />
+  </ItemGroup>
+</Project>

Bonsai.Configuration.Tests/ScriptExtensionsTests.cs

@@ -0,0 +1,211 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Xml;
+using System.Xml.Serialization;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Bonsai.Configuration.Tests;
+
+[TestClass]
+[DoNotParallelize] // Tests involve filesystem at current working directory and must not be ran concurrently
+public sealed class ScriptExtensionsTests
+{
+    private static void EnsureCleanWorkspace()
+    {
+        if (File.Exists("Extensions.csproj"))
+            File.Delete("Extensions.csproj");
+    }
+
+    private static readonly PackageReference DummyPackageA = new("DummyPackageA", "3226.42.1337");
+    private static readonly PackageReference DummyPackageB = new("DummyPackageB", "126.484.30");
+    private static PackageConfiguration CreateDummyPackageConfiguration()
+    {
+        string dummyBonsaiConfig =
+            $"""
+            <?xml version="1.0" encoding="utf-8"?>
+            <PackageConfiguration xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+              <Packages>
+                <Package id="{DummyPackageA.Id}" version="{DummyPackageA.Version}" />
+                <Package id="{DummyPackageB.Id}" version="{DummyPackageB.Version}" />
+              </Packages>
+              <AssemblyReferences>
+              </AssemblyReferences>
+              <AssemblyLocations>
+                <AssemblyLocation assemblyName="{DummyPackageA.Id}" processorArchitecture="MSIL" location="Packages/{DummyPackageA.Id}.{DummyPackageA.Version}/{DummyPackageA.Id}.dll" />
+                <AssemblyLocation assemblyName="{DummyPackageB.Id}" processorArchitecture="MSIL" location="Packages/{DummyPackageB.Id}.{DummyPackageB.Version}/{DummyPackageB.Id}.dll" />
+              </AssemblyLocations>
+              <LibraryFolders>
+              </LibraryFolders>
+            </PackageConfiguration>
+            """;
+
+        var serializer = new XmlSerializer(typeof(PackageConfiguration));
+        using XmlReader reader = XmlReader.Create(new StringReader(dummyBonsaiConfig));
+        var configuration = (PackageConfiguration)serializer.Deserialize(reader);
+        configuration.ConfigurationFile = Path.GetFullPath("DummyBonsai.config");
+        return configuration;
+    }
+
+    [TestMethod]
+    public void HasDefaultProjectMetadata()
+    {
+        EnsureCleanWorkspace();
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(HasDefaultProjectMetadata)}Output");
+
+        Assert.IsFalse(File.Exists("Extensions.csproj"));
+        ScriptExtensionsProjectMetadata metadata = extensions.LoadProjectMetadata();
+
+        Assert.IsFalse(metadata.Exists);
+        Assert.AreEqual(default(ScriptExtensionsProjectMetadata).GetProjectXml(), metadata.GetProjectXml());
+        Assert.IsFalse(File.Exists("Extensions.csproj")); // Project file should not be written automatically
+    }
+
+    [TestMethod]
+    public void WillLoadProjectMetadata()
+    {
+        EnsureCleanWorkspace();
+
+        File.WriteAllText
+        (
+            "Extensions.csproj",
+            """
+            <Project Sdk="Microsoft.NET.Sdk">
+              <PropertyGroup>
+                <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+                <TargetFramework>net472</TargetFramework>
+              </PropertyGroup>
+
+              <ItemGroup>
+                <PackageReference Include="FakePackage" Version="1.0.0" />
+              </ItemGroup>
+            </Project>
+            """
+        );
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(WillLoadProjectMetadata)}Output");
+        ScriptExtensionsProjectMetadata metadata = extensions.LoadProjectMetadata();
+        Assert.IsTrue(metadata.Exists);
+        Assert.IsTrue(metadata.AllowUnsafeBlocks);
+        CollectionAssert.Contains(metadata.GetPackageReferences().ToArray(), "FakePackage");
+    }
+
+    [TestMethod]
+    public void UpadateProjectMetdataInitializesProject()
+    {
+        EnsureCleanWorkspace();
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(UpadateProjectMetdataInitializesProject)}Output");
+
+        Assert.IsFalse(File.Exists("Extensions.csproj"));
+        extensions.UpdateProjectMetadata(default);
+
+        Assert.IsTrue(File.Exists("Extensions.csproj"));
+        Assert.AreEqual(default(ScriptExtensionsProjectMetadata).GetProjectXml(), extensions.LoadProjectMetadata().GetProjectXml());
+    }
+
+    [TestMethod]
+    public void AddAssemblyReferencesInitializesProject()
+    {
+        EnsureCleanWorkspace();
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(AddAssemblyReferencesInitializesProject)}Output");
+
+        Assert.IsFalse(File.Exists("Extensions.csproj"));
+        extensions.AddAssemblyReferences([DummyPackageA.Id]);
+
+        Assert.IsTrue(File.Exists("Extensions.csproj"));
+        var metadata = extensions.LoadProjectMetadata();
+        CollectionAssert.Contains(metadata.GetPackageReferences().ToArray(), DummyPackageA.Id);
+    }
+
+    [TestMethod]
+    public void AddAssemblyReferencesUpdatesExistingProject()
+    {
+        EnsureCleanWorkspace();
+
+        File.WriteAllText
+        (
+            "Extensions.csproj",
+            $"""
+            <Project Sdk="Microsoft.NET.Sdk">
+              <PropertyGroup>
+                <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+                <TargetFramework>net472</TargetFramework>
+              </PropertyGroup>
+
+              <ItemGroup>
+                <PackageReference Include="{DummyPackageA.Id}" Version="{DummyPackageA.Version}" />
+              </ItemGroup>
+            </Project>
+            """
+        );
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(AddAssemblyReferencesUpdatesExistingProject)}Output");
+
+        var before = extensions.LoadProjectMetadata().GetPackageReferences();
+        CollectionAssert.Contains(before.ToArray(), DummyPackageA.Id);
+        CollectionAssert.DoesNotContain(before.ToArray(), DummyPackageB.Id);
+
+        extensions.AddAssemblyReferences([DummyPackageB.Id]);
+
+        var after = extensions.LoadProjectMetadata().GetPackageReferences();
+        CollectionAssert.Contains(after.ToArray(), DummyPackageA.Id);
+        CollectionAssert.Contains(after.ToArray(), DummyPackageB.Id);
+    }
+
+    [TestMethod]
+    // Regression test for https://github.com/bonsai-rx/bonsai/issues/2055
+    public void AddAssemblyReferencesUpdatesAreNonDestructive()
+    {
+        EnsureCleanWorkspace();
+
+        // A few edge cases exist that are not handled due to the way System.Xml.Linq works and therefore intentionally not tested:
+        // * XML declaration / processing instructions are not preserved -- It is legacy/meaningless to include these in a csproj
+        // * Trailing whitespace after the final closing tag
+        // * Mixed newlines or newlines not matching the environment -- They're always normalized.
+        string projectFileStart =
+            $"""
+            <Project Sdk="Microsoft.NET.Sdk">
+                {"\t"}<PropertyGroup>
+            {"\t\t"}<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+                    <TargetFramework>net472</TargetFramework>
+              </PropertyGroup>
+
+
+
+            <ItemGroup>
+                <!-- This is a comment before the first package reference -->
+                <PackageReference Include="{DummyPackageA.Id}" Version="{DummyPackageA.Version}" />
+
+            """.NormalizeNewlines();
+        string projectFileEnd =
+            $"""
+                <!-- This one is last! -->
+              </ItemGroup>
+            </Project>
+            """.NormalizeNewlines();
+
+        File.WriteAllText
+        (
+            "Extensions.csproj",
+            projectFileStart + projectFileEnd,
+            Encoding.UTF8
+        );
+
+        var packageConfiguration = CreateDummyPackageConfiguration();
+        using var extensions = new ScriptExtensions(packageConfiguration, $"{nameof(AddAssemblyReferencesUpdatesExistingProject)}Output");
+
+        extensions.AddAssemblyReferences([DummyPackageB.Id]);
+
+        string after = File.ReadAllText("Extensions.csproj");
+        Assert.AreEqual(projectFileStart + $"""    <PackageReference Include="{DummyPackageB.Id}" Version="{DummyPackageB.Version}" />{Environment.NewLine}""" + projectFileEnd, after);
+    }
+}

Bonsai.Configuration.Tests/TestHelpers.cs

@@ -0,0 +1,11 @@
+using System;
+using System.Text.RegularExpressions;
+
+namespace Bonsai.Configuration.Tests;
+
+internal static class TestHelpers
+{
+    /// <summary>Normalizes newlines to <see cref="Environment.NewLine"/>, intended for use with raw strings when newlines matter</summary>
+    internal static string NormalizeNewlines(this string value)
+        => Regex.Replace(value, "\r?\n", Environment.NewLine);
+}

Bonsai.Configuration/ScriptExtensions.cs

@@ -1,11 +1,9 @@
-using NuGet.Configuration;
-using System;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Reflection;
-using System.Xml;
-using System.Xml.Linq;
+using NuGet.Configuration;
 
 namespace Bonsai.Configuration
 {
@@ -73,17 +71,8 @@ public ScriptExtensionsProjectMetadata LoadProjectMetadata()
             if (!File.Exists(ProjectFileName))
                 return default;
 
-            var readerSettings = new XmlReaderSettings()
-            {
-                IgnoreWhitespace = true,
-                IgnoreProcessingInstructions = true,
-                DtdProcessing = DtdProcessing.Prohibit
-            };
-
             using var stream = File.OpenRead(ProjectFileName);
-            using var reader = XmlReader.Create(stream, readerSettings);
-            var document = XDocument.Load(reader, LoadOptions.None);
-            return new ScriptExtensionsProjectMetadata(document);
+            return new ScriptExtensionsProjectMetadata(stream);
         }
 
         public void UpdateProjectMetadata(ScriptExtensionsProjectMetadata projectMetadata)

Bonsai.Configuration/ScriptExtensionsProjectMetadata.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.IO;
 using System.Linq;
 using System.Xml;
 using System.Xml.Linq;
@@ -24,20 +25,27 @@ public readonly struct ScriptExtensionsProjectMetadata
         const string UseWindowsFormsElement = "UseWindowsForms";
         const string AllowUnsafeBlocksElement = "AllowUnsafeBlocks";
 
-        const string ProjectFileTemplate = @"<Project Sdk=""Microsoft.NET.Sdk"">
+        private static XElement ProjectFileTemplate
+            => XElement.Parse
+            ("""
+            <Project Sdk="Microsoft.NET.Sdk">            
 
-  <PropertyGroup>
-    <TargetFramework>net472</TargetFramework>
-  </PropertyGroup>
+              <PropertyGroup>
+                <TargetFramework>net472</TargetFramework>
+              </PropertyGroup>
 
-</Project>";
+            </Project>
+            """, LoadOptions.PreserveWhitespace);
 
         internal ScriptExtensionsProjectMetadata(XDocument projectDocument)
         {
             Debug.Assert(projectDocument is null || projectDocument.Root is not null);
             this.projectDocument = projectDocument;
         }
 
+        internal ScriptExtensionsProjectMetadata(Stream stream)
+            => projectDocument = XDocument.Load(stream, LoadOptions.PreserveWhitespace);
+
         private XElement GetProperty(string key)
             => projectDocument?.XPathSelectElement($"/Project/PropertyGroup/{key}");
 
@@ -76,8 +84,7 @@ public IEnumerable<string> GetPackageReferences()
 
         public ScriptExtensionsProjectMetadata AddPackageReferences(IEnumerable<PackageReference> packageReferences)
         {
-            var root = projectDocument?.Root;
-            root ??= XElement.Parse(ProjectFileTemplate, LoadOptions.PreserveWhitespace);
+            var root = new XElement(projectDocument?.Root ?? ProjectFileTemplate);
 
             var projectReferences = root.Descendants(PackageReferenceElement).ToArray();
             var lastReference = projectReferences.LastOrDefault();
@@ -104,15 +111,20 @@ public ScriptExtensionsProjectMetadata AddPackageReferences(IEnumerable<PackageR
                 var referenceElement = new XElement(PackageReferenceElement, includeAttribute, versionAttribute);
                 if (lastReference == null)
                 {
-                    var itemGroup = new XElement(ItemGroupElement);
+                    // Add the reference to any bare ItemGroup (IE: no Condition and such) or create a new one
+                    var itemGroup = root.XPathSelectElement("/ItemGroup[not(@*)]");
+                    if (itemGroup is null)
+                    {
+                        itemGroup = new XElement(ItemGroupElement);
+                        root.Add(OuterIndent);
+                        root.Add(itemGroup);
+                        root.Add(Environment.NewLine);
+                        root.Add(Environment.NewLine);
+                    }
+
                     itemGroup.Add(Environment.NewLine + InnerIndent);
                     itemGroup.Add(referenceElement);
                     itemGroup.Add(Environment.NewLine + OuterIndent);
-
-                    root.Add(OuterIndent);
-                    root.Add(itemGroup);
-                    root.Add(Environment.NewLine);
-                    root.Add(Environment.NewLine);
                 }
                 else
                 {
@@ -127,6 +139,6 @@ public ScriptExtensionsProjectMetadata AddPackageReferences(IEnumerable<PackageR
         }
 
         public string GetProjectXml()
-            => Exists ? projectDocument.Root.ToString(SaveOptions.DisableFormatting) : ProjectFileTemplate;
+            => (projectDocument?.Root ?? ProjectFileTemplate).ToString(SaveOptions.DisableFormatting);
     }
 }

Bonsai.Editor/Properties/AssemblyInfo.cs

@@ -1,8 +1,7 @@
 using System.Runtime.CompilerServices;
 using Bonsai;
 
-// General Information about an assembly is controlled through the following 
+// General Information about an assembly is controlled through the following
 // set of attributes. Change these attribute values to modify the information
 // associated with an assembly.
 [assembly: WorkflowNamespaceIcon("Bonsai.Editor.Scripting", "Bonsai:ElementIcon.CSharp")]
-[assembly: InternalsVisibleTo("Bonsai.Editor.Tests")]