Merge branch 'develop' into stable

8 files changed, 68 insertions, 11 deletions

diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetCollection.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetCollection.cs
new file mode 100644
index 00000000..d160610e
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetCollection.cs
@@ -0,0 +1,10 @@
+// ReSharper disable CheckNamespace -- matches Stardew Valley's code
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+
+namespace Netcode
+{
+    /// <summary>A simplified version of Stardew Valley's <c>Netcode.NetCollection</c> for unit testing.</summary>
+    public class NetCollection<T> : Collection<T>, IList<T>, ICollection<T>, IEnumerable<T>, IEnumerable { }
+}
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetList.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetList.cs
new file mode 100644
index 00000000..1699f71c
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetList.cs
@@ -0,0 +1,9 @@
+// ReSharper disable CheckNamespace -- matches Stardew Valley's code
+using System.Collections;
+using System.Collections.Generic;
+
+namespace Netcode
+{
+    /// <summary>A simplified version of Stardew Valley's <c>Netcode.NetObjectList</c> for unit testing.</summary>
+    public class NetList<T> : List<T>, IList<T>, ICollection<T>, IEnumerable<T>, IEnumerable { }
+}
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetObjectList.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetObjectList.cs
new file mode 100644
index 00000000..7814e7d6
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetObjectList.cs
@@ -0,0 +1,6 @@
+// ReSharper disable CheckNamespace -- matches Stardew Valley's code
+namespace Netcode
+{
+    /// <summary>A simplified version of Stardew Valley's <c>Netcode.NetObjectList</c> for unit testing.</summary>
+    public class NetObjectList<T> : NetList<T> { }
+}
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs
index e0f0e30c..13fab069 100644
--- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs
@@ -1,4 +1,5 @@
 // ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code
+#pragma warning disable 649 // (never assigned) -- only used to test type conversions
 using System.Collections.Generic;
 
 namespace StardewValley
@@ -6,6 +7,7 @@ namespace StardewValley
     /// <summary>A simplified version of Stardew Valley's <c>StardewValley.Farmer</c> class for unit testing.</summary>
     internal class Farmer
     {
-        public IDictionary<string, int[]> friendships;
+        /// <summary>A sample field which should be replaced with a different property.</summary>
+        public readonly IDictionary<string, int[]> friendships;
     }
 }
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs
index 386767d7..1b6317c1 100644
--- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs
@@ -20,5 +20,14 @@ namespace StardewValley
 
         /// <summary>A generic net ref property with no equivalent non-net property.</summary>
         public NetRef<object> netRefProperty { get; } = new NetRef<object>();
+
+        /// <summary>A sample net list.</summary>
+        public readonly NetList<int> netList = new NetList<int>();
+
+        /// <summary>A sample net object list.</summary>
+        public readonly NetObjectList<int> netObjectList = new NetObjectList<int>();
+
+        /// <summary>A sample net collection.</summary>
+        public readonly NetCollection<int> netCollection = new NetCollection<int>();
     }
 }
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs
index 101f4c21..6f8c8b9b 100644
--- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs
@@ -59,7 +59,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
         /// <param name="expression">The expression which should be reported.</param>
         /// <param name="fromType">The source type name which should be reported.</param>
         /// <param name="toType">The target type name which should be reported.</param>
-        [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")]
+        [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")] // ↓ implicit conversion
         [TestCase("Item item = null; if (item.netIntField <= 42);", 22, "item.netIntField", "NetInt", "int")]
         [TestCase("Item item = null; if (item.netIntField > 42);", 22, "item.netIntField", "NetInt", "int")]
         [TestCase("Item item = null; if (item.netIntField >= 42);", 22, "item.netIntField", "NetInt", "int")]
@@ -79,20 +79,24 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
         [TestCase("Item item = null; if (item.netRefField != null);", 22, "item.netRefField", "NetRef", "object")]
         [TestCase("Item item = null; if (item.netRefProperty == null);", 22, "item.netRefProperty", "NetRef", "object")]
         [TestCase("Item item = null; if (item.netRefProperty != null);", 22, "item.netRefProperty", "NetRef", "object")]
-        [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ same as above, but inherited from base class
+        [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ implicit conversion for parent field
         [TestCase("SObject obj = null; if (obj.netIntProperty != 42);", 24, "obj.netIntProperty", "NetInt", "int")]
         [TestCase("SObject obj = null; if (obj.netRefField == null);", 24, "obj.netRefField", "NetRef", "object")]
         [TestCase("SObject obj = null; if (obj.netRefField != null);", 24, "obj.netRefField", "NetRef", "object")]
         [TestCase("SObject obj = null; if (obj.netRefProperty == null);", 24, "obj.netRefProperty", "NetRef", "object")]
         [TestCase("SObject obj = null; if (obj.netRefProperty != null);", 24, "obj.netRefProperty", "NetRef", "object")]
+        [TestCase("Item item = new Item(); object list = item.netList;", 38, "item.netList", "NetList", "object")] // ↓ NetList field converted to a non-interface type
+        [TestCase("Item item = new Item(); object list = item.netCollection;", 38, "item.netCollection", "NetCollection", "object")]
+        [TestCase("Item item = new Item(); int x = (int)item.netIntField;", 32, "item.netIntField", "NetInt", "int")] // ↓ explicit conversion to invalid type
+        [TestCase("Item item = new Item(); int x = item.netRefField as object;", 32, "item.netRefField", "NetRef", "object")]
         public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType)
         {
             // arrange
             string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText);
             DiagnosticResult expected = new DiagnosticResult
             {
-                Id = "SMAPI001",
-                Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/smapi001 for details.",
+                Id = "AvoidImplicitNetFieldCast",
+                Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/avoid-implicit-net-field-cast for details.",
                 Severity = DiagnosticSeverity.Warning,
                 Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) }
             };
@@ -101,6 +105,22 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
             this.VerifyCSharpDiagnostic(code, expected);
         }
 
