diff --git a/.global.editorconfig.ini b/.global.editorconfig.ini index 3cada92f05..da5694e99c 100644 --- a/.global.editorconfig.ini +++ b/.global.editorconfig.ini @@ -36,6 +36,9 @@ dotnet_diagnostic.BHI1120.severity = silent # Check result of IDictionary.TryGetValue, or discard it if default(T) is desired dotnet_diagnostic.BHI1200.severity = error +# Inferred type of bramches of ternary expression in interpolation don't match +dotnet_diagnostic.BHI1210.severity = error + # Call to FirstOrDefault when elements are of a value type; FirstOrNull may have been intended dotnet_diagnostic.BHI3100.severity = error # Use .Order()/.OrderDescending() shorthand diff --git a/ExternalProjects/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzer.cs b/ExternalProjects/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzer.cs new file mode 100644 index 0000000000..435981214f --- /dev/null +++ b/ExternalProjects/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzer.cs @@ -0,0 +1,89 @@ +namespace BizHawk.Analyzers; + +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class TernaryInferredTypeMismatchAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor DiagTernaryInferredTypeMismatch = new( + id: "BHI1210", + title: "Inferred type of bramches of ternary expression in interpolation don't match", +// messageFormat: "Inferred type of ternary expression is object (missing ToString call?)", + messageFormat: "{0}", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(DiagTernaryInferredTypeMismatch); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(initContext => + { + var objectSym = initContext.Compilation.GetTypeByMetadataName("System.Object")!; + var stringSym = initContext.Compilation.GetTypeByMetadataName("System.String")!; + initContext.RegisterOperationAction(oac => + { + var ifelseOrTernaryOp = (IConditionalOperation) oac.Operation; + if (ifelseOrTernaryOp.WhenFalse is null) return; + var parent = ifelseOrTernaryOp.Parent!; + if (parent.Kind is OperationKind.Conversion) parent = parent.Parent!; + if (parent.Kind is not OperationKind.Interpolation) return; + var ternaryOp = ifelseOrTernaryOp; + var typeTernary = ternaryOp.Type!; +#if false // never hit; either both branches are string and there are no conversions, or conversions are necessary + if (stringSym.Matches(typeTernary)) return; +#endif + var lhs = ternaryOp.WhenTrue; + var rhs = ternaryOp.WhenFalse; + + static IOperation TrimImplicitCast(IOperation op) + => op is IConversionOperation { Conversion.IsImplicit: true } implCastOp ? implCastOp.Operand : op; + var typeLHS = TrimImplicitCast(lhs).Type!; + var typeRHS = TrimImplicitCast(rhs).Type!; + if (typeLHS.Matches(typeRHS)) return; // unnecessary conversion operators on each branch? seen with `? this : this` + + const string ERR_MSG_OBJECT = "missing ToString means ternary branches are upcast to object"; + var fatal = false; + IOperation flaggedOp = ternaryOp; + string message; + if (stringSym.Matches(typeLHS)) + { + flaggedOp = rhs; + message = ERR_MSG_OBJECT; + } + else if (stringSym.Matches(typeRHS)) + { + flaggedOp = lhs; + message = ERR_MSG_OBJECT; + } + else if (objectSym.Matches(typeTernary)) + { + fatal = true; + message = "ternary branches are upcast to object! add ToString calls, or convert one to the other's type"; + } + else + { + // if one's already an e.g. int literal, flag the e.g. char literal + if (typeTernary.Matches(typeLHS)) flaggedOp = rhs; + else if (typeTernary.Matches(typeRHS)) flaggedOp = lhs; + message = $"ternary branches are converted to {typeTernary} before serialisation, possibly unintended"; + } + oac.ReportDiagnostic(Diagnostic.Create( + DiagTernaryInferredTypeMismatch, + flaggedOp.Syntax.GetLocation(), + fatal ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning, + additionalLocations: null, + properties: null, + messageArgs: message)); + }, + OperationKind.Conditional); + }); + } +} diff --git a/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzerTests.cs b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzerTests.cs new file mode 100644 index 0000000000..f638806fa2 --- /dev/null +++ b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/TernaryInferredTypeMismatchAnalyzerTests.cs @@ -0,0 +1,34 @@ +namespace BizHawk.Tests.Analyzers; + +using System.Threading.Tasks; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + BizHawk.Analyzers.TernaryInferredTypeMismatchAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +[TestClass] +public sealed class TernaryInferredTypeMismatchAnalyzerTests +{ + [TestMethod] + public Task CheckMisuseOfTernariesInInterpolations() + => Verify.VerifyAnalyzerAsync(""" + using System.Collections.Generic; + using System.Diagnostics; + public static class Cases { + private static string X(bool cond) + => $"{(cond ? 'p'.ToString() : 9.ToString())}"; + private static string Y(bool cond) + => $"{(cond ? "p" : 9.ToString())}"; + private static string Z(bool cond) + => $"{(cond ? "p" : "q")}"; + private static string A(bool cond) + => $"{(cond ? "p" : {|BHI1210:9|})}"; + private static string B(bool cond) + => $"{(cond ? {|BHI1210:'p'|} : 9)}"; + private static string C(bool cond, Process p, Queue q) + => $"{({|BHI1210:cond ? p : q|})}"; + } + """); +} diff --git a/References/BizHawk.Analyzer.dll b/References/BizHawk.Analyzer.dll index 600c82f8d9..5b0f2b86ac 100644 Binary files a/References/BizHawk.Analyzer.dll and b/References/BizHawk.Analyzer.dll differ diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index 21f53d04ef..4f898e51d1 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -68,7 +68,7 @@ namespace BizHawk.Client.EmuHawk var temp = new ToolStripMenuItemEx { Tag = i, - Text = $"{(quotient > 0 ? quotient : "")}&{remainder}x" + Text = $"{(quotient is not 0L ? quotient.ToString() : string.Empty)}&{remainder}x", }; temp.Click += this.WindowSize_Click; WindowSizeSubMenu.DropDownItems.Insert(i - 1, temp); diff --git a/src/BizHawk.Emulation.Common/filetype_detectors/SatellaviewFileTypeDetector.cs b/src/BizHawk.Emulation.Common/filetype_detectors/SatellaviewFileTypeDetector.cs index c8913be515..1d7d8cc1a2 100644 --- a/src/BizHawk.Emulation.Common/filetype_detectors/SatellaviewFileTypeDetector.cs +++ b/src/BizHawk.Emulation.Common/filetype_detectors/SatellaviewFileTypeDetector.cs @@ -87,7 +87,7 @@ namespace BizHawk.Emulation.Common => _header = header; public override string ToString() - => $"[{ContentTypeField >> 4:X1}] {Title} r{Revision} ({(IsSelfDestructing ? RemainingPlays : "unlimited")} plays left)"; + => $"[{ContentTypeField >> 4:X1}] {Title} r{Revision} ({(IsSelfDestructing ? RemainingPlays.ToString() : "unlimited")} plays left)"; public bool VerifyChecksum(ReadOnlySpan rom) => true; //TODO need to parse page mapping from offset 0x20..0x23 in order to calculate this diff --git a/src/BizHawk.Emulation.Cores/CPUs/FairchildF8/F3850.Disassembler.cs b/src/BizHawk.Emulation.Cores/CPUs/FairchildF8/F3850.Disassembler.cs index 602b1c3fb4..e634ff8bc4 100644 --- a/src/BizHawk.Emulation.Cores/CPUs/FairchildF8/F3850.Disassembler.cs +++ b/src/BizHawk.Emulation.Cores/CPUs/FairchildF8/F3850.Disassembler.cs @@ -28,7 +28,7 @@ namespace BizHawk.Emulation.Cores.Components.FairchildF8 if (format.ContainsOrdinal('d')) { var b = unchecked((sbyte)read(addr++)); - format = format.Replace("d", $"{(b < 0 ? '-' : '+')}{(b < 0 ? -b : b):X2}h"); + format = format.Replace("d", $"{(b < 0 ? '-' : '+')}{Math.Abs((short) b):X2}h"); } return format; diff --git a/src/BizHawk.Emulation.Cores/CPUs/Z80A/NewDisassembler.cs b/src/BizHawk.Emulation.Cores/CPUs/Z80A/NewDisassembler.cs index 405d818794..1a701b8bb5 100644 --- a/src/BizHawk.Emulation.Cores/CPUs/Z80A/NewDisassembler.cs +++ b/src/BizHawk.Emulation.Cores/CPUs/Z80A/NewDisassembler.cs @@ -374,7 +374,7 @@ namespace BizHawk.Emulation.Cores.Components.Z80A if (format.ContainsOrdinal('d')) { var b = unchecked ((sbyte) read(addr++)); - format = format.Replace("d", $"{(b < 0 ? '-' : '+')}{(b < 0 ? -b : b):X2}h"); + format = format.Replace("d", $"{(b < 0 ? '-' : '+')}{Math.Abs((short) b):X2}h"); } return format;