diff options
7 files changed, 83 insertions, 6 deletions
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..54e91682 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs @@ -1,11 +1,20 @@ // 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; +using Netcode; 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; + + /// <summary>A sample net list.</summary> + public readonly NetList<int> eventsSeen; + + /// <summary>A sample net object list.</summary> + public readonly NetObjectList<int> netObjectList; } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs index 79ce9263..15bcadcd 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -19,6 +19,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests using StardewValley; using Netcode; using SObject = StardewValley.Object; + using SFarmer = StardewValley.Farmer; namespace SampleMod { @@ -33,7 +34,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests "; /// <summary>The line number where the unit tested code is injected into <see cref="SampleProgram"/>.</summary> - private const int SampleCodeLine = 13; + private const int SampleCodeLine = 14; /// <summary>The column number where the unit tested code is injected into <see cref="SampleProgram"/>.</summary> private const int SampleCodeColumn = 25; @@ -85,6 +86,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests [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("SFarmer farmer = new SFarmer(); object list = farmer.eventsSeen;", 46, "farmer.eventsSeen", "NetList", "object")] // ↓ NetList field converted to a non-interface type public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { // arrange @@ -101,6 +103,21 @@ 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("SFarmer farmer = new SFarmer(); System.Collections.IEnumerable list = farmer.eventsSeen;")] + [TestCase("SFarmer farmer = new SFarmer(); System.Collections.Generic.IEnumerable<int> list = farmer.eventsSeen;")] + [TestCase("SFarmer farmer = new SFarmer(); System.Collections.Generic.IList<int> list = farmer.eventsSeen;")] + [TestCase("SFarmer farmer = new SFarmer(); 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> diff --git a/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs b/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs index 77e7812f..e0c0cd63 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -42,5 +43,16 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer memberName = null; return false; } + + /// <summary>Get the class types in a type's inheritance chain, including itself.</summary> + /// <param name="type">The initial type.</param> + public static IEnumerable<ITypeSymbol> GetConcreteTypes(ITypeSymbol type) + { + while (type != null) + { + yield return type; + type = type.BaseType; + } + } } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index 895eebf0..7c8b804e 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -1,6 +1,8 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; @@ -17,6 +19,9 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// <summary>The namespace for Stardew Valley's <c>Netcode</c> types.</summary> private const string NetcodeNamespace = "Netcode"; + /// <summary>The full name for Stardew Valley's <c>Netcode.NetList</c> type.</summary> + private readonly string NetListTypeFullName = "Netcode.NetList"; + /// <summary>Maps net fields to their equivalent non-net properties where available.</summary> private readonly IDictionary<string, string> NetFieldWrapperProperties = new Dictionary<string, string> { @@ -190,10 +195,9 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer return; if (!this.IsNetType(memberType.Type)) return; - bool isConverted = !this.IsNetType(memberType.ConvertedType); // warn: use property wrapper if available - for (ITypeSymbol type = declaringType; type != null; type = type.BaseType) + foreach (ITypeSymbol type in AnalyzerUtilities.GetConcreteTypes(declaringType)) { if (this.NetFieldWrapperProperties.TryGetValue($"{type}::{memberName}", out string suggestedPropertyName)) { @@ -203,7 +207,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer } // warn: implicit conversion - if (isConverted) + if (this.IsInvalidConversion(memberType)) context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidImplicitNetFieldCast"], context.Node.GetLocation(), context.Node, memberType.Type.Name, memberType.ConvertedType)); } catch (Exception ex) @@ -212,6 +216,26 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer } } + /// <summary>Get whether a net field was converted in an error-prone way.</summary> + /// <param name="typeInfo">The member access type info.</param> + private bool IsInvalidConversion(TypeInfo typeInfo) + { + // no conversion + if (!this.IsNetType(typeInfo.Type) || this.IsNetType(typeInfo.ConvertedType)) + return false; + + // list conversion to an implemented interface is OK + if (AnalyzerUtilities.GetConcreteTypes(typeInfo.Type).Any(p => p.ToString().StartsWith(this.NetListTypeFullName))) // StartsWith to ignore generics + { + string toType = typeInfo.ConvertedType.ToString(); + if (toType.StartsWith(typeof(IEnumerable<>).Namespace) || toType == typeof(IEnumerable).FullName) + return false; + } + + // avoid any other conversions + return true; + } + /// <summary>Get whether a type symbol references a <c>Netcode</c> type.</summary> /// <param name="typeSymbol">The type symbol.</param> private bool IsNetType(ITypeSymbol typeSymbol) diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs index dc21e505..943d0350 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs @@ -79,7 +79,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer return; // suggest replacement - for (ITypeSymbol type = declaringType; type != null; type = type.BaseType) + foreach (ITypeSymbol type in AnalyzerUtilities.GetConcreteTypes(declaringType)) { if (this.ReplacedFields.TryGetValue($"{type}::{memberName}", out string replacement)) { |