Add Analyzer to require checking or discarding `TryGetValue`

I'm pretty sure all the existing instances were intended to use `default(T)` if
the key isn't present, except maybe the cheatcode converters but I don't care to
rewrite those at the moment
This commit is contained in:
YoshiRulz 2023-05-03 04:28:56 +10:00
parent 384a4e1e18
commit 4a752f66b7
No known key found for this signature in database
GPG Key ID: C4DE31C245353FB7
19 changed files with 122 additions and 51 deletions
Common.ruleset
ExternalProjects
References
src
BizHawk.Bizware.BizwareGL
BizHawk.Bizware.DirectX
BizHawk.Client.Common
BizHawk.Client.EmuHawk
BizHawk.Emulation.Common/Database
BizHawk.Emulation.Cores

View File

@ -31,6 +31,9 @@
<!-- Don't call typeof(T).ToString(), use nameof operator or typeof(T).FullName -->
<Rule Id="BHI1103" Action="Error" />
<!-- Check result of IDictionary.TryGetValue, or discard it if default(T) is desired -->
<Rule Id="BHI1200" Action="Error" />
<!-- Call to FirstOrDefault when elements are of a value type; FirstOrNull may have been intended -->
<Rule Id="BHI3100" Action="Error" />

View File

@ -26,4 +26,16 @@ public static class RoslynUtils
public static bool Matches(this ISymbol expected, ISymbol? actual)
=> SymbolEqualityComparer.Default.Equals(expected, actual);
#if false // easier to just `.OriginalDefinition` always
public static bool Matches(this INamedTypeSymbol expected, INamedTypeSymbol? actual, bool degenericise)
{
if (degenericise)
{
if (expected.IsGenericType && !expected.IsUnboundGenericType) return expected.OriginalDefinition.Matches(actual, degenericise: true);
if (actual is not null && actual.IsGenericType && !actual.IsUnboundGenericType) return expected.Matches(actual.OriginalDefinition, degenericise: true);
}
return expected.Matches(actual as ISymbol);
}
#endif
}

View File

@ -0,0 +1,56 @@
namespace BizHawk.Analyzers;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class TryGetValueImplicitDiscardAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor DiagUncheckedTryGetValue = new(
id: "BHI1200",
title: "Check result of IDictionary.TryGetValue, or discard it if default(T) is desired",
messageFormat: "Assign the result of this TryGetValue call to a variable or discard",
category: "Usage",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagUncheckedTryGetValue);
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(static initContext =>
{
const string STR_TGV = "TryGetValue";
var rwDictSym = initContext.Compilation.GetTypeByMetadataName("System.Collections.Generic.IDictionary`2")!;
var roDictSym = initContext.Compilation.GetTypeByMetadataName("System.Collections.Generic.IReadOnlyDictionary`2")!;
bool IsBCLTryGetValue(IMethodSymbol calledSym)
=> calledSym.ContainingType.AllInterfaces.Any(intfSym =>
{
var degenericisedSym = intfSym.OriginalDefinition;
return rwDictSym.Matches(degenericisedSym) || roDictSym.Matches(degenericisedSym);
});
initContext.RegisterOperationAction(
oac =>
{
var operation = (IInvocationOperation) oac.Operation;
if (operation.Parent?.Kind is not OperationKind.ExpressionStatement) return;
var calledSym = operation.TargetMethod.ConstructedFrom;
if (calledSym.Name is STR_TGV) oac.ReportDiagnostic(Diagnostic.Create(
DiagUncheckedTryGetValue,
operation.Syntax.GetLocation(),
IsBCLTryGetValue(calledSym) ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning,
additionalLocations: null,
properties: null,
messageArgs: null));
},
OperationKind.Invocation);
});
}
}

Binary file not shown.

View File

@ -66,7 +66,7 @@ namespace BizHawk.Bizware.BizwareGL
public PipelineUniform TryGetUniform(string name)
{
Uniforms.TryGetValue(name, out var ret);
_ = Uniforms.TryGetValue(name, out var ret);
return ret;
}

View File

