From f52f7ca36f2ecf3d4478c5bc1e9cd95e5ff53929 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 19:32:00 -0400 Subject: add mod code analyzers to detect implicit net field conversion issues (#471) --- .../ImplicitNetFieldCastAnalyzer.cs | 107 +++++++++++++++++++++ .../Properties/AssemblyInfo.cs | 4 + ...tardewModdingAPI.ModBuildConfig.Analyzer.csproj | 23 +++++ .../tools/install.ps1 | 58 +++++++++++ .../tools/uninstall.ps1 | 65 +++++++++++++ 5 files changed, 257 insertions(+) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 (limited to 'src/SMAPI.ModBuildConfig.Analyzer') 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 +{ + /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ImplicitNetFieldCastAnalyzer : DiagnosticAnalyzer + { + /********* + ** Properties + *********/ + /// The namespace for Stardew Valley's Netcode types. + private const string NetcodeNamespace = "Netcode"; + + /// Describes the diagnostic rule covered by the analyzer. + 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 + *********/ + /// The descriptors for the diagnostics that this analyzer is capable of producing. + public override ImmutableArray SupportedDiagnostics { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + public ImplicitNetFieldCastAnalyzer() + { + this.SupportedDiagnostics = ImmutableArray.Create(ImplicitNetFieldCastAnalyzer.Rule); + } + + /// Called once at session start to register actions in the analysis context. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + context.RegisterSyntaxNodeAction( + this.Analyse, + SyntaxKind.EqualsExpression, + SyntaxKind.NotEqualsExpression, + SyntaxKind.GreaterThanExpression, + SyntaxKind.GreaterThanOrEqualExpression, + SyntaxKind.LessThanExpression, + SyntaxKind.LessThanOrEqualExpression + ); + } + + + /********* + ** Private methods + *********/ + /// Analyse a syntax node and add a diagnostic message if applicable. + /// The analysis context. + 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', ' ')}"); + } + } + + /// Analyse one operand in a binary expression (like a and b in a == b) and add a diagnostic message if applicable. + /// The analysis context. + /// The operand expression. + /// Returns whether a diagnostic message was raised. + 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 @@ + + + + netstandard1.3 + false + false + bin + + + + + + + + + + + + + + + + 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 + { + + } + } + } + } +} -- cgit From 4f5f463bd227a81c26224a81b178e2221946b9b2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 22:33:45 -0400 Subject: warn when directly using a net field that has a non-net wrapper (#471) --- .../ImplicitNetFieldCastAnalyzer.cs | 208 ++++++++++++++++++--- 1 file changed, 183 insertions(+), 25 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs index d23bdc2e..d64b1486 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -9,7 +10,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer { /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class ImplicitNetFieldCastAnalyzer : DiagnosticAnalyzer + public class NetFieldAnalyzer : DiagnosticAnalyzer { /********* ** Properties @@ -17,17 +18,136 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// The namespace for Stardew Valley's Netcode types. private const string NetcodeNamespace = "Netcode"; + /// Maps net fields to their equivalent non-net properties where available. + private readonly IDictionary NetFieldWrapperProperties = new Dictionary + { + // Character + ["StardewValley.Character::currentLocationRef"] = "currentLocation", + ["StardewValley.Character::facingDirection"] = "FacingDirection", + ["StardewValley.Character::name"] = "Name", + ["StardewValley.Character::position"] = "Position", + ["StardewValley.Character::scale"] = "Scale", + ["StardewValley.Character::speed"] = "Speed", + ["StardewValley.Character::sprite"] = "Sprite", + + // Chest + ["StardewValley.Objects.Chest::tint"] = "Tint", + + // Farmer + ["StardewValley.Farmer::houseUpgradeLevel"] = "HouseUpgradeLevel", + ["StardewValley.Farmer::isMale"] = "IsMale", + ["StardewValley.Farmer::items"] = "Items", + ["StardewValley.Farmer::magneticRadius"] = "MagneticRadius", + ["StardewValley.Farmer::stamina"] = "Stamina", + ["StardewValley.Farmer::uniqueMultiplayerID"] = "UniqueMultiplayerID", + ["StardewValley.Farmer::usingTool"] = "UsingTool", + + // Forest + ["StardewValley.Locations.Forest::netTravelingMerchantDay"] = "travelingMerchantDay", + ["StardewValley.Locations.Forest::netLog"] = "log", + + // FruitTree + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTileTree"] = "GreenHouseTileTree", + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTree"] = "GreenHouseTree", + + // GameLocation + ["StardewValley.GameLocation::isFarm"] = "IsFarm", + ["StardewValley.GameLocation::isOutdoors"] = "IsOutdoors", + ["StardewValley.GameLocation::lightLevel"] = "LightLevel", + ["StardewValley.GameLocation::name"] = "Name", + + // Item + ["StardewValley.Item::category"] = "Category", + ["StardewValley.Item::netName"] = "Name", + ["StardewValley.Item::parentSheetIndex"] = "ParentSheetIndex", + ["StardewValley.Item::specialVariable"] = "SpecialVariable", + + // Junimo + ["StardewValley.Characters.Junimo::eventActor"] = "EventActor", + + // LightSource + ["StardewValley.LightSource::identifier"] = "Identifier", + + // Monster + ["StardewValley.Monsters.Monster::damageToFarmer"] = "DamageToFarmer", + ["StardewValley.Monsters.Monster::experienceGained"] = "ExperienceGained", + ["StardewValley.Monsters.Monster::health"] = "Health", + ["StardewValley.Monsters.Monster::maxHealth"] = "MaxHealth", + ["StardewValley.Monsters.Monster::netFocusedOnFarmers"] = "focusedOnFarmers", + ["StardewValley.Monsters.Monster::netWildernessFarmMonster"] = "wildernessFarmMonster", + ["StardewValley.Monsters.Monster::slipperiness"] = "Slipperiness", + + // NPC + ["StardewValley.NPC::age"] = "Age", + ["StardewValley.NPC::birthday_Day"] = "Birthday_Day", + ["StardewValley.NPC::birthday_Season"] = "Birthday_Season", + ["StardewValley.NPC::breather"] = "Breather", + ["StardewValley.NPC::defaultMap"] = "DefaultMap", + ["StardewValley.NPC::gender"] = "Gender", + ["StardewValley.NPC::hideShadow"] = "HideShadow", + ["StardewValley.NPC::isInvisible"] = "IsInvisible", + ["StardewValley.NPC::isWalkingTowardPlayer"] = "IsWalkingTowardPlayer", + ["StardewValley.NPC::manners"] = "Manners", + ["StardewValley.NPC::optimism"] = "Optimism", + ["StardewValley.NPC::socialAnxiety"] = "SocialAnxiety", + + // Object + ["StardewValley.Object::canBeGrabbed"] = "CanBeGrabbed", + ["StardewValley.Object::canBeSetDown"] = "CanBeSetDown", + ["StardewValley.Object::edibility"] = "Edibility", + ["StardewValley.Object::flipped"] = "Flipped", + ["StardewValley.Object::fragility"] = "Fragility", + ["StardewValley.Object::hasBeenPickedUpByFarmer"] = "HasBeenPickedUpByFarmer", + ["StardewValley.Object::isHoedirt"] = "IsHoeDirt", + ["StardewValley.Object::isOn"] = "IsOn", + ["StardewValley.Object::isRecipe"] = "IsRecipe", + ["StardewValley.Object::isSpawnedObject"] = "IsSpawnedObject", + ["StardewValley.Object::minutesUntilReady"] = "MinutesUntilReady", + ["StardewValley.Object::netName"] = "name", + ["StardewValley.Object::price"] = "Price", + ["StardewValley.Object::quality"] = "Quality", + ["StardewValley.Object::scale"] = "Scale", + ["StardewValley.Object::stack"] = "Stack", + ["StardewValley.Object::tileLocation"] = "TileLocation", + ["StardewValley.Object::type"] = "Type", + + // Projectile + ["StardewValley.Projectiles.Projectile::ignoreLocationCollision"] = "IgnoreLocationCollision", + + // Tool + ["StardewValley.Tool::currentParentTileIndex"] = "CurrentParentTileIndex", + ["StardewValley.Tool::indexOfMenuItemView"] = "IndexOfMenuItemView", + ["StardewValley.Tool::initialParentTileIndex"] = "InitialParentTileIndex", + ["StardewValley.Tool::instantUse"] = "InstantUse", + ["StardewValley.Tool::netName"] = "BaseName", + ["StardewValley.Tool::stackable"] = "Stackable", + ["StardewValley.Tool::upgradeLevel"] = "UpgradeLevel" + }; + /// Describes the diagnostic rule covered by the analyzer. - 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" - ); + private readonly IDictionary Rules = new Dictionary + { + ["SMAPI001"] = 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" + ), + ["SMAPI002"] = new DiagnosticDescriptor( + id: "SMAPI002", + title: "Avoid Netcode types when possible", + messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + ) + }; /********* @@ -41,17 +161,24 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer ** Public methods *********/ /// Construct an instance. - public ImplicitNetFieldCastAnalyzer() + public NetFieldAnalyzer() { - this.SupportedDiagnostics = ImmutableArray.Create(ImplicitNetFieldCastAnalyzer.Rule); + this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); } /// Called once at session start to register actions in the analysis context. /// The analysis context. public override void Initialize(AnalysisContext context) { + // SMAPI002: avoid net fields if possible + context.RegisterSyntaxNodeAction( + this.AnalyzeAvoidableNetField, + SyntaxKind.SimpleMemberAccessExpression + ); + + // SMAPI001: avoid implicit net field conversion context.RegisterSyntaxNodeAction( - this.Analyse, + this.AnalyseNetFieldConversions, SyntaxKind.EqualsExpression, SyntaxKind.NotEqualsExpression, SyntaxKind.GreaterThanExpression, @@ -65,16 +192,44 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /********* ** Private methods *********/ - /// Analyse a syntax node and add a diagnostic message if applicable. + /// Analyse a syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available. + /// The analysis context. + private void AnalyzeAvoidableNetField(SyntaxNodeAnalysisContext context) + { + try + { + // check member type + MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; + TypeInfo memberType = context.SemanticModel.GetTypeInfo(node); + if (!this.IsNetType(memberType.Type)) + return; + + // get reference info + ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; + string propertyName = node.Name.Identifier.Text; + + // suggest replacement + if (this.NetFieldWrapperProperties.TryGetValue($"{declaringType}::{propertyName}", out string suggestedPropertyName)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + + /// Analyse a syntax node and add a diagnostic message if it implicitly converts a net field. /// The analysis context. - private void Analyse(SyntaxNodeAnalysisContext context) + private void AnalyseNetFieldConversions(SyntaxNodeAnalysisContext context) { try { BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; - bool leftHasWarning = this.Analyze(context, node.Left); + bool leftHasWarning = this.WarnIfOperandImplicitlyConvertsNetField(context, node.Left); if (!leftHasWarning) - this.Analyze(context, node.Right); + this.WarnIfOperandImplicitlyConvertsNetField(context, node.Right); } catch (Exception ex) { @@ -86,22 +241,25 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// The analysis context. /// The operand expression. /// Returns whether a diagnostic message was raised. - private bool Analyze(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) + private bool WarnIfOperandImplicitlyConvertsNetField(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) + if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) { string fromTypeName = operandType.Type.Name; string toTypeName = operandType.ConvertedType.Name; - context.ReportDiagnostic(Diagnostic.Create(ImplicitNetFieldCastAnalyzer.Rule, context.Node.GetLocation(), operand, fromTypeName, toTypeName)); + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, fromTypeName, toTypeName)); return true; } return false; } + + /// Get whether a type symbol references a Netcode type. + /// The type symbol. + private bool IsNetType(ITypeSymbol typeSymbol) + { + return typeSymbol?.ContainingNamespace?.Name == NetFieldAnalyzer.NetcodeNamespace; + } } } -- cgit From 416e1c3c1b884fb9b932968e72895babb6151d2f Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 22:34:11 -0400 Subject: rename file to match new scope (#471) --- .../ImplicitNetFieldCastAnalyzer.cs | 265 --------------------- .../NetFieldAnalyzer.cs | 265 +++++++++++++++++++++ 2 files changed, 265 insertions(+), 265 deletions(-) delete mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs deleted file mode 100644 index d64b1486..00000000 --- a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs +++ /dev/null @@ -1,265 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace StardewModdingAPI.ModBuildConfig.Analyzer -{ - /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class NetFieldAnalyzer : DiagnosticAnalyzer - { - /********* - ** Properties - *********/ - /// The namespace for Stardew Valley's Netcode types. - private const string NetcodeNamespace = "Netcode"; - - /// Maps net fields to their equivalent non-net properties where available. - private readonly IDictionary NetFieldWrapperProperties = new Dictionary - { - // Character - ["StardewValley.Character::currentLocationRef"] = "currentLocation", - ["StardewValley.Character::facingDirection"] = "FacingDirection", - ["StardewValley.Character::name"] = "Name", - ["StardewValley.Character::position"] = "Position", - ["StardewValley.Character::scale"] = "Scale", - ["StardewValley.Character::speed"] = "Speed", - ["StardewValley.Character::sprite"] = "Sprite", - - // Chest - ["StardewValley.Objects.Chest::tint"] = "Tint", - - // Farmer - ["StardewValley.Farmer::houseUpgradeLevel"] = "HouseUpgradeLevel", - ["StardewValley.Farmer::isMale"] = "IsMale", - ["StardewValley.Farmer::items"] = "Items", - ["StardewValley.Farmer::magneticRadius"] = "MagneticRadius", - ["StardewValley.Farmer::stamina"] = "Stamina", - ["StardewValley.Farmer::uniqueMultiplayerID"] = "UniqueMultiplayerID", - ["StardewValley.Farmer::usingTool"] = "UsingTool", - - // Forest - ["StardewValley.Locations.Forest::netTravelingMerchantDay"] = "travelingMerchantDay", - ["StardewValley.Locations.Forest::netLog"] = "log", - - // FruitTree - ["StardewValley.TerrainFeatures.FruitTree::greenHouseTileTree"] = "GreenHouseTileTree", - ["StardewValley.TerrainFeatures.FruitTree::greenHouseTree"] = "GreenHouseTree", - - // GameLocation - ["StardewValley.GameLocation::isFarm"] = "IsFarm", - ["StardewValley.GameLocation::isOutdoors"] = "IsOutdoors", - ["StardewValley.GameLocation::lightLevel"] = "LightLevel", - ["StardewValley.GameLocation::name"] = "Name", - - // Item - ["StardewValley.Item::category"] = "Category", - ["StardewValley.Item::netName"] = "Name", - ["StardewValley.Item::parentSheetIndex"] = "ParentSheetIndex", - ["StardewValley.Item::specialVariable"] = "SpecialVariable", - - // Junimo - ["StardewValley.Characters.Junimo::eventActor"] = "EventActor", - - // LightSource - ["StardewValley.LightSource::identifier"] = "Identifier", - - // Monster - ["StardewValley.Monsters.Monster::damageToFarmer"] = "DamageToFarmer", - ["StardewValley.Monsters.Monster::experienceGained"] = "ExperienceGained", - ["StardewValley.Monsters.Monster::health"] = "Health", - ["StardewValley.Monsters.Monster::maxHealth"] = "MaxHealth", - ["StardewValley.Monsters.Monster::netFocusedOnFarmers"] = "focusedOnFarmers", - ["StardewValley.Monsters.Monster::netWildernessFarmMonster"] = "wildernessFarmMonster", - ["StardewValley.Monsters.Monster::slipperiness"] = "Slipperiness", - - // NPC - ["StardewValley.NPC::age"] = "Age", - ["StardewValley.NPC::birthday_Day"] = "Birthday_Day", - ["StardewValley.NPC::birthday_Season"] = "Birthday_Season", - ["StardewValley.NPC::breather"] = "Breather", - ["StardewValley.NPC::defaultMap"] = "DefaultMap", - ["StardewValley.NPC::gender"] = "Gender", - ["StardewValley.NPC::hideShadow"] = "HideShadow", - ["StardewValley.NPC::isInvisible"] = "IsInvisible", - ["StardewValley.NPC::isWalkingTowardPlayer"] = "IsWalkingTowardPlayer", - ["StardewValley.NPC::manners"] = "Manners", - ["StardewValley.NPC::optimism"] = "Optimism", - ["StardewValley.NPC::socialAnxiety"] = "SocialAnxiety", - - // Object - ["StardewValley.Object::canBeGrabbed"] = "CanBeGrabbed", - ["StardewValley.Object::canBeSetDown"] = "CanBeSetDown", - ["StardewValley.Object::edibility"] = "Edibility", - ["StardewValley.Object::flipped"] = "Flipped", - ["StardewValley.Object::fragility"] = "Fragility", - ["StardewValley.Object::hasBeenPickedUpByFarmer"] = "HasBeenPickedUpByFarmer", - ["StardewValley.Object::isHoedirt"] = "IsHoeDirt", - ["StardewValley.Object::isOn"] = "IsOn", - ["StardewValley.Object::isRecipe"] = "IsRecipe", - ["StardewValley.Object::isSpawnedObject"] = "IsSpawnedObject", - ["StardewValley.Object::minutesUntilReady"] = "MinutesUntilReady", - ["StardewValley.Object::netName"] = "name", - ["StardewValley.Object::price"] = "Price", - ["StardewValley.Object::quality"] = "Quality", - ["StardewValley.Object::scale"] = "Scale", - ["StardewValley.Object::stack"] = "Stack", - ["StardewValley.Object::tileLocation"] = "TileLocation", - ["StardewValley.Object::type"] = "Type", - - // Projectile - ["StardewValley.Projectiles.Projectile::ignoreLocationCollision"] = "IgnoreLocationCollision", - - // Tool - ["StardewValley.Tool::currentParentTileIndex"] = "CurrentParentTileIndex", - ["StardewValley.Tool::indexOfMenuItemView"] = "IndexOfMenuItemView", - ["StardewValley.Tool::initialParentTileIndex"] = "InitialParentTileIndex", - ["StardewValley.Tool::instantUse"] = "InstantUse", - ["StardewValley.Tool::netName"] = "BaseName", - ["StardewValley.Tool::stackable"] = "Stackable", - ["StardewValley.Tool::upgradeLevel"] = "UpgradeLevel" - }; - - /// Describes the diagnostic rule covered by the analyzer. - private readonly IDictionary Rules = new Dictionary - { - ["SMAPI001"] = 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" - ), - ["SMAPI002"] = new DiagnosticDescriptor( - id: "SMAPI002", - title: "Avoid Netcode types when possible", - messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", - category: "SMAPI.CommonErrors", - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: "", - helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" - ) - }; - - - /********* - ** Accessors - *********/ - /// The descriptors for the diagnostics that this analyzer is capable of producing. - public override ImmutableArray SupportedDiagnostics { get; } - - - /********* - ** Public methods - *********/ - /// Construct an instance. - public NetFieldAnalyzer() - { - this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); - } - - /// Called once at session start to register actions in the analysis context. - /// The analysis context. - public override void Initialize(AnalysisContext context) - { - // SMAPI002: avoid net fields if possible - context.RegisterSyntaxNodeAction( - this.AnalyzeAvoidableNetField, - SyntaxKind.SimpleMemberAccessExpression - ); - - // SMAPI001: avoid implicit net field conversion - context.RegisterSyntaxNodeAction( - this.AnalyseNetFieldConversions, - SyntaxKind.EqualsExpression, - SyntaxKind.NotEqualsExpression, - SyntaxKind.GreaterThanExpression, - SyntaxKind.GreaterThanOrEqualExpression, - SyntaxKind.LessThanExpression, - SyntaxKind.LessThanOrEqualExpression - ); - } - - - /********* - ** Private methods - *********/ - /// Analyse a syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available. - /// The analysis context. - private void AnalyzeAvoidableNetField(SyntaxNodeAnalysisContext context) - { - try - { - // check member type - MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; - TypeInfo memberType = context.SemanticModel.GetTypeInfo(node); - if (!this.IsNetType(memberType.Type)) - return; - - // get reference info - ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; - string propertyName = node.Name.Identifier.Text; - - // suggest replacement - if (this.NetFieldWrapperProperties.TryGetValue($"{declaringType}::{propertyName}", out string suggestedPropertyName)) - { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); - } - } - catch (Exception ex) - { - throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); - } - } - - /// Analyse a syntax node and add a diagnostic message if it implicitly converts a net field. - /// The analysis context. - private void AnalyseNetFieldConversions(SyntaxNodeAnalysisContext context) - { - try - { - BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; - bool leftHasWarning = this.WarnIfOperandImplicitlyConvertsNetField(context, node.Left); - if (!leftHasWarning) - this.WarnIfOperandImplicitlyConvertsNetField(context, node.Right); - } - catch (Exception ex) - { - throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); - } - } - - /// Analyse one operand in a binary expression (like a and b in a == b) and add a diagnostic message if applicable. - /// The analysis context. - /// The operand expression. - /// Returns whether a diagnostic message was raised. - private bool WarnIfOperandImplicitlyConvertsNetField(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) - { - TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); - if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) - { - string fromTypeName = operandType.Type.Name; - string toTypeName = operandType.ConvertedType.Name; - context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, fromTypeName, toTypeName)); - return true; - } - - return false; - } - - /// Get whether a type symbol references a Netcode type. - /// The type symbol. - private bool IsNetType(ITypeSymbol typeSymbol) - { - return typeSymbol?.ContainingNamespace?.Name == NetFieldAnalyzer.NetcodeNamespace; - } - } -} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs new file mode 100644 index 00000000..d64b1486 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -0,0 +1,265 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace StardewModdingAPI.ModBuildConfig.Analyzer +{ + /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class NetFieldAnalyzer : DiagnosticAnalyzer + { + /********* + ** Properties + *********/ + /// The namespace for Stardew Valley's Netcode types. + private const string NetcodeNamespace = "Netcode"; + + /// Maps net fields to their equivalent non-net properties where available. + private readonly IDictionary NetFieldWrapperProperties = new Dictionary + { + // Character + ["StardewValley.Character::currentLocationRef"] = "currentLocation", + ["StardewValley.Character::facingDirection"] = "FacingDirection", + ["StardewValley.Character::name"] = "Name", + ["StardewValley.Character::position"] = "Position", + ["StardewValley.Character::scale"] = "Scale", + ["StardewValley.Character::speed"] = "Speed", + ["StardewValley.Character::sprite"] = "Sprite", + + // Chest + ["StardewValley.Objects.Chest::tint"] = "Tint", + + // Farmer + ["StardewValley.Farmer::houseUpgradeLevel"] = "HouseUpgradeLevel", + ["StardewValley.Farmer::isMale"] = "IsMale", + ["StardewValley.Farmer::items"] = "Items", + ["StardewValley.Farmer::magneticRadius"] = "MagneticRadius", + ["StardewValley.Farmer::stamina"] = "Stamina", + ["StardewValley.Farmer::uniqueMultiplayerID"] = "UniqueMultiplayerID", + ["StardewValley.Farmer::usingTool"] = "UsingTool", + + // Forest + ["StardewValley.Locations.Forest::netTravelingMerchantDay"] = "travelingMerchantDay", + ["StardewValley.Locations.Forest::netLog"] = "log", + + // FruitTree + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTileTree"] = "GreenHouseTileTree", + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTree"] = "GreenHouseTree", + + // GameLocation + ["StardewValley.GameLocation::isFarm"] = "IsFarm", + ["StardewValley.GameLocation::isOutdoors"] = "IsOutdoors", + ["StardewValley.GameLocation::lightLevel"] = "LightLevel", + ["StardewValley.GameLocation::name"] = "Name", + + // Item + ["StardewValley.Item::category"] = "Category", + ["StardewValley.Item::netName"] = "Name", + ["StardewValley.Item::parentSheetIndex"] = "ParentSheetIndex", + ["StardewValley.Item::specialVariable"] = "SpecialVariable", + + // Junimo + ["StardewValley.Characters.Junimo::eventActor"] = "EventActor", + + // LightSource + ["StardewValley.LightSource::identifier"] = "Identifier", + + // Monster + ["StardewValley.Monsters.Monster::damageToFarmer"] = "DamageToFarmer", + ["StardewValley.Monsters.Monster::experienceGained"] = "ExperienceGained", + ["StardewValley.Monsters.Monster::health"] = "Health", + ["StardewValley.Monsters.Monster::maxHealth"] = "MaxHealth", + ["StardewValley.Monsters.Monster::netFocusedOnFarmers"] = "focusedOnFarmers", + ["StardewValley.Monsters.Monster::netWildernessFarmMonster"] = "wildernessFarmMonster", + ["StardewValley.Monsters.Monster::slipperiness"] = "Slipperiness", + + // NPC + ["StardewValley.NPC::age"] = "Age", + ["StardewValley.NPC::birthday_Day"] = "Birthday_Day", + ["StardewValley.NPC::birthday_Season"] = "Birthday_Season", + ["StardewValley.NPC::breather"] = "Breather", + ["StardewValley.NPC::defaultMap"] = "DefaultMap", + ["StardewValley.NPC::gender"] = "Gender", + ["StardewValley.NPC::hideShadow"] = "HideShadow", + ["StardewValley.NPC::isInvisible"] = "IsInvisible", + ["StardewValley.NPC::isWalkingTowardPlayer"] = "IsWalkingTowardPlayer", + ["StardewValley.NPC::manners"] = "Manners", + ["StardewValley.NPC::optimism"] = "Optimism", + ["StardewValley.NPC::socialAnxiety"] = "SocialAnxiety", + + // Object + ["StardewValley.Object::canBeGrabbed"] = "CanBeGrabbed", + ["StardewValley.Object::canBeSetDown"] = "CanBeSetDown", + ["StardewValley.Object::edibility"] = "Edibility", + ["StardewValley.Object::flipped"] = "Flipped", + ["StardewValley.Object::fragility"] = "Fragility", + ["StardewValley.Object::hasBeenPickedUpByFarmer"] = "HasBeenPickedUpByFarmer", + ["StardewValley.Object::isHoedirt"] = "IsHoeDirt", + ["StardewValley.Object::isOn"] = "IsOn", + ["StardewValley.Object::isRecipe"] = "IsRecipe", + ["StardewValley.Object::isSpawnedObject"] = "IsSpawnedObject", + ["StardewValley.Object::minutesUntilReady"] = "MinutesUntilReady", + ["StardewValley.Object::netName"] = "name", + ["StardewValley.Object::price"] = "Price", + ["StardewValley.Object::quality"] = "Quality", + ["StardewValley.Object::scale"] = "Scale", + ["StardewValley.Object::stack"] = "Stack", + ["StardewValley.Object::tileLocation"] = "TileLocation", + ["StardewValley.Object::type"] = "Type", + + // Projectile + ["StardewValley.Projectiles.Projectile::ignoreLocationCollision"] = "IgnoreLocationCollision", + + // Tool + ["StardewValley.Tool::currentParentTileIndex"] = "CurrentParentTileIndex", + ["StardewValley.Tool::indexOfMenuItemView"] = "IndexOfMenuItemView", + ["StardewValley.Tool::initialParentTileIndex"] = "InitialParentTileIndex", + ["StardewValley.Tool::instantUse"] = "InstantUse", + ["StardewValley.Tool::netName"] = "BaseName", + ["StardewValley.Tool::stackable"] = "Stackable", + ["StardewValley.Tool::upgradeLevel"] = "UpgradeLevel" + }; + + /// Describes the diagnostic rule covered by the analyzer. + private readonly IDictionary Rules = new Dictionary + { + ["SMAPI001"] = 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" + ), + ["SMAPI002"] = new DiagnosticDescriptor( + id: "SMAPI002", + title: "Avoid Netcode types when possible", + messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + ) + }; + + + /********* + ** Accessors + *********/ + /// The descriptors for the diagnostics that this analyzer is capable of producing. + public override ImmutableArray SupportedDiagnostics { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + public NetFieldAnalyzer() + { + this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); + } + + /// Called once at session start to register actions in the analysis context. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + // SMAPI002: avoid net fields if possible + context.RegisterSyntaxNodeAction( + this.AnalyzeAvoidableNetField, + SyntaxKind.SimpleMemberAccessExpression + ); + + // SMAPI001: avoid implicit net field conversion + context.RegisterSyntaxNodeAction( + this.AnalyseNetFieldConversions, + SyntaxKind.EqualsExpression, + SyntaxKind.NotEqualsExpression, + SyntaxKind.GreaterThanExpression, + SyntaxKind.GreaterThanOrEqualExpression, + SyntaxKind.LessThanExpression, + SyntaxKind.LessThanOrEqualExpression + ); + } + + + /********* + ** Private methods + *********/ + /// Analyse a syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available. + /// The analysis context. + private void AnalyzeAvoidableNetField(SyntaxNodeAnalysisContext context) + { + try + { + // check member type + MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; + TypeInfo memberType = context.SemanticModel.GetTypeInfo(node); + if (!this.IsNetType(memberType.Type)) + return; + + // get reference info + ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; + string propertyName = node.Name.Identifier.Text; + + // suggest replacement + if (this.NetFieldWrapperProperties.TryGetValue($"{declaringType}::{propertyName}", out string suggestedPropertyName)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + + /// Analyse a syntax node and add a diagnostic message if it implicitly converts a net field. + /// The analysis context. + private void AnalyseNetFieldConversions(SyntaxNodeAnalysisContext context) + { + try + { + BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; + bool leftHasWarning = this.WarnIfOperandImplicitlyConvertsNetField(context, node.Left); + if (!leftHasWarning) + this.WarnIfOperandImplicitlyConvertsNetField(context, node.Right); + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + + /// Analyse one operand in a binary expression (like a and b in a == b) and add a diagnostic message if applicable. + /// The analysis context. + /// The operand expression. + /// Returns whether a diagnostic message was raised. + private bool WarnIfOperandImplicitlyConvertsNetField(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) + { + TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); + if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) + { + string fromTypeName = operandType.Type.Name; + string toTypeName = operandType.ConvertedType.Name; + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, fromTypeName, toTypeName)); + return true; + } + + return false; + } + + /// Get whether a type symbol references a Netcode type. + /// The type symbol. + private bool IsNetType(ITypeSymbol typeSymbol) + { + return typeSymbol?.ContainingNamespace?.Name == NetFieldAnalyzer.NetcodeNamespace; + } + } +} -- cgit From c8db771e11a0e5224aa3b0766134afc8e733896e Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 23:25:10 -0400 Subject: tweak message output and unit tests (#471) --- src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index d64b1486..a9987733 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -246,9 +246,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) { - string fromTypeName = operandType.Type.Name; - string toTypeName = operandType.ConvertedType.Name; - context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, fromTypeName, toTypeName)); + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, operandType.Type.Name, operandType.ConvertedType)); return true; } -- cgit From 971ed1a17559064ee0ee42a83e786b3e3076059f Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 23:43:13 -0400 Subject: fix net field replacements not reported for a subclass reference (#471) --- src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index a9987733..2cb1ac4c 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -209,9 +209,13 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer string propertyName = node.Name.Identifier.Text; // suggest replacement - if (this.NetFieldWrapperProperties.TryGetValue($"{declaringType}::{propertyName}", out string suggestedPropertyName)) + for (ITypeSymbol type = declaringType; type != null; type = type.BaseType) { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); + if (this.NetFieldWrapperProperties.TryGetValue($"{type}::{propertyName}", out string suggestedPropertyName)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); + break; + } } } catch (Exception ex) -- cgit From 1fb625dc42a7cf5fdf74329454bc3ecde806ae10 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:08 -0400 Subject: fix some net field comparisons to null not flagged (#471) --- .../NetFieldAnalyzer.cs | 46 ++++++++++++---------- 1 file changed, 26 insertions(+), 20 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index 2cb1ac4c..ae646fd8 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -230,10 +230,32 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer { try { - BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; - bool leftHasWarning = this.WarnIfOperandImplicitlyConvertsNetField(context, node.Left); - if (!leftHasWarning) - this.WarnIfOperandImplicitlyConvertsNetField(context, node.Right); + BinaryExpressionSyntax binaryExpression = (BinaryExpressionSyntax)context.Node; + foreach (var pair in new[] { Tuple.Create(binaryExpression.Left, binaryExpression.Right), Tuple.Create(binaryExpression.Right, binaryExpression.Left) }) + { + // get node info + ExpressionSyntax curExpression = pair.Item1; // the side of the comparison being examined + ExpressionSyntax otherExpression = pair.Item2; // the other side + TypeInfo typeInfo = context.SemanticModel.GetTypeInfo(curExpression); + if (!this.IsNetType(typeInfo.Type)) + continue; + + // warn for implicit conversion + if (!this.IsNetType(typeInfo.ConvertedType)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), curExpression, typeInfo.Type.Name, typeInfo.ConvertedType)); + break; + } + + // warn for comparison to null + // An expression like `building.indoors != null` will sometimes convert `building.indoors` to NetFieldBase instead of object before comparison. Haven't reproduced this in unit tests yet. + Optional otherValue = context.SemanticModel.GetConstantValue(otherExpression); + if (otherValue.HasValue && otherValue.Value == null) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), curExpression, typeInfo.Type.Name, "null")); + break; + } + } } catch (Exception ex) { @@ -241,22 +263,6 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer } } - /// Analyse one operand in a binary expression (like a and b in a == b) and add a diagnostic message if applicable. - /// The analysis context. - /// The operand expression. - /// Returns whether a diagnostic message was raised. - private bool WarnIfOperandImplicitlyConvertsNetField(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) - { - TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); - if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) - { - context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, operandType.Type.Name, operandType.ConvertedType)); - return true; - } - - return false; - } - /// Get whether a type symbol references a Netcode type. /// The type symbol. private bool IsNetType(ITypeSymbol typeSymbol) -- cgit From c6c2302baf830a441b269ef7402068bd9dece22e Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:39 -0400 Subject: tweak analyzer code & documentation (#471) --- src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'src/SMAPI.ModBuildConfig.Analyzer') diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index ae646fd8..915a50e8 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -130,22 +130,20 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer ["SMAPI001"] = 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.", + 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" + helpLinkUri: "https://smapi.io/buildmsg/smapi001" ), ["SMAPI002"] = new DiagnosticDescriptor( id: "SMAPI002", title: "Avoid Netcode types when possible", - messageFormat: "'{0}' is a {1} field; consider using the {2} property instead.