From 9e5c3912b6cfce0348aa2d88d30a11fe5b410e9c Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:22:16 -0400 Subject: move mock classes out of sample code (#471) --- .../Mock/Netcode/NetFieldBase.cs | 16 ++++++++++++++++ .../Mock/Netcode/NetInt.cs | 6 ++++++ .../Mock/Netcode/NetRef.cs | 6 ++++++ .../Mock/StardewValley/Item.cs | 18 ++++++++++++++++++ .../Mock/StardewValley/Object.cs | 6 ++++++ 5 files changed, 52 insertions(+) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetFieldBase.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetInt.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs (limited to 'src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock') diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetFieldBase.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetFieldBase.cs new file mode 100644 index 00000000..1684229a --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetFieldBase.cs @@ -0,0 +1,16 @@ +// ReSharper disable CheckNamespace -- matches Stardew Valley's code +namespace Netcode +{ + /// A simplified version of Stardew Valley's Netcode.NetFieldBase for unit testing. + /// The type of the synchronised value. + /// The type of the current instance. + public class NetFieldBase where TSelf : NetFieldBase + { + /// The synchronised value. + public T Value { get; set; } + + /// Implicitly convert a net field to the its type. + /// The field to convert. + public static implicit operator T(NetFieldBase field) => field.Value; + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetInt.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetInt.cs new file mode 100644 index 00000000..b3abc467 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetInt.cs @@ -0,0 +1,6 @@ +// ReSharper disable CheckNamespace -- matches Stardew Valley's code +namespace Netcode +{ + /// A simplified version of Stardew Valley's Netcode.NetInt for unit testing. + public class NetInt : NetFieldBase { } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs new file mode 100644 index 00000000..714c4a8d --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs @@ -0,0 +1,6 @@ +// ReSharper disable CheckNamespace -- matches Stardew Valley's code +namespace Netcode +{ + /// A simplified version of Stardew Valley's Netcode.NetRef for unit testing. + public class NetRef : NetFieldBase { } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs new file mode 100644 index 00000000..88723a56 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs @@ -0,0 +1,18 @@ +// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code +using Netcode; + +namespace StardewValley +{ + /// A simplified version of Stardew Valley's StardewValley.Item class for unit testing. + public class Item + { + /// A net int field with an equivalent non-net Category property. + public NetInt category { get; } = new NetInt { Value = 42 }; + + /// A net int field with no equivalent non-net property. + public NetInt type { get; } = new NetInt { Value = 42 }; + + /// A net reference field. + public NetRef refField { get; } = null; + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs new file mode 100644 index 00000000..498c38c1 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs @@ -0,0 +1,6 @@ +// ReSharper disable CheckNamespace -- matches Stardew Valley's code +namespace StardewValley +{ + /// A simplified version of Stardew Valley's StardewValley.Object class for unit testing. + public class Object : Item { } +} -- cgit From 35c2e5968579ea578cf04e7550ab43bc5a4b8074 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:22:34 -0400 Subject: expand analyzer unit tests (#471) --- .../Mock/Netcode/NetRef.cs | 2 +- .../Mock/StardewValley/Item.cs | 16 ++++--- .../Mock/StardewValley/Object.cs | 10 ++++- .../UnitTests.cs | 52 +++++++++++++--------- 4 files changed, 51 insertions(+), 29 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock') diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs index 714c4a8d..be2459cc 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/Netcode/NetRef.cs @@ -2,5 +2,5 @@ namespace Netcode { /// A simplified version of Stardew Valley's Netcode.NetRef for unit testing. - public class NetRef : NetFieldBase { } + public class NetRef : NetFieldBase> where T : class { } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs index 88723a56..386767d7 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Item.cs @@ -7,12 +7,18 @@ namespace StardewValley public class Item { /// A net int field with an equivalent non-net Category property. - public NetInt category { get; } = new NetInt { Value = 42 }; + public readonly NetInt category = new NetInt { Value = 42 }; - /// A net int field with no equivalent non-net property. - public NetInt type { get; } = new NetInt { Value = 42 }; + /// A generic net int field with no equivalent non-net property. + public readonly NetInt netIntField = new NetInt { Value = 42 }; - /// A net reference field. - public NetRef refField { get; } = null; + /// A generic net ref field with no equivalent non-net property. + public readonly NetRef netRefField = new NetRef(); + + /// A generic net int property with no equivalent non-net property. + public NetInt netIntProperty = new NetInt { Value = 42 }; + + /// A generic net ref property with no equivalent non-net property. + public NetRef netRefProperty { get; } = new NetRef(); } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs index 498c38c1..3dd66a6d 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Object.cs @@ -1,6 +1,12 @@ -// ReSharper disable CheckNamespace -- matches Stardew Valley's code +// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code +using Netcode; + namespace StardewValley { /// A simplified version of Stardew Valley's StardewValley.Object class for unit testing. - public class Object : Item { } + public class Object : Item + { + /// A net int field with an equivalent non-net property. + public NetInt type = new NetInt { Value = 42 }; + } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs index c12bb839..51e0b059 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs @@ -26,21 +26,14 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests { public void Entry() { - Item item = null; - SObject obj = null; - - // this line should raise diagnostics {{test-code}} - - // these lines should not - if (item.type.Value != 42); } } } "; /// The line number where the unit tested code is injected into . - private const int SampleCodeLine = 17; + private const int SampleCodeLine = 13; /// The column number where the unit tested code is injected into . private const int SampleCodeColumn = 25; @@ -66,15 +59,32 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests /// The expression which should be reported. /// The source type name which should be reported. /// The target type name which should be reported. - [TestCase("if (item.type < 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.type <= 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.type > 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.type >= 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.type == 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.type != 42);", 4, "item.type", "NetInt", "int")] - [TestCase("if (item.refField != null);", 4, "item.refField", "NetRef", "object")] - [TestCase("if (item?.type != 42);", 4, "item?.type", "NetInt", "int")] - [TestCase("if (obj.type != 42);", 4, "obj.type", "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")] + [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")] + [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 != null);", 22, "item?.netIntField", "NetInt", "object")] + [TestCase("Item item = null; if (item.netIntProperty < 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty <= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty > 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty >= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty == 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty != 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != 42);", 22, "item?.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != null);", 22, "item?.netIntProperty", "NetInt", "object")] + [TestCase("Item item = null; if (item.netRefField == null);", 22, "item.netRefField", "NetRef", "object")] + [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.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")] public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { // arrange @@ -97,10 +107,10 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests /// The expression which should be reported. /// The net type name which should be reported. /// The suggested property name which should be reported. - [TestCase("int category = item.category;", 15, "item.category", "NetInt", "Category")] - [TestCase("int category = (item).category;", 15, "(item).category", "NetInt", "Category")] - [TestCase("int category = ((Item)item).category;", 15, "((Item)item).category", "NetInt", "Category")] - [TestCase("int category = obj.category;", 15, "obj.category", "NetInt", "Category")] + [TestCase("Item item = null; int category = item.category;", 33, "item.category", "NetInt", "Category")] + [TestCase("Item item = null; int category = (item).category;", 33, "(item).category", "NetInt", "Category")] + [TestCase("Item item = null; int category = ((Item)item).category;", 33, "((Item)item).category", "NetInt", "Category")] + [TestCase("SObject obj = null; int category = obj.category;", 35, "obj.category", "NetInt", "Category")] public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) { // arrange -- cgit From 13f31e8b725e46ca8442a943a5675723d22b4fdc Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:57 -0400 Subject: warn for fields which no longer work (#471) --- docs/mod-build-config.md | 6 + .../Mock/StardewValley/Farmer.cs | 11 ++ .../NetFieldAnalyzerTests.cs | 140 +++++++++++++++++++++ .../ObsoleteFieldAnalyzerTests.cs | 88 +++++++++++++ .../UnitTests.cs | 140 --------------------- .../ObsoleteFieldAnalyzer.cs | 98 +++++++++++++++ 6 files changed, 343 insertions(+), 140 deletions(-) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs delete mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs (limited to 'src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 00f9a356..99a567f2 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -194,6 +194,12 @@ field has an equivalent non-net property that avoids those issues. Suggested fix: access the suggested property name instead. +### SMAPI003 +**Avoid obsolete fields:** +> The '{{old field}}' field is obsolete and should be replaced with '{{new field}}'. + +Your code accesses a field which is obsolete or no longer works. Use the suggested field instead. + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs new file mode 100644 index 00000000..e0f0e30c --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs @@ -0,0 +1,11 @@ +// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code +using System.Collections.Generic; + +namespace StardewValley +{ + /// A simplified version of Stardew Valley's StardewValley.Farmer class for unit testing. + internal class Farmer + { + public IDictionary friendships; + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs new file mode 100644 index 00000000..101f4c21 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -0,0 +1,140 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; +using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; +using StardewModdingAPI.ModBuildConfig.Analyzer; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests +{ + /// Unit tests for . + [TestFixture] + public class NetFieldAnalyzerTests : DiagnosticVerifier + { + /********* + ** Properties + *********/ + /// Sample C# mod code, with a {{test-code}} placeholder for the code in the Entry method to test. + const string SampleProgram = @" + using System; + using StardewValley; + using Netcode; + using SObject = StardewValley.Object; + + namespace SampleMod + { + class ModEntry + { + public void Entry() + { + {{test-code}} + } + } + } + "; + + /// The line number where the unit tested code is injected into . + private const int SampleCodeLine = 13; + + /// The column number where the unit tested code is injected into . + private const int SampleCodeColumn = 25; + + + /********* + ** Unit tests + *********/ + /// Test that no diagnostics are raised for an empty code block. + [TestCase] + public void EmptyCode_HasNoDiagnostics() + { + // arrange + string test = @""; + + // assert + this.VerifyCSharpDiagnostic(test); + } + + /// Test that the expected diagnostic message is raised for implicit net field comparisons. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The source type name which should be reported. + /// The target type name which should be reported. + [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")] + [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")] + [TestCase("Item item = null; if (item?.netIntField != 42);", 22, "item?.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntField != null);", 22, "item?.netIntField", "NetInt", "object")] + [TestCase("Item item = null; if (item.netIntProperty < 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty <= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty > 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty >= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty == 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty != 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != 42);", 22, "item?.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != null);", 22, "item?.netIntProperty", "NetInt", "object")] + [TestCase("Item item = null; if (item.netRefField == null);", 22, "item.netRefField", "NetRef", "object")] + [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.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")] + 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.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + /// Test that the expected diagnostic message is raised for avoidable net field references. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The net type name which should be reported. + /// The suggested property name which should be reported. + [TestCase("Item item = null; int category = item.category;", 33, "item.category", "NetInt", "Category")] + [TestCase("Item item = null; int category = (item).category;", 33, "(item).category", "NetInt", "Category")] + [TestCase("Item item = null; int category = ((Item)item).category;", 33, "((Item)item).category", "NetInt", "Category")] + [TestCase("SObject obj = null; int category = obj.category;", 35, "obj.category", "NetInt", "Category")] + public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) + { + // arrange + 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.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + + /********* + ** Helpers + *********/ + /// Get the analyzer being tested. + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new NetFieldAnalyzer(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs new file mode 100644 index 00000000..dc7476ef --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs @@ -0,0 +1,88 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; +using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; +using StardewModdingAPI.ModBuildConfig.Analyzer; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests +{ + /// Unit tests for . + [TestFixture] + public class ObsoleteFieldAnalyzerTests : DiagnosticVerifier + { + /********* + ** Properties + *********/ + /// Sample C# mod code, with a {{test-code}} placeholder for the code in the Entry method to test. + const string SampleProgram = @" + using System; + using StardewValley; + using Netcode; + using SObject = StardewValley.Object; + + namespace SampleMod + { + class ModEntry + { + public void Entry() + { + {{test-code}} + } + } + } + "; + + /// The line number where the unit tested code is injected into . + private const int SampleCodeLine = 13; + + /// The column number where the unit tested code is injected into . + private const int SampleCodeColumn = 25; + + + /********* + ** Unit tests + *********/ + /// Test that no diagnostics are raised for an empty code block. + [TestCase] + public void EmptyCode_HasNoDiagnostics() + { + // arrange + string test = @""; + + // assert + this.VerifyCSharpDiagnostic(test); + } + + /// Test that the expected diagnostic message is raised for an obsolete field reference. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The old field name which should be reported. + /// The new field name which should be reported. + [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.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", ObsoleteFieldAnalyzerTests.SampleCodeLine, ObsoleteFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + + /********* + ** Helpers + *********/ + /// Get the analyzer being tested. + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ObsoleteFieldAnalyzer(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs deleted file mode 100644 index 8ca27847..00000000 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ /dev/null @@ -1,140 +0,0 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; -using NUnit.Framework; -using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; -using StardewModdingAPI.ModBuildConfig.Analyzer; - -namespace SMAPI.ModBuildConfig.Analyzer.Tests -{ - /// Unit tests for the C# analyzers. - [TestFixture] - public class UnitTests : DiagnosticVerifier - { - /********* - ** Properties - *********/ - /// Sample C# code which contains a simplified representation of Stardew Valley's Netcode types, and sample mod code with a {{test-code}} placeholder for the code being tested. - const string SampleProgram = @" - using System; - using StardewValley; - using Netcode; - using SObject = StardewValley.Object; - - namespace SampleMod - { - class ModEntry - { - public void Entry() - { - {{test-code}} - } - } - } - "; - - /// The line number where the unit tested code is injected into . - private const int SampleCodeLine = 13; - - /// The column number where the unit tested code is injected into . - private const int SampleCodeColumn = 25; - - - /********* - ** Unit tests - *********/ - /// Test that no diagnostics are raised for an empty code block. - [TestCase] - public void EmptyCode_HasNoDiagnostics() - { - // arrange - string test = @""; - - // assert - this.VerifyCSharpDiagnostic(test); - } - - /// Test that the expected diagnostic message is raised for implicit net field comparisons. - /// The code line to test. - /// The column within the code line where the diagnostic message should be reported. - /// The expression which should be reported. - /// The source type name which should be reported. - /// The target type name which should be reported. - [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")] - [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")] - [TestCase("Item item = null; if (item?.netIntField != 42);", 22, "item?.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntField != null);", 22, "item?.netIntField", "NetInt", "object")] - [TestCase("Item item = null; if (item.netIntProperty < 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty <= 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty > 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty >= 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty == 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty != 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntProperty != 42);", 22, "item?.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntProperty != null);", 22, "item?.netIntProperty", "NetInt", "object")] - [TestCase("Item item = null; if (item.netRefField == null);", 22, "item.netRefField", "NetRef", "object")] - [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.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")] - public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) - { - // arrange - string code = UnitTests.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.", - Severity = DiagnosticSeverity.Warning, - Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } - }; - - // assert - this.VerifyCSharpDiagnostic(code, expected); - } - - /// Test that the expected diagnostic message is raised for avoidable net field references. - /// The code line to test. - /// The column within the code line where the diagnostic message should be reported. - /// The expression which should be reported. - /// The net type name which should be reported. - /// The suggested property name which should be reported. - [TestCase("Item item = null; int category = item.category;", 33, "item.category", "NetInt", "Category")] - [TestCase("Item item = null; int category = (item).category;", 33, "(item).category", "NetInt", "Category")] - [TestCase("Item item = null; int category = ((Item)item).category;", 33, "((Item)item).category", "NetInt", "Category")] - [TestCase("SObject obj = null; int category = obj.category;", 35, "obj.category", "NetInt", "Category")] - public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) - { - // arrange - string code = UnitTests.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.", - Severity = DiagnosticSeverity.Warning, - Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } - }; - - // assert - this.VerifyCSharpDiagnostic(code, expected); - } - - - /********* - ** Helpers - *********/ - /// Get the analyzer being tested. - protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() - { - return new NetFieldAnalyzer(); - } - } -} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs new file mode 100644 index 00000000..00565329 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs @@ -0,0 +1,98 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace StardewModdingAPI.ModBuildConfig.Analyzer +{ + /// Detects references to a field which has been replaced. + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ObsoleteFieldAnalyzer : DiagnosticAnalyzer + { + /********* + ** Properties + *********/ + /// Maps obsolete fields/properties to their non-obsolete equivalent. + private readonly IDictionary ReplacedFields = new Dictionary + { + // Farmer + ["StardewValley.Farmer::friendships"] = "friendshipData" + }; + + /// Describes the diagnostic rule covered by the analyzer. + private readonly IDictionary Rules = new Dictionary + { + ["SMAPI003"] = new DiagnosticDescriptor( + id: "SMAPI003", + title: "Reference to obsolete field", + messageFormat: "The '{0}' field is obsolete and should be replaced with '{1}'. See https://smapi.io/buildmsg/smapi003 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://smapi.io/buildmsg/smapi003" + ) + }; + + + /********* + ** Accessors + *********/ + /// The descriptors for the diagnostics that this analyzer is capable of producing. + public override ImmutableArray SupportedDiagnostics { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + public ObsoleteFieldAnalyzer() + { + this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); + } + + /// Called once at session start to register actions in the analysis context. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + // SMAPI003: avoid obsolete fields + context.RegisterSyntaxNodeAction( + this.AnalyzeObsoleteFields, + SyntaxKind.SimpleMemberAccessExpression + ); + } + + + /********* + ** Private methods + *********/ + /// Analyse a syntax node and add a diagnostic message if it references an obsolete field. + /// The analysis context. + private void AnalyzeObsoleteFields(SyntaxNodeAnalysisContext context) + { + try + { + // get reference info + MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; + ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; + string propertyName = node.Name.Identifier.Text; + + // suggest replacement + for (ITypeSymbol type = declaringType; type != null; type = type.BaseType) + { + if (this.ReplacedFields.TryGetValue($"{type}::{propertyName}", out string replacement)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI003"], context.Node.GetLocation(), $"{type}.{propertyName}", replacement)); + break; + } + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + } +} -- cgit