@ -54,8 +54,8 @@ namespace BizHawk.Bizware.DirectX
continue;
for (int b = 0, n = pad.NumButtons; b < n; b++) handleButton(pad.InputNamePrefix + pad.ButtonName(b), pad.Pressed(b), ClientInputFocus.Pad);
foreach (var (axisName, f) in pad.GetAxes()) handleAxis(pad.InputNamePrefix + axisName, (int) f);
_lastHapticsSnapshot.TryGetValue(pad.InputNamePrefix + "Left", out var leftStrength);
_lastHapticsSnapshot.TryGetValue(pad.InputNamePrefix + "Right", out var rightStrength);
_ = _lastHapticsSnapshot.TryGetValue(pad.InputNamePrefix + "Left", out var leftStrength);
_ = _lastHapticsSnapshot.TryGetValue(pad.InputNamePrefix + "Right", out var rightStrength);
pad.SetVibration(leftStrength, rightStrength); // values will be 0 if not found
}
foreach (var pad in GamePad.EnumerateDevices())

View File

@ -47,7 +47,7 @@ namespace BizHawk.Client.Common.FilterManager
{
get
{
_filterNameIndex.TryGetValue(name, out var ret);
_ = _filterNameIndex.TryGetValue(name, out var ret);
return ret;
}
}

View File

