diff options
Diffstat (limited to 'src')
4 files changed, 112 insertions, 46 deletions
diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs index ab3f2c5e..6f8c8b9b 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -59,7 +59,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests /// <param name="expression">The expression which should be reported.</param> /// <param name="fromType">The source type name which should be reported.</param> /// <param name="toType">The target type name which should be reported.</param> - [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")] // ↓ implicit conversion [TestCase("Item item = null; if (item.netIntField <= 42);", 22, "item.netIntField", "NetInt", "int")] [TestCase("Item item = null; if (item.netIntField > 42);", 22, "item.netIntField", "NetInt", "int")] [TestCase("Item item = null; if (item.netIntField >= 42);", 22, "item.netIntField", "NetInt", "int")] @@ -79,7 +79,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests [TestCase("Item item = null; if (item.netRefField != null);", 22, "item.netRefField", "NetRef", "object")] [TestCase("Item item = null; if (item.netRefProperty == null);", 22, "item.netRefProperty", "NetRef", "object")] [TestCase("Item item = null; if (item.netRefProperty != null);", 22, "item.netRefProperty", "NetRef", "object")] - [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ same as above, but inherited from base class + [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ implicit conversion for parent field [TestCase("SObject obj = null; if (obj.netIntProperty != 42);", 24, "obj.netIntProperty", "NetInt", "int")] [TestCase("SObject obj = null; if (obj.netRefField == null);", 24, "obj.netRefField", "NetRef", "object")] [TestCase("SObject obj = null; if (obj.netRefField != null);", 24, "obj.netRefField", "NetRef", "object")] @@ -87,6 +87,8 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests [TestCase("SObject obj = null; if (obj.netRefProperty != null);", 24, "obj.netRefProperty", "NetRef", "object")] [TestCase("Item item = new Item(); object list = item.netList;", 38, "item.netList", "NetList", "object")] // ↓ NetList field converted to a non-interface type [TestCase("Item item = new Item(); object list = item.netCollection;", 38, "item.netCollection", "NetCollection", "object")] + [TestCase("Item item = new Item(); int x = (int)item.netIntField;", 32, "item.netIntField", "NetInt", "int")] // ↓ explicit conversion to invalid type + [TestCase("Item item = new Item(); int x = item.netRefField as object;", 32, "item.netRefField", "NetRef", "object")] public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { // arrange diff --git a/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs b/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs index e0c0cd63..68b5001e 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/AnalyzerUtilities.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace StardewModdingAPI.ModBuildConfig.Analyzer @@ -10,6 +11,40 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /********* ** Public methods *********/ + /// <summary>Get the metadata for an explicit cast or 'x as y' expression.</summary> + /// <param name="node">The member access expression.</param> + /// <param name="semanticModel">provides methods for asking semantic questions about syntax nodes.</param> + /// <param name="fromExpression">The expression whose value is being converted.</param> + /// <param name="fromType">The type being converted from.</param> + /// <param name="toType">The type being converted to.</param> + /// <returns>Returns true if the node is a matched expression, else false.</returns> + public static bool TryGetCastOrAsInfo(SyntaxNode node, SemanticModel semanticModel, out ExpressionSyntax fromExpression, out TypeInfo fromType, out TypeInfo toType) + { + // (type)x + if (node is CastExpressionSyntax cast) + { + fromExpression = cast.Expression; + fromType = semanticModel.GetTypeInfo(fromExpression); + toType = semanticModel.GetTypeInfo(cast.Type); + return true; + } + + // x as y + if (node is BinaryExpressionSyntax binary && binary.Kind() == SyntaxKind.AsExpression) + { + fromExpression = binary.Left; + fromType = semanticModel.GetTypeInfo(fromExpression); + toType = semanticModel.GetTypeInfo(binary.Right); + return true; + } + + // invalid + fromExpression = null; + fromType = default(TypeInfo); + toType = default(TypeInfo); + return false; + } + /// <summary>Get the metadata for a member access expression.</summary> /// <param name="node">The member access expression.</param> /// <param name="semanticModel">provides methods for asking semantic questions about syntax nodes.</param> @@ -17,7 +52,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// <param name="memberType">The type of the accessed member.</param> /// <param name="memberName">The name of the accessed member.</param> /// <returns>Returns true if the node is a member access expression, else false.</returns> - public static bool GetMemberInfo(SyntaxNode node, SemanticModel semanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName) + public static bool TryGetMemberInfo(SyntaxNode node, SemanticModel semanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName) { // simple access if (node is MemberAccessExpressionSyntax memberAccess) diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index 39b9abc1..d1e5c59a 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -125,28 +125,27 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer ["StardewValley.Tool::upgradeLevel"] = "UpgradeLevel" }; - /// <summary>Describes the diagnostic rule covered by the analyzer.</summary> - private readonly IDictionary<string, DiagnosticDescriptor> Rules = new Dictionary<string, DiagnosticDescriptor> - { - ["AvoidImplicitNetFieldCast"] = new DiagnosticDescriptor( - id: "AvoidImplicitNetFieldCast", - 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/avoid-implicit-net-field-cast for details.", - category: "SMAPI.CommonErrors", - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true, - helpLinkUri: "https://smapi.io/buildmsg/avoid-implicit-net-field-cast" - ), - ["AvoidNetField"] = new DiagnosticDescriptor( - id: "AvoidNetField", - title: "Avoid Netcode types when possible", - messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/avoid-net-field for details.", - category: "SMAPI.CommonErrors", - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true, - helpLinkUri: "https://smapi.io/buildmsg/avoid-net-field" - ) - }; + /// <summary>The diagnostic info for an implicit net field cast.</summary> + private readonly DiagnosticDescriptor AvoidImplicitNetFieldCastRule = new DiagnosticDescriptor( + id: "AvoidImplicitNetFieldCast", + 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/avoid-implicit-net-field-cast for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://smapi.io/buildmsg/avoid-implicit-net-field-cast" + ); + + /// <summary>The diagnostic info for an avoidable net field access.</summary> + private readonly DiagnosticDescriptor AvoidNetFieldRule = new DiagnosticDescriptor( + id: "AvoidNetField", + title: "Avoid Netcode types when possible", + messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/avoid-net-field for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://smapi.io/buildmsg/avoid-net-field" + ); /********* @@ -162,7 +161,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// <summary>Construct an instance.</summary> public NetFieldAnalyzer() { - this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); + this.SupportedDiagnostics = ImmutableArray.CreateRange(new[] { this.AvoidNetFieldRule, this.AvoidImplicitNetFieldCastRule }); } /// <summary>Called once at session start to register actions in the analysis context.</summary> @@ -175,6 +174,11 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer SyntaxKind.ConditionalAccessExpression ); context.RegisterSyntaxNodeAction( + this.AnalyzeCast, + SyntaxKind.CastExpression, + SyntaxKind.AsExpression + ); + context.RegisterSyntaxNodeAction( this.AnalyzeBinaryComparison, SyntaxKind.EqualsExpression, SyntaxKind.NotEqualsExpression, @@ -189,15 +193,15 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /********* ** Private methods *********/ - /// <summary>Analyse a member access syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available.</summary> + /// <summary>Analyse a member access syntax node and add a diagnostic message if applicable.</summary> /// <param name="context">The analysis context.</param> /// <returns>Returns whether any warnings were added.</returns> private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context) { - try + this.HandleErrors(context.Node, () => { // get member access info - if (!AnalyzerUtilities.GetMemberInfo(context.Node, context.SemanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName)) + if (!AnalyzerUtilities.TryGetMemberInfo(context.Node, context.SemanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName)) return; if (!this.IsNetType(memberType.Type)) return; @@ -207,32 +211,45 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer { if (this.NetFieldWrapperProperties.TryGetValue($"{type}::{memberName}", out string suggestedPropertyName)) { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidNetField"], context.Node.GetLocation(), context.Node, memberType.Type.Name, suggestedPropertyName)); + context.ReportDiagnostic(Diagnostic.Create(this.AvoidNetFieldRule, context.Node.GetLocation(), context.Node, memberType.Type.Name, suggestedPropertyName)); return; } } // warn: implicit conversion - if (this.IsInvalidConversion(memberType)) + if (this.IsInvalidConversion(memberType.Type, memberType.ConvertedType)) { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidImplicitNetFieldCast"], context.Node.GetLocation(), context.Node, memberType.Type.Name, memberType.ConvertedType)); + context.ReportDiagnostic(Diagnostic.Create(this.AvoidImplicitNetFieldCastRule, context.Node.GetLocation(), context.Node, memberType.Type.Name, memberType.ConvertedType)); return; } - } - catch (Exception ex) + }); + } + + /// <summary>Analyse an explicit cast or 'x as y' node and add a diagnostic message if applicable.</summary> + /// <param name="context">The analysis context.</param> + /// <returns>Returns whether any warnings were added.</returns> + private void AnalyzeCast(SyntaxNodeAnalysisContext context) + { + // NOTE: implicit conversion within the expression is detected by the member access + // checks. This method is only concerned with the conversion of its final value. + this.HandleErrors(context.Node, () => { - throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); - } + if (AnalyzerUtilities.TryGetCastOrAsInfo(context.Node, context.SemanticModel, out ExpressionSyntax fromExpression, out TypeInfo fromType, out TypeInfo toType)) + { + if (this.IsInvalidConversion(fromType.ConvertedType, toType.Type)) + context.ReportDiagnostic(Diagnostic.Create(this.AvoidImplicitNetFieldCastRule, context.Node.GetLocation(), fromExpression, fromType.ConvertedType.Name, toType.Type)); + } + }); } - /// <summary>Analyse a binary comparison syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available.</summary> + /// <summary>Analyse a binary comparison syntax node and add a diagnostic message if applicable.</summary> /// <param name="context">The analysis context.</param> /// <returns>Returns whether any warnings were added.</returns> private void AnalyzeBinaryComparison(SyntaxNodeAnalysisContext context) { // NOTE: implicit conversion within an operand is detected by the member access checks. // This method is only concerned with the conversion of each side's final value. - try + this.HandleErrors(context.Node, () => { BinaryExpressionSyntax expression = (BinaryExpressionSyntax)context.Node; foreach (var pair in new[] { Tuple.Create(expression.Left, expression.Right), Tuple.Create(expression.Right, expression.Left) }) @@ -250,34 +267,46 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer Optional<object> otherValue = context.SemanticModel.GetConstantValue(otherExpression); if (otherValue.HasValue && otherValue.Value == null) { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidImplicitNetFieldCast"], context.Node.GetLocation(), curExpression, curType.Type.Name, "null")); + context.ReportDiagnostic(Diagnostic.Create(this.AvoidImplicitNetFieldCastRule, context.Node.GetLocation(), curExpression, curType.Type.Name, "null")); break; } // warn for implicit conversion if (!this.IsNetType(otherType.ConvertedType)) { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["AvoidImplicitNetFieldCast"], context.Node.GetLocation(), curExpression, curType.Type.Name, curType.ConvertedType)); + context.ReportDiagnostic(Diagnostic.Create(this.AvoidImplicitNetFieldCastRule, context.Node.GetLocation(), curExpression, curType.Type.Name, curType.ConvertedType)); break; } } + }); + } + + /// <summary>Handle exceptions raised while analyzing a node.</summary> + /// <param name="node">The node being analysed.</param> + /// <param name="action">The callback to invoke.</param> + private void HandleErrors(SyntaxNode node, Action action) + { + try + { + action(); } catch (Exception ex) { - throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + throw new InvalidOperationException($"Failed processing expression: '{node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); } } /// <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) + /// <param name="fromType">The source type.</param> + /// <param name="toType">The target type.</param> + private bool IsInvalidConversion(ITypeSymbol fromType, ITypeSymbol toType) { // no conversion - if (!this.IsNetType(typeInfo.Type) || this.IsNetType(typeInfo.ConvertedType)) + if (!this.IsNetType(fromType) || this.IsNetType(toType)) return false; // conversion to implemented interface is OK - if (typeInfo.Type.AllInterfaces.Contains(typeInfo.ConvertedType)) + if (fromType.AllInterfaces.Contains(toType)) return false; // avoid any other conversions diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs index a770f47d..3d353e52 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs @@ -74,7 +74,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer try { // get reference info - if (!AnalyzerUtilities.GetMemberInfo(context.Node, context.SemanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName)) + if (!AnalyzerUtilities.TryGetMemberInfo(context.Node, context.SemanticModel, out ITypeSymbol declaringType, out TypeInfo memberType, out string memberName)) return; // suggest replacement |