Add analyzers to detect incorrect usage of Content Managers for vanilla assets

src/SMAPI.ModBuildConfig.Analyzer.Tests/ContentManagerAnalyzerTests.cs

@@ -0,0 +1,153 @@
+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
+{
+    /// <summary>Unit tests for <see cref="ContentManagerAnalyzer"/>.</summary>
+    [TestFixture]
+    public class ContentManagerAnalyzerTests : DiagnosticVerifier
+    {
+        /*********
+        ** Fields
+        *********/
+        /// <summary>Sample C# mod code, with a {{test-code}} placeholder for the code in the Entry method to test.</summary>
+        const string SampleProgram = @"
+            using System;
+            using System.Collections.Generic;
+            using StardewValley;
+            using Netcode;
+            using SObject = StardewValley.Object;
+
+            namespace SampleMod
+            {
+                class ModEntry
+                {
+                    public void Entry()
+                    {
+                        {{test-code}}
+                    }
+                }
+            }
+        ";
+
+        const string SampleUnrelatedGoodProgram = @"
+            using System;
+            using System.Collections.Generic;
+
+            namespace Sample;
+            class Loader
+            {
+                public T Load<T>(string arg)
+                {
+                    return default(T);
+                }
+            }
+            class ModEntry
+            {
+                public void Entry()
+                {
+                    var loader = new Loader();
+                    var test = loader.Load<Dictionary<int,string>>(""Data\Fish"");
+                }
+            }
+        ";
+
+        /// <summary>The line number where the unit tested code is injected into <see cref="SampleProgram"/>.</summary>
+        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;
+
+
+        /*********
+        ** Unit tests
+        *********/
+        /// <summary>Test that no diagnostics are raised for an empty code block.</summary>
+        [TestCase]
+        public void EmptyCode_HasNoDiagnostics()
+        {
+            // arrange
+            string test = @"";
+
+            // assert
+            this.VerifyCSharpDiagnostic(test);
+        }
+
+        /// <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>
+        /// <param name="expression">The expression which should be reported.</param>
+        /// <param name="netType">The net type name which should be reported.</param>
+        /// <param name="suggestedProperty">The suggested property name which should be reported.</param>
+        [TestCase("Game1.content.Load<Dictionary<int, string>>(\"Data\\\\Fish\");", 0, "Data\\Fish", "System.Collections.Generic.Dictionary<int, string>", "System.Collections.Generic.Dictionary<string, string>")]
+        public void BadType_RaisesDiagnostic(string codeText, int column, string assetName, string expectedType, string suggestedType)
+        {
+            // arrange
+            string code = SampleProgram.Replace("{{test-code}}", codeText);
+            DiagnosticResult expected = new()
+            {
+                Id = "AvoidContentManagerBadType",
+                Message = $"'{assetName}' uses the {suggestedType} type, but {expectedType} is in use instead. See https://smapi.io/package/avoid-contentmanager-type for details.",
+                Severity = DiagnosticSeverity.Error,
+                Locations = [new DiagnosticResultLocation("Test0.cs", SampleCodeLine, SampleCodeColumn + column)]
+            };
+            DiagnosticResult preferDataLoader = new()
+            {
+                Id = "PreferContentManagerDataLoader",
+                Message = $"'{assetName}' can be accessed using 'DataLoader.{assetName[5..]}(LocalizedContentManager content)' instead. See https://smapi.io/package/prefer-contentmanager-dataloader for details.",
+                Severity = DiagnosticSeverity.Info,
+                Locations = [new DiagnosticResultLocation("Test0.cs", SampleCodeLine, SampleCodeColumn + column)]
+            };
+
+            // assert
+            this.VerifyCSharpDiagnostic(code, expected, preferDataLoader);
+        }
+
+
+
+        /// <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>
+        /// <param name="expression">The expression which should be reported.</param>
+        /// <param name="netType">The net type name which should be reported.</param>
+        /// <param name="suggestedProperty">The suggested property name which should be reported.</param>
+        [TestCase("Game1.content.Load<Dictionary<string, string>>(\"Data\\\\Fish\");", 0, "Data\\Fish")]
+        public void PreferDataLoader_RaisesDiagnostic(string codeText, int column, string assetName)
+        {
+            // arrange
+            string code = SampleProgram.Replace("{{test-code}}", codeText);
+            DiagnosticResult preferDataLoader = new()
+            {
+                Id = "PreferContentManagerDataLoader",
+                Message = $"'{assetName}' can be accessed using 'DataLoader.{assetName[5..]}(LocalizedContentManager content)' instead. See https://smapi.io/package/prefer-contentmanager-dataloader for details.",
+                Severity = DiagnosticSeverity.Info,
+                Locations = [new DiagnosticResultLocation("Test0.cs", SampleCodeLine, SampleCodeColumn + column)]
+            };
+
+            // assert
+            this.VerifyCSharpDiagnostic(code, preferDataLoader);
+        }
+
+        [TestCase("Game1.content.Load<Dictionary<string, string>>(\"Data\\\\Custom_Asset\");", true)]
+        [TestCase(SampleUnrelatedGoodProgram, false)]
+
+        public void ValidCode_HasNoDiagnostics(string codeText, bool useWrapper)
+        {
+            string code = useWrapper ? SampleProgram.Replace("{{test-code}}", codeText) : codeText;
+            this.VerifyCSharpDiagnostic(code);
+        }
+
+
+        /*********
+        ** Helpers
+        *********/
+        /// <summary>Get the analyzer being tested.</summary>
+        protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
+        {
+            return new ContentManagerAnalyzer();
+        }
+    }
+}

