add mod code analyzers to detect implicit net field conversion issues (#471)

5 files changed, 257 insertions, 0 deletions

diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs
new file mode 100644
index 00000000..d23bdc2e
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs
@@ -0,0 +1,107 @@
+using System;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace StardewModdingAPI.ModBuildConfig.Analyzer
+{
+    /// <summary>Detects implicit conversion from Stardew Valley's <c>Netcode</c> types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks.</summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class ImplicitNetFieldCastAnalyzer : DiagnosticAnalyzer
+    {
+        /*********
+        ** Properties
+        *********/
+        /// <summary>The namespace for Stardew Valley's <c>Netcode</c> types.</summary>
+        private const string NetcodeNamespace = "Netcode";
+
+        /// <summary>Describes the diagnostic rule covered by the analyzer.</summary>
+        private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
+            id: "SMAPI001",
+            title: "Netcode types shouldn't be implicitly converted",
+            messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.",
+            category: "SMAPI.CommonErrors",
+            defaultSeverity: DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            description: "",
+            helpLinkUri: "https://smapi.io/buildmsg/SMAPI001"
+        );
+
+
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>The descriptors for the diagnostics that this analyzer is capable of producing.</summary>
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Construct an instance.</summary>
+        public ImplicitNetFieldCastAnalyzer()
+        {
+            this.SupportedDiagnostics = ImmutableArray.Create(ImplicitNetFieldCastAnalyzer.Rule);
+        }
+
+        /// <summary>Called once at session start to register actions in the analysis context.</summary>
+        /// <param name="context">The analysis context.</param>
+        public override void Initialize(AnalysisContext context)
+        {
+            context.RegisterSyntaxNodeAction(
+                this.Analyse,
+                SyntaxKind.EqualsExpression,
+                SyntaxKind.NotEqualsExpression,
+                SyntaxKind.GreaterThanExpression,
+                SyntaxKind.GreaterThanOrEqualExpression,
+                SyntaxKind.LessThanExpression,
+                SyntaxKind.LessThanOrEqualExpression
+            );
+        }
+
+
+        /*********
+        ** Private methods
+        *********/
+        /// <summary>Analyse a syntax node and add a diagnostic message if applicable.</summary>
+        /// <param name="context">The analysis context.</param>
+        private void Analyse(SyntaxNodeAnalysisContext context)
+        {
+            try
+            {
+                BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node;
+                bool leftHasWarning = this.Analyze(context, node.Left);
+                if (!leftHasWarning)
+                    this.Analyze(context, node.Right);
+            }
+            catch (Exception ex)
+            {
+                throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}");
+            }
+        }
+
+        /// <summary>Analyse one operand in a binary expression (like <c>a</c> and <c>b</c> in <c>a == b</c>) and add a diagnostic message if applicable.</summary>
+        /// <param name="context">The analysis context.</param>
+        /// <param name="operand">The operand expression.</param>
+        /// <returns>Returns whether a diagnostic message was raised.</returns>
+        private bool Analyze(SyntaxNodeAnalysisContext context, ExpressionSyntax operand)
+        {
+            const string netcodeNamespace = ImplicitNetFieldCastAnalyzer.NetcodeNamespace;
+
+            TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand);
+            string fromNamespace = operandType.Type?.ContainingNamespace?.Name;
+            string toNamespace = operandType.ConvertedType?.ContainingNamespace?.Name;
+            if (fromNamespace == netcodeNamespace && fromNamespace != toNamespace && toNamespace != null)
+            {
+                string fromTypeName = operandType.Type.Name;
+                string toTypeName = operandType.ConvertedType.Name;
+                context.ReportDiagnostic(Diagnostic.Create(ImplicitNetFieldCastAnalyzer.Rule, context.Node.GetLocation(), operand, fromTypeName, toTypeName));
+                return true;
+            }
+
+            return false;
+        }
+    }
+}
diff --git a/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs b/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs
new file mode 100644
index 00000000..1cc41000
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs
@@ -0,0 +1,4 @@
+using System.Reflection;
+
+[assembly: AssemblyTitle("SMAPI.ModBuildConfig.Analyzer")]
+[assembly: AssemblyDescription("")]
diff --git a/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj b/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj
new file mode 100644
index 00000000..c32343e3
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj
@@ -0,0 +1,23 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>netstandard1.3</TargetFramework>
+    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <IncludeBuildOutput>false</IncludeBuildOutput>
+    <OutputPath>bin</OutputPath>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Compile Include="..\..\build\GlobalAssemblyInfo.cs" Link="Properties\GlobalAssemblyInfo.cs" />
+  </ItemGroup>
+   
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.4.0" PrivateAssets="all" />
+    <PackageReference Update="NETStandard.Library" PrivateAssets="all" />
+  </ItemGroup>
+  
+  <ItemGroup>
+    <None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
+  </ItemGroup>
+
+</Project>
diff --git a/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 b/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1
new file mode 100644
index 00000000..ff051759
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1
@@ -0,0 +1,58 @@
+param($installPath, $toolsPath, $package, $project)
+
+if($project.Object.SupportsPackageDependencyResolution)
+{
+    if($project.Object.SupportsPackageDependencyResolution())
+    {
+        # Do not install analyzers via install.ps1, instead let the project system handle it.
+        return
+    }
+}
+
+$analyzersPaths = Join-Path (Join-Path (Split-Path -Path $toolsPath -Parent) "analyzers") * -Resolve
+
+foreach($analyzersPath in $analyzersPaths)
+{
+    if (Test-Path $analyzersPath)
+    {
+        # Install the language agnostic analyzers.
+        foreach ($analyzerFilePath in Get-ChildItem -Path "$analyzersPath\*.dll" -Exclude *.resources.dll)
+        {
+            if($project.Object.AnalyzerReferences)
+            {
+                $project.Object.AnalyzerReferences.Add($analyzerFilePath.FullName)
+            }
+        }
+    }
+}
+
+# $project.Type gives the language name like (C# or VB.NET)
+$languageFolder = ""
+if($project.Type -eq "C#")
+{
+    $languageFolder = "cs"
+}
+if($project.Type -eq "VB.NET")
+{
+    $languageFolder = "vb"
+}
+if($languageFolder -eq "")
+{
+    return
+}
+
+foreach($analyzersPath in $analyzersPaths)
+{
+    # Install language specific analyzers.
+    $languageAnalyzersPath = join-path $analyzersPath $languageFolder
+    if (Test-Path $languageAnalyzersPath)
+    {
+        foreach ($analyzerFilePath in Get-ChildItem -Path "$languageAnalyzersPath\*.dll" -Exclude *.resources.dll)
+        {
+            if($project.Object.AnalyzerReferences)
+            {
+                $project.Object.AnalyzerReferences.Add($analyzerFilePath.FullName)
+            }
+        }
+    }
+}
diff --git a/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 b/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1
new file mode 100644
index 00000000..4bed3337
--- /dev/null
+++ b/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1
@@ -0,0 +1,65 @@
+param($installPath, $toolsPath, $package, $project)
+
+if($project.Object.SupportsPackageDependencyResolution)
+{
+    if($project.Object.SupportsPackageDependencyResolution())
+    {
+        # Do not uninstall analyzers via uninstall.ps1, instead let the project system handle it.
+        return
+    }
+}
+
+$analyzersPaths = Join-Path (Join-Path (Split-Path -Path $toolsPath -Parent) "analyzers") * -Resolve
+
+foreach($analyzersPath in $analyzersPaths)
+{
+    # Uninstall the language agnostic analyzers.
+    if (Test-Path $analyzersPath)
+    {
+        foreach ($analyzerFilePath in Get-ChildItem -Path "$analyzersPath\*.dll" -Exclude *.resources.dll)
+        {
+            if($project.Object.AnalyzerReferences)
+            {
+                $project.Object.AnalyzerReferences.Remove($analyzerFilePath.FullName)
+            }
+        }
+    }
+}
+
+# $project.Type gives the language name like (C# or VB.NET)
+$languageFolder = ""
+if($project.Type -eq "C#")
+{
+    $languageFolder = "cs"
+}
+if($project.Type -eq "VB.NET")
+{
+    $languageFolder = "vb"
+}
+if($languageFolder -eq "")
+{
+    return
+}
+
+foreach($analyzersPath in $analyzersPaths)
+{
+    # Uninstall language specific analyzers.
+    $languageAnalyzersPath = join-path $analyzersPath $languageFolder
+    if (Test-Path $languageAnalyzersPath)
+    {
+        foreach ($analyzerFilePath in Get-ChildItem -Path "$languageAnalyzersPath\*.dll" -Exclude *.resources.dll)
+        {
+            if($project.Object.AnalyzerReferences)
+            {
+                try
+                {
+                    $project.Object.AnalyzerReferences.Remove($analyzerFilePath.FullName)
+                }
+                catch
+                {
+
+                }
+            }
+        }
+    }
+}