@ -379,7 +379,7 @@ namespace BizHawk.Client.Common
}
else
{
_config.PreferredCores.TryGetValue(cip.Game.System, out var preferredCore);
_ = _config.PreferredCores.TryGetValue(cip.Game.System, out var preferredCore);
var dbForcedCoreName = cip.Game.ForcedCore;
cores = CoreInventory.Instance.GetCores(cip.Game.System)
.OrderBy(c =>

View File

@ -52,20 +52,20 @@ namespace BizHawk.Client.Common.cheats
// Getting Value
if (_code.Length > 0)
{
_gbGgGameGenieTable.TryGetValue(_code[0], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[0], out x);
result.Value = x << 4;
}
if (_code.Length > 1)
{
_gbGgGameGenieTable.TryGetValue(_code[1], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[1], out x);
result.Value |= x;
}
// Address
if (_code.Length > 2)
{
_gbGgGameGenieTable.TryGetValue(_code[2], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[2], out x);
result.Value = x << 8;
}
else
@ -75,30 +75,30 @@ namespace BizHawk.Client.Common.cheats
if (_code.Length > 3)
{
_gbGgGameGenieTable.TryGetValue(_code[3], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[3], out x);
result.Address |= x << 4;
}
if (_code.Length > 4)
{
_gbGgGameGenieTable.TryGetValue(_code[4], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[4], out x);
result.Address |= x;
}
if (_code.Length > 5)
{
_gbGgGameGenieTable.TryGetValue(_code[5], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[5], out x);
result.Address |= (x ^ 0xF) << 12;
}
// compare need to be full
if (_code.Length > 8)
{
_gbGgGameGenieTable.TryGetValue(_code[6], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[6], out x);
var comp = x << 2;
// 8th character ignored
_gbGgGameGenieTable.TryGetValue(_code[8], out x);
_ = _gbGgGameGenieTable.TryGetValue(_code[8], out x);
comp |= (x & 0xC) >> 2;
comp |= (x & 0x3) << 6;
result.Compare = comp ^ 0xBA;

View File

@ -65,7 +65,7 @@ namespace BizHawk.Client.Common.cheats
foreach (var t in code)
{
hexCode <<= 5;
GameGenieTable.TryGetValue(t, out var y);
_ = GameGenieTable.TryGetValue(t, out var y);
hexCode |= y;
}

View File

@ -47,26 +47,26 @@ namespace BizHawk.Client.Common.cheats
result.Value = 0;
result.Address = 0x8000;
GameGenieTable.TryGetValue(code[0], out var x);
_ = GameGenieTable.TryGetValue(code[0], out var x);
result.Value |= x & 0x07;
result.Value |= (x & 0x08) << 4;
GameGenieTable.TryGetValue(code[1], out x);
_ = GameGenieTable.TryGetValue(code[1], out x);
result.Value |= (x & 0x07) << 4;
result.Address |= (x & 0x08) << 4;
GameGenieTable.TryGetValue(code[2], out x);
_ = GameGenieTable.TryGetValue(code[2], out x);
result.Address |= (x & 0x07) << 4;
GameGenieTable.TryGetValue(code[3], out x);
_ = GameGenieTable.TryGetValue(code[3], out x);
result.Address |= (x & 0x07) << 12;
result.Address |= x & 0x08;
GameGenieTable.TryGetValue(code[4], out x);
_ = GameGenieTable.TryGetValue(code[4], out x);
result.Address |= x & 0x07;
result.Address |= (x & 0x08) << 8;
GameGenieTable.TryGetValue(code[5], out x);
_ = GameGenieTable.TryGetValue(code[5], out x);
result.Address |= (x & 0x07) << 8;
result.Value |= x & 0x08;
}
@ -79,34 +79,34 @@ namespace BizHawk.Client.Common.cheats
result.Address = 0x8000;
result.Compare = 0;
GameGenieTable.TryGetValue(code[0], out var x);
_ = GameGenieTable.TryGetValue(code[0], out var x);
result.Value |= x & 0x07;
result.Value |= (x & 0x08) << 4;
GameGenieTable.TryGetValue(code[1], out x);
_ = GameGenieTable.TryGetValue(code[1], out x);
result.Value |= (x & 0x07) << 4;
result.Address |= (x & 0x08) << 4;
GameGenieTable.TryGetValue(code[2], out x);
_ = GameGenieTable.TryGetValue(code[2], out x);
result.Address |= (x & 0x07) << 4;
GameGenieTable.TryGetValue(code[3], out x);
_ = GameGenieTable.TryGetValue(code[3], out x);
result.Address |= (x & 0x07) << 12;
result.Address |= x & 0x08;
GameGenieTable.TryGetValue(code[4], out x);
_ = GameGenieTable.TryGetValue(code[4], out x);
result.Address |= x & 0x07;
result.Address |= (x & 0x08) << 8;
GameGenieTable.TryGetValue(code[5], out x);
_ = GameGenieTable.TryGetValue(code[5], out x);
result.Address |= (x & 0x07) << 8;
result.Compare |= x & 0x08;
GameGenieTable.TryGetValue(code[6], out x);
_ = GameGenieTable.TryGetValue(code[6], out x);
result.Compare |= x & 0x07;
result.Compare |= (x & 0x08) << 4;
GameGenieTable.TryGetValue(code[7], out x);
_ = GameGenieTable.TryGetValue(code[7], out x);
result.Compare |= (x & 0x07) << 4;
result.Value |= x & 0x08;
}

View File

@ -55,53 +55,53 @@ namespace BizHawk.Client.Common.cheats
// Value
if (code.Length > 0)
{
SNESGameGenieTable.TryGetValue(code[0], out x);
_ = SNESGameGenieTable.TryGetValue(code[0], out x);
result.Value = x << 4;
}
if (code.Length > 1)
{
SNESGameGenieTable.TryGetValue(code[1], out x);
_ = SNESGameGenieTable.TryGetValue(code[1], out x);
result.Value |= x;
}
// Address
if (code.Length > 2)
{
SNESGameGenieTable.TryGetValue(code[2], out x);
_ = SNESGameGenieTable.TryGetValue(code[2], out x);
result.Address = x << 12;
}
if (code.Length > 3)
{
SNESGameGenieTable.TryGetValue(code[3], out x);
_ = SNESGameGenieTable.TryGetValue(code[3], out x);
result.Address |= x << 4;
}
if (code.Length > 4)
{
SNESGameGenieTable.TryGetValue(code[4], out x);
_ = SNESGameGenieTable.TryGetValue(code[4], out x);
result.Address |= (x & 0xC) << 6;
result.Address |= (x & 0x3) << 22;
}
if (code.Length > 5)
{
SNESGameGenieTable.TryGetValue(code[5], out x);
_ = SNESGameGenieTable.TryGetValue(code[5], out x);
result.Address |= (x & 0xC) << 18;
result.Address |= (x & 0x3) << 2;
}
if (code.Length > 6)
{
SNESGameGenieTable.TryGetValue(code[6], out x);
_ = SNESGameGenieTable.TryGetValue(code[6], out x);
result.Address |= (x & 0xC) >> 2;
result.Address |= (x & 0x3) << 18;
}
if (code.Length > 7)
{
SNESGameGenieTable.TryGetValue(code[7], out x);
_ = SNESGameGenieTable.TryGetValue(code[7], out x);
result.Address |= (x & 0xC) << 14;
result.Address |= (x & 0x3) << 10;
}

View File

@ -45,7 +45,7 @@ namespace BizHawk.Client.Common
/// <returns>null if no settings were saved, or there was an error deserializing</returns>
public static object GetCoreSettings(this Config config, Type coreType, Type settingsType)
{
config.CoreSettings.TryGetValue(coreType.ToString(), out var j);
_ = config.CoreSettings.TryGetValue(coreType.ToString(), out var j);
return Deserialize(j, settingsType);
}
@ -85,7 +85,7 @@ namespace BizHawk.Client.Common
/// <returns>null if no settings were saved, or there was an error deserializing</returns>
public static object GetCoreSyncSettings(this Config config, Type coreType, Type syncSettingsType)
{
config.CoreSyncSettings.TryGetValue(coreType.ToString(), out var j);
_ = config.CoreSyncSettings.TryGetValue(coreType.ToString(), out var j);
return Deserialize(j, syncSettingsType);
}

View File

@ -81,9 +81,9 @@ namespace BizHawk.Client.EmuHawk
entry.Click += ClickHandler;
return (ToolStripItem) entry;
}).ToArray());
submenu.DropDownOpened += (openedSender, _) =>
submenu.DropDownOpened += (openedSender, _1) =>
{
Config.PreferredCores.TryGetValue(groupLabel, out var preferred);
_ = Config.PreferredCores.TryGetValue(groupLabel, out var preferred);
foreach (ToolStripMenuItem entry in ((ToolStripMenuItem) openedSender).DropDownItems) entry.Checked = entry.Text == preferred;
};
CoresSubMenu.DropDownItems.Add(submenu);

