diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/ContentManagerAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ContentManagerAnalyzerTests.cs new file mode 100644 index 000000000..6a7e6ccf0 --- /dev/null +++ b/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(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/DataLoader.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/DataLoader.cs new file mode 100644 index 000000000..646b8c7e7 --- /dev/null +++ b/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!; + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Game1.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Game1.cs new file mode 100644 index 000000000..b5849eaf0 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Game1.cs @@ -0,0 +1,7 @@ +namespace StardewValley +{ + public class Game1 + { + public static LocalizedContentManager content = new(); + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/LocalizedContentManager.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/LocalizedContentManager.cs new file mode 100644 index 000000000..3b88a8c56 --- /dev/null +++ b/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!; + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ContentManagerAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ContentManagerAnalyzer.cs new file mode 100644 index 000000000..3330af31e --- /dev/null +++ b/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)); + } + } + } +}