Add Analyzer for suggesting adding `checked` operators
This commit is contained in:
parent
9a388c6693
commit
8d71c8a505
|
@ -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
|
||||
|
|
|
@ -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<AttributeSyntax> Matching(
|
||||
this SyntaxList<AttributeListSyntax> 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<AttributeSyntax> Matching(
|
||||
this SyntaxList<AttributeListSyntax> list,
|
||||
INamedTypeSymbol targetAttrSym,
|
||||
SyntaxNodeAnalysisContext snac)
|
||||
=> list.Matching(targetAttrSym, snac.SemanticModel, snac.CancellationToken);
|
||||
}
|
||||
|
|
|
@ -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<string, string> 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<DiagnosticDescriptor> 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<T>(SyntaxNode node, string checkedName, Func<T, string> getMethodName)
|
||||
=> ((TypeDeclarationSyntax) node.Parent!).Members.OfType<T>() //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<ConversionOperatorDeclarationSyntax>(
|
||||
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<OperatorDeclarationSyntax>(
|
||||
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);
|
||||
}
|
||||
}
|
|
@ -44,8 +44,7 @@ public sealed class FeatureNotImplementedAnalyzer : DiagnosticAnalyzer
|
|||
// else correct usage, do not flag
|
||||
}
|
||||
bool IncludesFNIAttribute(SyntaxList<AttributeListSyntax> 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);
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
""");
|
||||
}
|
Binary file not shown.
|
@ -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;
|
||||
|
|
Loading…
Reference in New Issue