diff --git a/.global.editorconfig.ini b/.global.editorconfig.ini index db8ef801fc..3a8906f759 100644 --- a/.global.editorconfig.ini +++ b/.global.editorconfig.ini @@ -43,6 +43,9 @@ dotnet_diagnostic.BHI1200.severity = error # Inferred type of branches of ternary expression in interpolation don't match dotnet_diagnostic.BHI1210.severity = error +# Declare checked operators +dotnet_diagnostic.BHI1300.severity = warning + # 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/AnalyzersCommon/RoslynUtils.cs b/ExternalProjects/AnalyzersCommon/RoslynUtils.cs index 8710e0be71..43e9c33831 100644 --- a/ExternalProjects/AnalyzersCommon/RoslynUtils.cs +++ b/ExternalProjects/AnalyzersCommon/RoslynUtils.cs @@ -1,5 +1,7 @@ namespace BizHawk.Analyzers; +using System.Collections.Generic; +using System.Linq; using System.Threading; public static class RoslynUtils @@ -11,6 +13,63 @@ public static class RoslynUtils return parent; } + public static string GetMethodName(this ConversionOperatorDeclarationSyntax cods) + => cods.ImplicitOrExplicitKeyword.ToString() is "implicit" + ? WellKnownMemberNames.ImplicitConversionName + : cods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.ExplicitConversionName + : WellKnownMemberNames.CheckedExplicitConversionName; + + public static string GetMethodName(this OperatorDeclarationSyntax ods) + => ods.OperatorToken.ToString() switch + { + "!" => WellKnownMemberNames.LogicalNotOperatorName, + "!=" => WellKnownMemberNames.InequalityOperatorName, + "%" => WellKnownMemberNames.ModulusOperatorName, + "&" => WellKnownMemberNames.BitwiseAndOperatorName, + "&&" => WellKnownMemberNames.LogicalAndOperatorName, + "*" => ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.MultiplyOperatorName + : WellKnownMemberNames.CheckedMultiplyOperatorName, + "+" => ods.ParameterList.Parameters.Count is 1 + ? WellKnownMemberNames.UnaryPlusOperatorName + : ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.AdditionOperatorName + : WellKnownMemberNames.CheckedAdditionOperatorName, + "++" => ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.IncrementOperatorName + : WellKnownMemberNames.CheckedIncrementOperatorName, + "-" => ods.ParameterList.Parameters.Count is 1 + ? ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.UnaryNegationOperatorName + : WellKnownMemberNames.CheckedUnaryNegationOperatorName + : ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.SubtractionOperatorName + : WellKnownMemberNames.CheckedSubtractionOperatorName, + "--" => ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.DecrementOperatorName + : WellKnownMemberNames.CheckedDecrementOperatorName, + "/" => ods.CheckedKeyword.IsEmpty() + ? WellKnownMemberNames.DivisionOperatorName + : WellKnownMemberNames.CheckedDivisionOperatorName, + "<" => WellKnownMemberNames.LessThanOperatorName, + "<<" => WellKnownMemberNames.LeftShiftOperatorName, + "<=" => WellKnownMemberNames.LessThanOrEqualOperatorName, + "==" => WellKnownMemberNames.EqualityOperatorName, + ">" => WellKnownMemberNames.GreaterThanOperatorName, + ">=" => WellKnownMemberNames.GreaterThanOrEqualOperatorName, + ">>" => WellKnownMemberNames.RightShiftOperatorName, + ">>>" => WellKnownMemberNames.UnsignedRightShiftOperatorName, + "^" => WellKnownMemberNames.ExclusiveOrOperatorName, + "false" => WellKnownMemberNames.FalseOperatorName, + "true" => WellKnownMemberNames.TrueOperatorName, + "|" => WellKnownMemberNames.BitwiseOrOperatorName, + "||" => WellKnownMemberNames.LogicalOrOperatorName, + "~" => WellKnownMemberNames.OnesComplementOperatorName, + // ...and some operators only exist in VB.NET (dw, you're not missing anything) + _ => throw new ArgumentException(paramName: nameof(ods), message: "pretend this is a BHI6660 unexpected token in AST (in this case, a new kind of operator was added to C#)") + }; + private static ITypeSymbol? GetThrownExceptionType(this SemanticModel model, ExpressionSyntax exprSyn) => exprSyn is ObjectCreationExpressionSyntax ? model.GetTypeInfo(exprSyn).Type @@ -28,6 +87,9 @@ public static class RoslynUtils return model.GetTypeInfo(syn is ArgumentSyntax argSyn ? argSyn.Expression : syn, cancellationToken); } + public static bool IsEmpty(this SyntaxToken token) + => token.ToString().Length is 0; + public static bool Matches(this ISymbol expected, ISymbol? actual) => SymbolEqualityComparer.Default.Equals(expected, actual); @@ -42,4 +104,18 @@ public static class RoslynUtils return expected.Matches(actual as ISymbol); } #endif + + public static IEnumerable Matching( + this SyntaxList list, + INamedTypeSymbol targetAttrSym, + SemanticModel semanticModel, + CancellationToken cancellationToken) + => list.SelectMany(static als => als.Attributes) + .Where(aSyn => targetAttrSym.Matches(semanticModel.GetTypeInfo(aSyn, cancellationToken).Type)); + + public static IEnumerable Matching( + this SyntaxList list, + INamedTypeSymbol targetAttrSym, + SyntaxNodeAnalysisContext snac) + => list.Matching(targetAttrSym, snac.SemanticModel, snac.CancellationToken); } diff --git a/ExternalProjects/BizHawk.Analyzer/DefineCheckedOpAnalyzer.cs b/ExternalProjects/BizHawk.Analyzer/DefineCheckedOpAnalyzer.cs new file mode 100644 index 0000000000..f16f614e2e --- /dev/null +++ b/ExternalProjects/BizHawk.Analyzer/DefineCheckedOpAnalyzer.cs @@ -0,0 +1,85 @@ +namespace BizHawk.Analyzers; + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class DefineCheckedOpAnalyzer : DiagnosticAnalyzer +{ + private const string ERR_MSG_DEPRECATE = "consider marking this unchecked operator as [Obsolete]"; + + private const string ERR_MSG_MAKE_CHECKED = "define a checked version of this operator"; + + private static readonly DiagnosticDescriptor DiagDefineCheckedOp = new( + id: "BHI1300", + title: "Declare checked operators", + messageFormat: "{0}", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + private static readonly Dictionary UncheckedOps = new() + { + [WellKnownMemberNames.AdditionOperatorName] = WellKnownMemberNames.CheckedAdditionOperatorName, + [WellKnownMemberNames.DecrementOperatorName] = WellKnownMemberNames.CheckedDecrementOperatorName, + [WellKnownMemberNames.DivisionOperatorName] = WellKnownMemberNames.CheckedDivisionOperatorName, + [WellKnownMemberNames.ExplicitConversionName] = WellKnownMemberNames.CheckedExplicitConversionName, + [WellKnownMemberNames.IncrementOperatorName] = WellKnownMemberNames.CheckedIncrementOperatorName, + [WellKnownMemberNames.MultiplyOperatorName] = WellKnownMemberNames.CheckedMultiplyOperatorName, + [WellKnownMemberNames.SubtractionOperatorName] = WellKnownMemberNames.CheckedSubtractionOperatorName, + [WellKnownMemberNames.UnaryNegationOperatorName] = WellKnownMemberNames.CheckedUnaryNegationOperatorName, + }; + + public override ImmutableArray SupportedDiagnostics { get; } + = ImmutableArray.Create(/*HawkSourceAnalyzer.DiagWTF,*/ DiagDefineCheckedOp); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + INamedTypeSymbol? obsoleteAttrSym = null; + context.RegisterSyntaxNodeAction( + snac => + { + obsoleteAttrSym ??= snac.Compilation.GetTypeByMetadataName("System.ObsoleteAttribute")!; + bool HasCheckedCounterpart(SyntaxNode node, string checkedName, Func getMethodName) + => ((TypeDeclarationSyntax) node.Parent!).Members.OfType() //TODO don't think this accounts for `partial` types + .Any(syn => checkedName.Equals(getMethodName(syn), StringComparison.Ordinal)); + void SuggestDeprecation(SyntaxNode node) + => DiagDefineCheckedOp.ReportAt(node, DiagnosticSeverity.Warning, snac, ERR_MSG_DEPRECATE); + switch (snac.Node) + { + case ConversionOperatorDeclarationSyntax cods: + if (UncheckedOps.TryGetValue(cods.GetMethodName(), out var checkedName)) + { + var hasCheckedCounterpart = HasCheckedCounterpart( + cods, + checkedName, + RoslynUtils.GetMethodName); + if (!hasCheckedCounterpart) DiagDefineCheckedOp.ReportAt(cods, snac, ERR_MSG_MAKE_CHECKED); + else if (!cods.AttributeLists.Matching(obsoleteAttrSym, snac).Any()) SuggestDeprecation(cods); + // else you did it good job + } + break; + case OperatorDeclarationSyntax ods: + if (UncheckedOps.TryGetValue(ods.GetMethodName(), out var checkedName1)) + { + var hasCheckedCounterpart = HasCheckedCounterpart( + ods, + checkedName1, + RoslynUtils.GetMethodName); + if (!hasCheckedCounterpart) DiagDefineCheckedOp.ReportAt(ods, snac, ERR_MSG_MAKE_CHECKED); + else if (!ods.AttributeLists.Matching(obsoleteAttrSym, snac).Any()) SuggestDeprecation(ods); + // else you did it good job + } + break; + default: + HawkSourceAnalyzer.ReportWTF(snac.Node, snac, message: $"[{nameof(DefineCheckedOpAnalyzer)}] unexpected node kind {snac.Node.GetType().FullName}"); + break; + } + }, + SyntaxKind.ConversionOperatorDeclaration, + SyntaxKind.OperatorDeclaration); + } +} diff --git a/ExternalProjects/BizHawk.Analyzer/FeatureNotImplementedAnalyzer.cs b/ExternalProjects/BizHawk.Analyzer/FeatureNotImplementedAnalyzer.cs index 2d64ff198c..f0c91605e1 100644 --- a/ExternalProjects/BizHawk.Analyzer/FeatureNotImplementedAnalyzer.cs +++ b/ExternalProjects/BizHawk.Analyzer/FeatureNotImplementedAnalyzer.cs @@ -44,8 +44,7 @@ public sealed class FeatureNotImplementedAnalyzer : DiagnosticAnalyzer // else correct usage, do not flag } bool IncludesFNIAttribute(SyntaxList mds) - => mds.SelectMany(static als => als.Attributes) - .Any(aSyn => featureNotImplementedAttrSym.Matches(snac.SemanticModel.GetTypeInfo(aSyn, snac.CancellationToken).Type)); + => mds.Matching(featureNotImplementedAttrSym, snac).Any(); void CheckBlockBody(BlockSyntax bs, Location location) { if (bs.Statements is [ ThrowStatementSyntax tss ]) MaybeReportFor(snac.SemanticModel.GetThrownExceptionType(tss), tss); diff --git a/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/DefineCheckedOpAnalyzerTests.cs b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/DefineCheckedOpAnalyzerTests.cs new file mode 100644 index 0000000000..e5ba526a66 --- /dev/null +++ b/ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/DefineCheckedOpAnalyzerTests.cs @@ -0,0 +1,79 @@ +namespace BizHawk.Tests.Analyzers; + +using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< + BizHawk.Analyzers.DefineCheckedOpAnalyzer, + Microsoft.CodeAnalysis.Testing.DefaultVerifier>; + +[TestClass] +public sealed class DefineCheckedOpAnalyzerTests +{ + [TestMethod] + public Task CheckMisuseOfCheckedOperatorOverloading() + => Verify.VerifyAnalyzerAsync(""" + /* very much a sample; real numeric types should follow these rules according to [the docs](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/arithmetic-operators#user-defined-checked-operators): + * - A checked operator throws an OverflowException. + * - An operator without the checked modifier returns an instance representing a truncated result. + * I didn't do any of that, I just came up with something that threw and something that didn't and called it a day --yoshi + */ + using System; + public struct GF7 { + private static readonly int[] Inverses = [ -1, 1, 4, 5, 2, 3, 6 ]; + public static GF7 operator checked -(GF7 x) => new(-x._value); // the most useless but here for completeness + {|BHI1300:public static GF7 operator /*unchecked*/ -(GF7 x) => new(-x._value % 7);|} + public static GF7 operator checked ++(GF7 x) => new(x._value + 1); + {|BHI1300:public static GF7 operator /*unchecked*/ ++(GF7 x) => new((x._value + 1) % 7);|} + public static GF7 operator checked --(GF7 x) => new(x._value - 1); + {|BHI1300:public static GF7 operator /*unchecked*/ --(GF7 x) => new((x._value - 1) % 7);|} + public static GF7 operator checked +(GF7 x, GF7 y) => new(x._value + y._value); + {|BHI1300:public static GF7 operator /*unchecked*/ +(GF7 x, GF7 y) => new((x._value + y._value) % 7);|} + public static GF7 operator checked +(GF7 x, int y) => new(x._value + y); + {|BHI1300:public static GF7 operator /*unchecked*/ +(GF7 x, int y) => new((x._value + y) % 7);|} + public static GF7 operator checked -(GF7 minuend, int subtrahend) => new(minuend._value - subtrahend); + {|BHI1300:public static GF7 operator /*unchecked*/ -(GF7 minuend, int subtrahend) => new((minuend._value - subtrahend) % 7);|} + public static GF7 operator checked *(GF7 x, int y) => new(x._value * y); + {|BHI1300:public static GF7 operator /*unchecked*/ *(GF7 x, int y) => new((x._value * y) % 7);|} + public static GF7 operator checked /(GF7 dividend, GF7 divisor) => new((dividend._value * Inverses[divisor._value]) % 7); + {|BHI1300:public static GF7 operator /*unchecked*/ /(GF7 dividend, GF7 divisor) => divisor._value is 0 ? default : new((dividend._value * Inverses[divisor._value]) % 7);|} + public static explicit operator checked GF7(int n) => new(n); + {|BHI1300:public static explicit operator /*unchecked*/ GF7(int n) => new(n % 7);|} + private byte _value; + public byte Value { + get => _value; + set { + if (value < 0 || 6 < value) throw new ArgumentOutOfRangeException(paramName: nameof(value), value, message: "must be in 0..<7"); + _value = value; + } + } + public GF7(byte value) => Value = value; + private GF7(int value) => Value = unchecked((byte) value); + } + """); + + [TestMethod] + public Task CheckMisuseOfUncheckedOperatorOverloading() + => Verify.VerifyAnalyzerAsync(""" + using System; + public struct GF7 { + private static readonly int[] Inverses = [ -1, 1, 4, 5, 2, 3, 6 ]; + {|BHI1300:public static GF7 operator -(GF7 x) => new(-x._value % 7);|} + {|BHI1300:public static GF7 operator ++(GF7 x) => new((x._value + 1) % 7);|} + {|BHI1300:public static GF7 operator --(GF7 x) => new((x._value - 1) % 7);|} + {|BHI1300:public static GF7 operator +(GF7 x, GF7 y) => new((x._value + y._value) % 7);|} + {|BHI1300:public static GF7 operator +(GF7 x, int y) => new((x._value + y) % 7);|} + {|BHI1300:public static GF7 operator -(GF7 minuend, int subtrahend) => new((minuend._value - subtrahend) % 7);|} + {|BHI1300:public static GF7 operator *(GF7 x, int y) => new((x._value * y) % 7);|} + {|BHI1300:public static GF7 operator /(GF7 dividend, GF7 divisor) => divisor._value is 0 ? default : new((dividend._value * Inverses[divisor._value]) % 7);|} + {|BHI1300:public static explicit operator GF7(int n) => new(n % 7);|} + private byte _value; + public byte Value { + get => _value; + set { + if (value < 0 || 6 < value) throw new ArgumentOutOfRangeException(paramName: nameof(value), value, message: "must be in 0..<7"); + _value = value; + } + } + public GF7(byte value) => Value = value; + private GF7(int value) => Value = unchecked((byte) value); + } + """); +} diff --git a/References/BizHawk.Analyzer.dll b/References/BizHawk.Analyzer.dll index 55fc503de8..17c066a187 100644 Binary files a/References/BizHawk.Analyzer.dll and b/References/BizHawk.Analyzer.dll differ diff --git a/src/BizHawk.Common/Win32/ShellLinkImports.cs b/src/BizHawk.Common/Win32/ShellLinkImports.cs index dc8036523d..797f899b88 100644 --- a/src/BizHawk.Common/Win32/ShellLinkImports.cs +++ b/src/BizHawk.Common/Win32/ShellLinkImports.cs @@ -1,4 +1,5 @@ #nullable disable +#pragma warning disable BHI1300 // explicit conversion operators w/ no checked variant using System.Runtime.CompilerServices; using System.Runtime.InteropServices;