View File

@ -267,7 +267,7 @@ namespace BizHawk.Client.EmuHawk
private void SetControllerPicture(string controlName)
{
ControllerImages.TryGetValue(controlName, out var lazyBmp);
_ = ControllerImages.TryGetValue(controlName, out var lazyBmp);
if (lazyBmp != null)
{
var bmp = lazyBmp.Value;

View File

@ -194,7 +194,7 @@ namespace BizHawk.Emulation.Common
_acquire.WaitOne();
var hashFormatted = FormatHash(hash);
DB.TryGetValue(hashFormatted, out var cgi);
_ = DB.TryGetValue(hashFormatted, out var cgi);
if (cgi == null)
{
Console.WriteLine($"DB: hash {hash} not in game database.");

View File

@ -29,7 +29,7 @@ namespace BizHawk.Emulation.Cores.Nintendo.SNES
public byte* QUERY_get_memory_data(SNES_MEMORY id)
{
string name = QUERY_MemoryNameForId(id);
_sharedMemoryBlocks.TryGetValue(name, out var ret);
_ = _sharedMemoryBlocks.TryGetValue(name, out var ret);
return (byte*)ret;
}

View File

@ -151,7 +151,7 @@ namespace BizHawk.Emulation.Cores
public IEnumerable<Core> GetCores(string system)
{
_systems.TryGetValue(system, out var cores);
_ = _systems.TryGetValue(system, out var cores);
return cores ?? Enumerable.Empty<Core>();
}

View File

@ -113,8 +113,8 @@ namespace BizHawk.Emulation.Cores.Waterbox
var possible = info.AllOverrides.Where(kvp => kvp.Value.NonSync && kvp.Value.NoRestart).Select(kvp => kvp.Key);
return possible.Where(key =>
{
x.MednafenValues.TryGetValue(key, out var xx);
y.MednafenValues.TryGetValue(key, out var yy);
_ = x.MednafenValues.TryGetValue(key, out var xx);
_ = y.MednafenValues.TryGetValue(key, out var yy);
return xx != yy;
});
}
@ -124,8 +124,8 @@ namespace BizHawk.Emulation.Cores.Waterbox
var restarters = info.AllOverrides.Where(kvp => kvp.Value.NonSync && !kvp.Value.NoRestart).Select(kvp => kvp.Key);
foreach (var key in restarters)
{
x.MednafenValues.TryGetValue(key, out var xx);
y.MednafenValues.TryGetValue(key, out var yy);
_ = x.MednafenValues.TryGetValue(key, out var xx);
_ = y.MednafenValues.TryGetValue(key, out var yy);
if (xx != yy)
return PutSettingsDirtyBits.RebootCore;
}
@ -186,8 +186,8 @@ namespace BizHawk.Emulation.Cores.Waterbox
var restarters = info.AllOverrides.Where(kvp => !kvp.Value.NonSync && !kvp.Value.NoRestart).Select(kvp => kvp.Key);
foreach (var key in restarters)
{
x.MednafenValues.TryGetValue(key, out var xx);
y.MednafenValues.TryGetValue(key, out var yy);
_ = x.MednafenValues.TryGetValue(key, out var xx);
_ = y.MednafenValues.TryGetValue(key, out var yy);
if (xx != yy)
return PutSettingsDirtyBits.RebootCore;
}
@ -206,7 +206,7 @@ namespace BizHawk.Emulation.Cores.Waterbox
{
// try to get actual value from settings
var dict = ovr.NonSync ? _settings.MednafenValues : _syncSettingsActual.MednafenValues;
dict.TryGetValue(name, out val);
_ = dict.TryGetValue(name, out val);
}
if (val == null)
{