Keep the whole NoWarn list, and each target framework once (#826)

Directory.Build.props declared NoWarn twice. The second declaration replaced the
first rather than adding to it, so CS0649, NU1608 and NU1109 were suppressed
nowhere - and with TreatWarningsAsErrors on, the next unassigned field would have
failed the build rather than warned in it. One list now, with everything both had.

DiffEngine.csproj also listed net9.0 and net10.0 in both of its TargetFrameworks
lines. MSBuild collapses that, so it built the same set, but the file said
something it did not mean.

Neither shows up in a build, which is why the two rules are asserted in a test
instead.

Author: SimonCropp

src/DiffEngine.Tests/BuildFileTests.cs

@@ -0,0 +1,58 @@
+/// <summary>
+/// Two things about the build files that a build cannot tell anyone. A second property of the same
+/// name replaces the first rather than adding to it, and a target framework listed twice is
+/// collapsed, so both mistakes build clean and say nothing.
+/// </summary>
+public class BuildFileTests
+{
+    /// <summary>
+    /// There were two, so only the later list was in force and CS0649, NU1608 and NU1109 were
+    /// suppressed nowhere - which under TreatWarningsAsErrors is a build failure waiting for the
+    /// first unassigned field.
+    /// </summary>
+    [Test]
+    public async Task NoWarn_is_declared_once()
+    {
+        var props = await File.ReadAllTextAsync(Path.Combine(Source(), "Directory.Build.props"));
+
+        var declarations = props.Split(["<NoWarn>"], StringSplitOptions.None).Length - 1;
+
+        await Assert.That(declarations).IsEqualTo(1);
+    }
+
+    [Test]
+    public async Task No_target_framework_is_listed_twice()
+    {
+        var project = await File.ReadAllTextAsync(Path.Combine(Source(), "DiffEngine", "DiffEngine.csproj"));
+
+        var listed = project
+            .Split('\n')
+            .Where(_ => _.Contains("<TargetFrameworks"))
+            .SelectMany(_ => _[(_.IndexOf('>') + 1)..^"</TargetFrameworks>".Length].Split(';'))
+            .Where(_ => _.StartsWith("net", StringComparison.Ordinal))
+            .ToList();
+
+        await Assert.That(listed).IsNotEmpty();
+        await Assert.That(listed.Distinct()).IsEquivalentTo(listed);
+    }
+
+    /// <summary>
+    /// The src directory, found by walking up from the test output rather than by counting
+    /// directories, which differs per target framework and configuration.
+    /// </summary>
+    static string Source()
+    {
+        var directory = new DirectoryInfo(AppContext.BaseDirectory);
+        while (directory != null)
+        {
+            if (File.Exists(Path.Combine(directory.FullName, "Directory.Build.props")))
+            {
+                return directory.FullName;
+            }
+
+            directory = directory.Parent;
+        }
+
+        throw new("Could not find Directory.Build.props above the test output.");
+    }
+}

src/DiffEngine/DiffEngine.csproj

@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFrameworks Condition=" '$(OS)' == 'Windows_NT'">net462;net472;net48;net9.0;net10.0</TargetFrameworks>
-    <TargetFrameworks>$(TargetFrameworks);net6.0;net7.0;net8.0;net9.0;net10.0</TargetFrameworks>
+    <TargetFrameworks>net6.0;net7.0;net8.0;net9.0;net10.0</TargetFrameworks>
+    <!-- The .NET Framework legs, which only Windows can build -->
+    <TargetFrameworks Condition=" '$(OS)' == 'Windows_NT'">net462;net472;net48;$(TargetFrameworks)</TargetFrameworks>
     <AllowUnsafeBlocks Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</AllowUnsafeBlocks>
     <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible>
   </PropertyGroup>

src/Directory.Build.props

@@ -1,7 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Project>
   <PropertyGroup>
-    <NoWarn>CS1591;CS0649;NU1608;NU1109</NoWarn>
     <Version>20.0.0-beta.30</Version>
     <AssemblyVersion>1.0.0</AssemblyVersion>
     <PackageTags>Testing, Snapshot, Diff, Compare</PackageTags>
@@ -14,7 +13,12 @@
     <CheckEolTargetFramework>false</CheckEolTargetFramework>
     <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
     <ResolveAssemblyReferencesSilent>true</ResolveAssemblyReferencesSilent>
-    <NoWarn>CA1416;CS1591</NoWarn>
+    <!--
+    One NoWarn, because a second property of the same name replaces the first rather than adding to
+    it. There were two, so CS0649, NU1608 and NU1109 were not suppressed at all - and with
+    TreatWarningsAsErrors below they would have failed a build rather than warned in one.
+    -->
+    <NoWarn>CS1591;CS0649;NU1608;NU1109;CA1416</NoWarn>
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
     <SuppressNETCoreSdkPreviewMessage>true</SuppressNETCoreSdkPreviewMessage>