From 13f31e8b725e46ca8442a943a5675723d22b4fdc Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:57 -0400 Subject: [PATCH] warn for fields which no longer work (#471) --- docs/mod-build-config.md | 6 ++ .../Mock/StardewValley/Farmer.cs | 11 +++ ...{UnitTests.cs => NetFieldAnalyzerTests.cs} | 14 +-- .../ObsoleteFieldAnalyzerTests.cs | 88 +++++++++++++++++ .../ObsoleteFieldAnalyzer.cs | 98 +++++++++++++++++++ 5 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs rename src/SMAPI.ModBuildConfig.Analyzer.Tests/{UnitTests.cs => NetFieldAnalyzerTests.cs} (92%) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs 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/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs similarity index 92% rename from src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs rename to src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs index 8ca27847..101f4c21 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -6,14 +6,14 @@ using StardewModdingAPI.ModBuildConfig.Analyzer; namespace SMAPI.ModBuildConfig.Analyzer.Tests { - /// Unit tests for the C# analyzers. + /// Unit tests for . [TestFixture] - public class UnitTests : DiagnosticVerifier + public class NetFieldAnalyzerTests : 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. + /// 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; @@ -88,13 +88,13 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { // arrange - string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); + 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", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } }; // assert @@ -114,13 +114,13 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) { // arrange - string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); + 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", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } }; // assert 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/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', ' ')}"); + } + } + } +}