+        /// <summary>Test that the net field analyzer doesn't raise any warnings for safe member access.</summary>
+        /// <param name="codeText">The code line to test.</param>
+        [TestCase("Item item = new Item(); System.Collections.IEnumerable list = farmer.eventsSeen;")]
+        [TestCase("Item item = new Item(); System.Collections.Generic.IEnumerable<int> list = farmer.netList;")]
+        [TestCase("Item item = new Item(); System.Collections.Generic.IList<int> list = farmer.netList;")]
+        [TestCase("Item item = new Item(); System.Collections.Generic.ICollection<int> list = farmer.netCollection;")]
+        [TestCase("Item item = new Item(); System.Collections.Generic.IList<int> list = farmer.netObjectList;")] // subclass of NetList
+        public void AvoidImplicitNetFieldComparisons_AllowsSafeAccess(string codeText)
+        {
+            // arrange
+            string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText);
+
+            // assert
+            this.VerifyCSharpDiagnostic(code);
+        }
+
         /// <summary>Test that the expected diagnostic message is raised for avoidable net field references.</summary>
         /// <param name="codeText">The code line to test.</param>
         /// <param name="column">The column within the code line where the diagnostic message should be reported.</param>
@@ -117,8 +137,8 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
             string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText);
             DiagnosticResult expected = new DiagnosticResult
             {
-                Id = "SMAPI002",
-                Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/smapi002 for details.",
+                Id = "AvoidNetField",
+                Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/avoid-net-field for details.",
                 Severity = DiagnosticSeverity.Warning,
                 Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) }
             };
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs
index dc7476ef..102a80d1 100644
--- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs
@@ -59,14 +59,15 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
         /// <param name="oldName">The old field name which should be reported.</param>
         /// <param name="newName">The new field name which should be reported.</param>
         [TestCase("var x = new Farmer().friendships;", 8, "StardewValley.Farmer.friendships", "friendshipData")]
+        [TestCase("var x = new Farmer()?.friendships;", 8, "StardewValley.Farmer.friendships", "friendshipData")]
         public void AvoidObsoleteField_RaisesDiagnostic(string codeText, int column, string oldName, string newName)
         {
             // arrange
             string code = ObsoleteFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText);
             DiagnosticResult expected = new DiagnosticResult
             {
-                Id = "SMAPI003",
-                Message = $"The '{oldName}' field is obsolete and should be replaced with '{newName}'. See https://smapi.io/buildmsg/smapi003 for details.",
+                Id = "AvoidObsoleteField",
+                Message = $"The '{oldName}' field is obsolete and should be replaced with '{newName}'. See https://smapi.io/buildmsg/avoid-obsolete-field for details.",
                 Severity = DiagnosticSeverity.Warning,
                 Locations = new[] { new DiagnosticResultLocation("Test0.cs", ObsoleteFieldAnalyzerTests.SampleCodeLine, ObsoleteFieldAnalyzerTests.SampleCodeColumn + column) }
             };
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj b/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj
index b5630314..c6241ecb 100644
--- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj
+++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj
@@ -5,8 +5,8 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.4.0" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.8.2" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.2" />
     <PackageReference Include="NUnit" Version="3.10.1" />
     <PackageReference Include="NUnit3TestAdapter" Version="3.10.0" />
   </ItemGroup>