diff --git a/.global.editorconfig.ini b/.global.editorconfig.ini index 711d075af8..0913fddc13 100644 --- a/.global.editorconfig.ini +++ b/.global.editorconfig.ini @@ -29,6 +29,8 @@ dotnet_diagnostic.BHI1102.severity = error dotnet_diagnostic.BHI1103.severity = error # Don't use ^= (XOR-assign) for inverting the value of booleans dotnet_diagnostic.BHI1104.severity = error +# Use unambiguous decimal<=>float/double conversion methods +dotnet_diagnostic.BHI1105.severity = error # Brackets of collection expression should be separated with spaces dotnet_diagnostic.BHI1110.severity = warning # Expression-bodied member should be flowed to next line correctly diff --git a/ExternalProjects/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzer.cs b/ExternalProjects/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzer.cs new file mode 100644 index 0000000000..085e740570 --- /dev/null +++ b/ExternalProjects/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzer.cs @@ -0,0 +1,80 @@ +namespace BizHawk.Analyzers; + +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class AmbiguousMoneyToFloatConversionAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor DiagAmbiguousMoneyToFloatConversion = new( + id: "BHI1105", + title: "Use unambiguous decimal<=>float/double conversion methods", + messageFormat: "use {0} for checked conversion, or {1} for unchecked", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(DiagAmbiguousMoneyToFloatConversion); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(initContext => + { + var decimalSym = initContext.Compilation.GetTypeByMetadataName("System.Decimal")!; + var doubleSym = initContext.Compilation.GetTypeByMetadataName("System.Double")!; + var floatSym = initContext.Compilation.GetTypeByMetadataName("System.Single")!; + initContext.RegisterOperationAction(oac => + { + var conversionOp = (IConversionOperation) oac.Operation; + var typeOutput = conversionOp.Type; + var typeInput = conversionOp.Operand.Type; + bool isToDecimal; + bool isDoublePrecision; + if (decimalSym.Matches(typeOutput)) + { + if (doubleSym.Matches(typeInput)) isDoublePrecision = true; + else if (floatSym.Matches(typeInput)) isDoublePrecision = false; + else return; + isToDecimal = true; + } + else if (decimalSym.Matches(typeInput)) + { + if (doubleSym.Matches(typeOutput)) isDoublePrecision = true; + else if (floatSym.Matches(typeOutput)) isDoublePrecision = false; + else return; + isToDecimal = false; + } + else + { + return; + } + var conversionSyn = conversionOp.Syntax; + //TODO check the suggested methods are accessible (i.e. BizHawk.Common is referenced) + oac.ReportDiagnostic(Diagnostic.Create( + DiagAmbiguousMoneyToFloatConversion, + (conversionSyn.Parent?.Kind() is SyntaxKind.CheckedExpression or SyntaxKind.UncheckedExpression + ? conversionSyn.Parent + : conversionSyn).GetLocation(), + conversionOp.IsChecked ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning, + additionalLocations: null, + properties: null, + messageArgs: isToDecimal + ? [ + $"new decimal({(isDoublePrecision ? "double" : "float")})", // "checked" + "static NumberExtensions.ConvertToMoneyTruncated", // "unchecked" + ] + : [ + $"decimal.{(isDoublePrecision ? "ConvertToF64" : "ConvertToF32")} ext. (from NumberExtensions)", // "checked" + $"static Decimal.{(isDoublePrecision ? "ToDouble" : "ToSingle")}", // "unchecked" + ])); + }, + OperationKind.Conversion); + }); + } +} diff --git a/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzerTests.cs b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzerTests.cs new file mode 100644 index 0000000000..b6b91d8b44 --- /dev/null +++ b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/AmbiguousMoneyToFloatConversionAnalyzerTests.cs @@ -0,0 +1,32 @@ +namespace BizHawk.Tests.Analyzers; + +using System.Threading.Tasks; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + BizHawk.Analyzers.AmbiguousMoneyToFloatConversionAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +[TestClass] +public sealed class AmbiguousMoneyToFloatConversionAnalyzerTests +{ + [TestMethod] + public Task CheckMisuseOfDecimalExplicitCastOperators() + => Verify.VerifyAnalyzerAsync(""" + public static class Cases { + private static float Y(decimal m) + => decimal.ToSingle(m); + private static decimal Z(double d) + => new(d); + private static float A(decimal m) + => {|BHI1105:unchecked((float) m)|}; + private static decimal B(double d) + => {|BHI1105:checked((decimal) d)|}; + private static decimal C(float d) + => {|BHI1105:unchecked((decimal) d)|}; + private static double D(decimal m) + => {|BHI1105:checked((double) m)|}; + } + """); +} diff --git a/ExternalProjects/LibCommon.props b/ExternalProjects/LibCommon.props index a97edb4583..621e2ff051 100644 --- a/ExternalProjects/LibCommon.props +++ b/ExternalProjects/LibCommon.props @@ -1,7 +1,7 @@ - $(NoWarn);MEN018;SA1200 + $(NoWarn);BHI1105;MEN018;SA1200 diff --git a/References/BizHawk.Analyzer.dll b/References/BizHawk.Analyzer.dll index 39d8a83466..bb26800e98 100644 Binary files a/References/BizHawk.Analyzer.dll and b/References/BizHawk.Analyzer.dll differ diff --git a/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs b/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs index 404794d389..6e2efa5086 100644 --- a/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs +++ b/src/BizHawk.Client.Common/movie/BasicMovieInfo.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using BizHawk.Common.NumberExtensions; using BizHawk.Common.StringExtensions; using BizHawk.Emulation.Common; @@ -71,7 +72,7 @@ namespace BizHawk.Client.Common const decimal attosInSec = 1_000_000_000_000_000_000.0M; var m = attosInSec; m /= ulong.Parse(vsyncAttoStr); - return checked((double) m); + return m.ConvertToF64(); } return PlatformFrameRates.GetFrameRate(SystemID, IsPal); diff --git a/src/BizHawk.Client.EmuHawk/config/RewindConfig.cs b/src/BizHawk.Client.EmuHawk/config/RewindConfig.cs index 003df64f52..2667da4b69 100755 --- a/src/BizHawk.Client.EmuHawk/config/RewindConfig.cs +++ b/src/BizHawk.Client.EmuHawk/config/RewindConfig.cs @@ -1,3 +1,4 @@ +using System.Numerics; using System.Windows.Forms; using BizHawk.Client.Common; @@ -57,7 +58,12 @@ namespace BizHawk.Client.EmuHawk RewindEnabledBox.Checked = _config.Rewind.Enabled; UseCompression.Checked = _config.Rewind.UseCompression; cbDeltaCompression.Checked = _config.Rewind.UseDelta; - BufferSizeUpDown.Value = Math.Max((decimal) Math.Log(_config.Rewind.BufferSize, 2), BufferSizeUpDown.Minimum); + BufferSizeUpDown.Value = Math.Max( + BufferSizeUpDown.Minimum, + _config.Rewind.BufferSize < 0L + ? 0.0M + : new decimal(BitOperations.Log2(unchecked((ulong) _config.Rewind.BufferSize))) + ); TargetFrameLengthRadioButton.Checked = !_config.Rewind.UseFixedRewindInterval; TargetRewindIntervalRadioButton.Checked = _config.Rewind.UseFixedRewindInterval; TargetFrameLengthNumeric.Value = Math.Max(_config.Rewind.TargetFrameLength, TargetFrameLengthNumeric.Minimum); diff --git a/src/BizHawk.Client.EmuHawk/tools/BasicBot/BotControlsRow.cs b/src/BizHawk.Client.EmuHawk/tools/BasicBot/BotControlsRow.cs index 3766dc01cf..d4217c7d16 100644 --- a/src/BizHawk.Client.EmuHawk/tools/BasicBot/BotControlsRow.cs +++ b/src/BizHawk.Client.EmuHawk/tools/BasicBot/BotControlsRow.cs @@ -1,5 +1,7 @@ using System.Windows.Forms; +using BizHawk.Common.NumberExtensions; + namespace BizHawk.Client.EmuHawk { public partial class BotControlsRow : UserControl @@ -21,8 +23,8 @@ namespace BizHawk.Client.EmuHawk public double Probability { - get => (double)ProbabilityUpDown.Value; - set => ProbabilityUpDown.Value = (decimal)value; + get => ProbabilityUpDown.Value.ConvertToF64(); + set => ProbabilityUpDown.Value = new(value); } private void ProbabilityUpDown_ValueChanged(object sender, EventArgs e) diff --git a/src/BizHawk.Client.EmuHawk/tools/VirtualPads/controls/VirtualPadTargetScreen.cs b/src/BizHawk.Client.EmuHawk/tools/VirtualPads/controls/VirtualPadTargetScreen.cs index 26130f31f4..a53a01f6e5 100644 --- a/src/BizHawk.Client.EmuHawk/tools/VirtualPads/controls/VirtualPadTargetScreen.cs +++ b/src/BizHawk.Client.EmuHawk/tools/VirtualPads/controls/VirtualPadTargetScreen.cs @@ -1,6 +1,7 @@ using System.Drawing; using System.Windows.Forms; using BizHawk.Client.Common; +using BizHawk.Common.NumberExtensions; using BizHawk.Emulation.Common; namespace BizHawk.Client.EmuHawk @@ -195,7 +196,7 @@ namespace BizHawk.Client.EmuHawk XNumeric.Value = XNumeric.Maximum; } - _stickyXorAdapter.SetAxis(XName, (int)((float)XNumeric.Value * MultiplierX)); + _stickyXorAdapter.SetAxis(XName, (XNumeric.Value.ConvertToF32() * MultiplierX).RoundToInt()); _isSet = true; } } @@ -217,7 +218,7 @@ namespace BizHawk.Client.EmuHawk YNumeric.Value = YNumeric.Maximum; } - _stickyXorAdapter.SetAxis(YName, (int)((float)YNumeric.Value * MultiplierY)); + _stickyXorAdapter.SetAxis(YName, (YNumeric.Value.ConvertToF32() * MultiplierY).RoundToInt()); _isSet = true; } } diff --git a/src/BizHawk.Common/Extensions/NumberExtensions.cs b/src/BizHawk.Common/Extensions/NumberExtensions.cs index f2714e272a..b7f0f0bb6f 100644 --- a/src/BizHawk.Common/Extensions/NumberExtensions.cs +++ b/src/BizHawk.Common/Extensions/NumberExtensions.cs @@ -5,6 +5,8 @@ namespace BizHawk.Common.NumberExtensions { public static class NumberExtensions { + private const string ERR_MSG_PRECISION_LOSS = "unable to convert from decimal without loss of precision"; + public static string ToHexString(this int n, int numDigits) { return string.Format($"{{0:X{numDigits}}}", n); @@ -55,6 +57,76 @@ namespace BizHawk.Common.NumberExtensions return (byte)(((v / 16) * 10) + (v % 16)); } + /// the whose value is closest to + /// loss of precision (the value won't survive a round-trip) + /// like a checked conversion + public static float ConvertToF32(this decimal m) + { + var f = decimal.ToSingle(m); + return m.Equals(new decimal(f)) ? f : throw new OverflowException(ERR_MSG_PRECISION_LOSS); + } + + /// the whose value is closest to + /// loss of precision (the value won't survive a round-trip) + /// like a checked conversion + public static double ConvertToF64(this decimal m) + { + var d = decimal.ToDouble(m); + return m.Equals(new decimal(d)) ? d : throw new OverflowException(ERR_MSG_PRECISION_LOSS); + } + + /// the whose value is closest to + /// + /// iff is NaN and is set + /// (infinite values are rounded to /) + /// + /// like an unchecked conversion + public static decimal ConvertToMoneyTruncated(float f, bool throwIfNaN = false) + { + try + { +#pragma warning disable BHI1105 // this is the sanctioned call-site + return (decimal) f; +#pragma warning restore BHI1105 + } + catch (OverflowException) + { + return float.IsNaN(f) + ? throwIfNaN + ? throw new NotFiniteNumberException(f) + : default + : f < 0.0f + ? decimal.MinValue + : decimal.MaxValue; + } + } + + /// the whose value is closest to + /// + /// iff is NaN and is set + /// (infinite values are rounded to /) + /// + /// like an unchecked conversion + public static decimal ConvertToMoneyTruncated(double d, bool throwIfNaN = false) + { + try + { +#pragma warning disable BHI1105 // this is the sanctioned call-site + return (decimal) d; +#pragma warning restore BHI1105 + } + catch (OverflowException) + { + return double.IsNaN(d) + ? throwIfNaN + ? throw new NotFiniteNumberException(d) + : default + : d < 0.0 + ? decimal.MinValue + : decimal.MaxValue; + } + } + /// /// Receives a number and returns the number of hexadecimal digits it is /// Note: currently only returns 2, 4, 6, or 8