don't warn for NetList conversion to implemented interface (#471)

This commit is contained in:
Jesse Plamondon-Willard 2018-04-14 19:51:50 -04:00
parent c2cb76b799
commit 6d8cf614a2
7 changed files with 83 additions and 6 deletions

View File

@ -0,0 +1,9 @@
// ReSharper disable CheckNamespace -- matches Stardew Valley's code
using System.Collections;
using System.Collections.Generic;
namespace Netcode
{
/// <summary>A simplified version of Stardew Valley's <c>Netcode.NetObjectList</c> for unit testing.</summary>
public class NetList<T> : List<T>, IList<T>, ICollection<T>, IEnumerable<T>, IEnumerable { }
}

View File

@ -0,0 +1,6 @@
// ReSharper disable CheckNamespace -- matches Stardew Valley's code
namespace Netcode
{
/// <summary>A simplified version of Stardew Valley's <c>Netcode.NetObjectList</c> for unit testing.</summary>
public class NetObjectList<T> : NetList<T> { }
}

View File

@ -1,11 +1,20 @@
// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code
#pragma warning disable 649 // (never assigned) -- only used to test type conversions
using System.Collections.Generic;
using Netcode;
namespace StardewValley
{
/// <summary>A simplified version of Stardew Valley's <c>StardewValley.Farmer</c> class for unit testing.</summary>
internal class Farmer
{
public IDictionary<string, int[]> friendships;
/// <summary>A sample field which should be replaced with a different property.</summary>
public readonly IDictionary<string, int[]> friendships;
/// <summary>A sample net list.</summary>
public readonly NetList<int> eventsSeen;
/// <summary>A sample net object list.</summary>
public readonly NetObjectList<int> netObjectList;
}
}

View File

@ -19,6 +19,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
using StardewValley;
using Netcode;
using SObject = StardewValley.Object;
using SFarmer = StardewValley.Farmer;
namespace SampleMod
{
@ -33,7 +34,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
";
/// <summary>The line number where the unit tested code is injected into <see cref="SampleProgram"/>.</summary>
private const int SampleCodeLine = 13;
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;
@ -85,6 +86,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
[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")]
[TestCase("SFarmer farmer = new SFarmer(); object list = farmer.eventsSeen;", 46, "farmer.eventsSeen", "NetList", "object")] // ↓ NetList field converted to a non-interface type
public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType)
{
// arrange
@ -101,6 +103,21 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests
this.VerifyCSharpDiagnostic(code, expected);
}
/// <summary>Test that the net field analyzer doesn't raise any warnings for safe member access.</summary>
/// <param name="codeText">The code line to test.</param>
[TestCase("SFarmer farmer = new SFarmer(); System.Collections.IEnumerable list = farmer.eventsSeen;")]
[TestCase("SFarmer farmer = new SFarmer(); System.Collections.Generic.IEnumerable<int> list = farmer.eventsSeen;")]
[TestCase("SFarmer farmer = new SFarmer(); System.Collections.Generic.IList<int> list = farmer.eventsSeen;")]
[TestCase("SFarmer farmer = new SFarmer(); System.Collections.Generic.IList<int> list = farmer.netObjectList;")] // subclass of NetList
public void AvoidImplicitNetFieldComparisons_AllowsSafeAccess(string codeText)
{
// arrange
string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText);
// assert
this.VerifyCSharpDiagnostic(code);
}
/// <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>

View File

@ -1,3 +1,4 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
@ -42,5 +43,16 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
memberName = null;
return false;
}
/// <summary>Get the class types in a type's inheritance chain, including itself.</summary>
/// <param name="type">The initial type.</param>
public static IEnumerable<ITypeSymbol> GetConcreteTypes(ITypeSymbol type)
{
while (type != null)
{
yield return type;
type = type.BaseType;
}
}
}
}

View File

@ -1,6 +1,8 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
@ -17,6 +19,9 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
/// <summary>The namespace for Stardew Valley's <c>Netcode</c> types.</summary>
private const string NetcodeNamespace = "Netcode";
/// <summary>The full name for Stardew Valley's <c>Netcode.NetList</c> type.</summary>
private readonly string NetListTypeFullName = "Netcode.NetList";
/// <summary>Maps net fields to their equivalent non-net properties where available.</summary>
private readonly IDictionary<string, string> NetFieldWrapperProperties = new Dictionary<string, string>
{
@ -190,10 +195,9 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
return;
if (!this.IsNetType(memberType.Type))
return;
bool isConverted = !this.IsNetType(memberType.ConvertedType);
// warn: use property wrapper if available
for (ITypeSymbol type = declaringType; type != null; type = type.BaseType)
foreach (ITypeSymbol type in AnalyzerUtilities.GetConcreteTypes(declaringType))
{
if (this.NetFieldWrapperProperties.TryGetValue($"{type}::{memberName}", out string suggestedPropertyName))
{
@ -203,7 +207,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
}
// warn: implicit conversion
if (isConverted)
if (this.IsInvalidConversion(memberType))
context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidImplicitNetFieldCast"], context.Node.GetLocation(), context.Node, memberType.Type.Name, memberType.ConvertedType));
}
catch (Exception ex)
@ -212,6 +216,26 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
}
}
/// <summary>Get whether a net field was converted in an error-prone way.</summary>
/// <param name="typeInfo">The member access type info.</param>
private bool IsInvalidConversion(TypeInfo typeInfo)
{
// no conversion
if (!this.IsNetType(typeInfo.Type) || this.IsNetType(typeInfo.ConvertedType))
return false;
// list conversion to an implemented interface is OK
if (AnalyzerUtilities.GetConcreteTypes(typeInfo.Type).Any(p => p.ToString().StartsWith(this.NetListTypeFullName))) // StartsWith to ignore generics
{
string toType = typeInfo.ConvertedType.ToString();
if (toType.StartsWith(typeof(IEnumerable<>).Namespace) || toType == typeof(IEnumerable).FullName)
return false;
}
// avoid any other conversions
return true;
}
/// <summary>Get whether a type symbol references a <c>Netcode</c> type.</summary>
/// <param name="typeSymbol">The type symbol.</param>
private bool IsNetType(ITypeSymbol typeSymbol)

View File

@ -79,7 +79,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer
return;
// suggest replacement
for (ITypeSymbol type = declaringType; type != null; type = type.BaseType)
foreach (ITypeSymbol type in AnalyzerUtilities.GetConcreteTypes(declaringType))
{
if (this.ReplacedFields.TryGetValue($"{type}::{memberName}", out string replacement))
{