src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/DataLoader.cs

@@ -0,0 +1,15 @@
+// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code
+// ReSharper disable UnusedMember.Global -- used dynamically for unit tests
+using System.Collections.Generic;
+
+namespace StardewValley
+{
+    /// <summary>A simplified version of Stardew Valley's <c>StardewValley.DataLoader</c> class for unit testing.</summary>
+    public static class DataLoader
+    {
+        public static Dictionary<string, string> Fish(LocalizedContentManager content)
+        {
+            return default!;
+        }
+    }
+}

src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Game1.cs

@@ -0,0 +1,7 @@
+namespace StardewValley
+{
+    public class Game1
+    {
+        public static LocalizedContentManager content = new();
+    }
+}

src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/LocalizedContentManager.cs

@@ -0,0 +1,13 @@
+// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code
+// ReSharper disable UnusedMember.Global -- used dynamically for unit tests
+namespace StardewValley
+{
+    /// <summary>A simplified version of Stardew Valley's <c>StardewValley.LocalizedContentManager</c> class for unit testing.</summary>
+    public class LocalizedContentManager
+    {
+        public T Load<T>(string assetName)
+        {
+            return default!;
+        }
+    }
+}

src/SMAPI.ModBuildConfig.Analyzer/ContentManagerAnalyzer.cs

@@ -0,0 +1,86 @@
+using System;
+using System.Collections.Generic;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using System.Reflection;
+using System.Linq;
+
+namespace StardewModdingAPI.ModBuildConfig.Analyzer
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class ContentManagerAnalyzer : DiagnosticAnalyzer
+    {
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+
+
+        /// <summary>The diagnostic info for an avoidable runtime casting error.</summary>
+        private readonly DiagnosticDescriptor AvoidBadTypeRule = new(
+            id: "AvoidContentManagerBadType",
+            title: "Avoid incorrectly typing ContentManager Loads",
+            messageFormat: "'{0}' uses the {1} type, but {2} is in use instead. See https://smapi.io/package/avoid-contentmanager-type for details.",
+            category: "SMAPI.CommonErrors",
+            defaultSeverity: DiagnosticSeverity.Error,
+            isEnabledByDefault: true,
+            helpLinkUri: "https://smapi.io/package/avoid-contentmanager-type"
+        );
+        /// <summary>The diagnostic info for best practices using DataLoader</summary>
+        private readonly DiagnosticDescriptor PreferDataLoader = new(
+            id: "PreferContentManagerDataLoader",
+            title: "Prefer using DataLoader to ContentManager Loads",
+            messageFormat: "'{0}' can be accessed using 'DataLoader.{1}(LocalizedContentManager content)' instead. See https://smapi.io/package/prefer-contentmanager-dataloader for details.",
+            category: "SMAPI.CommonErrors",
+            defaultSeverity: DiagnosticSeverity.Info,
+            isEnabledByDefault: true,
+            helpLinkUri: "https://smapi.io/package/prefer-contentmanager-dataloader"
+        );
+
+        public ContentManagerAnalyzer()
+        {
+            this.SupportedDiagnostics = ImmutableArray.CreateRange(new[] { this.AvoidBadTypeRule, this.PreferDataLoader });
+        }
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+            context.EnableConcurrentExecution();
+
+            context.RegisterSyntaxNodeAction(
+                this.AnalyzeContentManagerLoads,
+                SyntaxKind.InvocationExpression
+            );
+        }
+
+        private void AnalyzeContentManagerLoads(SyntaxNodeAnalysisContext context)
+        {
+            var invocation = (InvocationExpressionSyntax)context.Node;
+            var memberAccess = invocation.Expression as MemberAccessExpressionSyntax;
+            if (memberAccess == null || memberAccess.Name.Identifier.ValueText != "Load")
+                return;
+            string? loadNamespace = context.SemanticModel.GetSymbolInfo(memberAccess).Symbol?.ContainingNamespace.Name;
+            if (!(loadNamespace == "StardewValley" || loadNamespace == "StardewModdingAPI"))
+                return;
+            // "Data\\Fish" -> Data\Fish
+            string assetName = invocation.ArgumentList.Arguments[0].ToString().Replace("\"", "").Replace("\\\\", "\\");
+
+            if (!assetName.StartsWith("Data", StringComparison.InvariantCultureIgnoreCase)) return;
+            string dataAsset = assetName.Substring(5);
+
+            var dataLoader = context.Compilation.GetTypeByMetadataName("StardewValley.DataLoader");
+            var dataMatch = dataLoader.GetMembers().FirstOrDefault(m => m.Name == dataAsset);
+            if (dataMatch == null) return;
+            if (dataMatch is IMethodSymbol method)
+            {
+                var genericArgument = context.SemanticModel.GetTypeInfo((memberAccess.Name as GenericNameSyntax).TypeArgumentList.Arguments[0]).Type;
+                // Can't use the proper way of using SymbolEquityComparer due to System.Collections overlapping with CoreLib.
+                if (method.ReturnType.ToString() != genericArgument.ToString())
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(this.AvoidBadTypeRule, context.Node.GetLocation(), assetName, method.ReturnType.ToString(), genericArgument.ToString()));
+                }
+                context.ReportDiagnostic(Diagnostic.Create(this.PreferDataLoader, context.Node.GetLocation(), assetName, dataAsset));
+            }
+        }
+    }
+}