Add Analyzer rule to enforce throwing in `[FeatureNotImplemented]`

Common.ruleset

@@ -15,6 +15,9 @@
 
 		<!-- Default branch of switch expression should throw InvalidOperationException/SwitchExpressionException or not throw -->
 		<Rule Id="BHI1005" Action="Error" />
+
+		<!-- Throw NotImplementedException from methods/props marked [FeatureNotImplemented] -->
+		<Rule Id="BHI3300" Action="Error" />
 	</Rules>
 	<Rules AnalyzerId="DocumentationAnalyzers" RuleNamespace="DocumentationAnalyzers.StyleRules">
 		<!-- Place text in paragraphs -->

ExternalProjects/BizHawk.Analyzer/FeatureNotImplementedAnalyzer.cs

@@ -0,0 +1,111 @@
+namespace BizHawk.Analyzers;
+
+using System.Collections.Immutable;
+using System.Linq;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class FeatureNotImplementedAnalyzer : DiagnosticAnalyzer
+{
+	private const string ERR_MSG_DOES_NOT_THROW = "Throw NotImplementedException in [FeatureNotImplemented] method/prop body (or remove attribute)";
+
+	private const string ERR_MSG_METHOD_THROWS_UNKNOWN = "Indeterminable exception type in [FeatureNotImplemented] method/prop body, should be NotImplementedException";
+
+	private const string ERR_MSG_THROWS_WRONG_TYPE = "Incorrect exception type in [FeatureNotImplemented] method/prop body, should be NotImplementedException";
+
+	private const string ERR_MSG_UNEXPECTED_INCANTATION = "It seems [FeatureNotImplemented] should not be applied to whatever this is";
+
+	private static readonly DiagnosticDescriptor DiagShouldThrowNIE = new(
+		id: "BHI3300",
+		title: "Throw NotImplementedException from methods/props marked [FeatureNotImplemented]",
+		messageFormat: "{0}",
+		category: "Usage",
+		defaultSeverity: DiagnosticSeverity.Error,
+		isEnabledByDefault: true);
+
+	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagShouldThrowNIE);
+
+	public override void Initialize(AnalysisContext context)
+	{
+		context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+		context.EnableConcurrentExecution();
+		var skipThisProject = false;
+		INamedTypeSymbol? featureNotImplementedAttrSym = null;
+		INamedTypeSymbol? notImplementedExceptionSym = null;
+		context.RegisterSyntaxNodeAction(
+			snac =>
+			{
+				if (skipThisProject) return;
+				if (featureNotImplementedAttrSym is null)
+				{
+					featureNotImplementedAttrSym = snac.Compilation.GetTypeByMetadataName("BizHawk.Emulation.Common.FeatureNotImplementedAttribute");
+					if (featureNotImplementedAttrSym is null)
+					{
+						// project does not have BizHawk.Emulation.Common dependency
+						skipThisProject = true;
+						return;
+					}
+					notImplementedExceptionSym = snac.Compilation.GetTypeByMetadataName("System.NotImplementedException")!;
+				}
+				void Wat(Location location)
+					=> snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_UNEXPECTED_INCANTATION));
+				void MaybeReportFor(ITypeSymbol? thrownExceptionType, Location location)
+				{
+					if (thrownExceptionType is null) snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_METHOD_THROWS_UNKNOWN));
+					else if (!notImplementedExceptionSym!.Matches(thrownExceptionType)) snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_THROWS_WRONG_TYPE));
+					// 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).Type));
+				void CheckBlockBody(BlockSyntax bs, Location location)
+				{
+					if (bs.Statements.Count is not 1) snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_DOES_NOT_THROW));
+					else if (bs.Statements[0] is not ThrowStatementSyntax tss) snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_DOES_NOT_THROW));
+					else MaybeReportFor(snac.SemanticModel.GetThrownExceptionType(tss), tss.GetLocation());
+				}
+				void CheckExprBody(ArrowExpressionClauseSyntax aecs, Location location)
+				{
+					if (aecs.Expression is not ThrowExpressionSyntax tes) snac.ReportDiagnostic(Diagnostic.Create(DiagShouldThrowNIE, location, ERR_MSG_DOES_NOT_THROW));
+					else MaybeReportFor(snac.SemanticModel.GetThrownExceptionType(tes), tes.GetLocation());
+				}
+				void CheckAccessor(AccessorDeclarationSyntax ads)
+				{
+					if (!IncludesFNIAttribute(ads.AttributeLists)) return;
+					if (ads.ExpressionBody is not null) CheckExprBody(ads.ExpressionBody, ads.GetLocation());
+					else if (ads.Body is not null) CheckBlockBody(ads.Body, ads.GetLocation());
+					else Wat(ads.GetLocation());
+				}
+				switch (snac.Node)
+				{
+					case AccessorDeclarationSyntax ads:
+						CheckAccessor(ads);
+						break;
+					case MethodDeclarationSyntax mds:
+						if (!IncludesFNIAttribute(mds.AttributeLists)) return;
+						if (mds.ExpressionBody is not null) CheckExprBody(mds.ExpressionBody, mds.GetLocation());
+						else if (mds.Body is not null) CheckBlockBody(mds.Body, mds.GetLocation());
+						else Wat(mds.GetLocation());
+						break;
+					case PropertyDeclarationSyntax pds:
+						if (pds.ExpressionBody is not null)
+						{
+							if (IncludesFNIAttribute(pds.AttributeLists)) CheckExprBody(pds.ExpressionBody, pds.GetLocation());
+						}
+						else
+						{
+							if (IncludesFNIAttribute(pds.AttributeLists)) Wat(pds.GetLocation());
+							else foreach (var accessor in pds.AccessorList!.Accessors) CheckAccessor(accessor);
+						}
+						break;
+				}
+			},
+			SyntaxKind.GetAccessorDeclaration,
+			SyntaxKind.MethodDeclaration,
+			SyntaxKind.PropertyDeclaration,
+			SyntaxKind.SetAccessorDeclaration);
+	}
+}

ExternalProjects/BizHawk.Analyzer/RoslynUtils.cs

@@ -13,6 +13,9 @@ internal static class RoslynUtils
 	public static ITypeSymbol? GetThrownExceptionType(this SemanticModel model, ThrowExpressionSyntax tes)
 		=> model.GetThrownExceptionType(tes.Expression);
 
+	public static ITypeSymbol? GetThrownExceptionType(this SemanticModel model, ThrowStatementSyntax tss)
+		=> model.GetThrownExceptionType(tss.Expression!);
+
 	public static bool Matches(this ITypeSymbol expected, ITypeSymbol? actual)
 		=> SymbolEqualityComparer.Default.Equals(expected, actual);
 }

src/BizHawk.Emulation.Cores/Computers/AppleII/AppleII.IVideoProvider.cs

@@ -17,15 +17,9 @@ namespace BizHawk.Emulation.Cores.Computers.AppleII
 		public int BackgroundColor => 0;
 
 		public int VsyncNumerator
-		{
+			=> NullVideo.DefaultVsyncNum; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented] // TODO: precise numbers or confirm the default is okay
-			get => NullVideo.DefaultVsyncNum;
-		}
 
 		public int VsyncDenominator
-		{
+			=> NullVideo.DefaultVsyncDen; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented] // TODO: precise numbers or confirm the default is okay
-			get => NullVideo.DefaultVsyncDen;
-		}
 	}
 }

src/BizHawk.Emulation.Cores/Computers/SinclairSpectrum/ZXSpectrum.ICodeDataLog.cs

@@ -1,6 +1,8 @@
-using BizHawk.Emulation.Common;
+using System;
 using System.IO;
 
+using BizHawk.Emulation.Common;
+
 namespace BizHawk.Emulation.Cores.Computers.SinclairSpectrum
 {
 	/// <summary>
@@ -64,9 +66,7 @@ namespace BizHawk.Emulation.Cores.Computers.SinclairSpectrum
 
 		[FeatureNotImplemented]
 		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-
-		}
 
 		public enum CDLType
 		{

src/BizHawk.Emulation.Cores/Consoles/Coleco/TMS9918A.cs

@@ -471,16 +471,10 @@ namespace BizHawk.Emulation.Cores.ColecoVision
 		public int BackgroundColor => 0;
 
 		public int VsyncNumerator
-		{
+			=> NullVideo.DefaultVsyncNum; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncNum;
-		}
 
 		public int VsyncDenominator
-		{
+			=> NullVideo.DefaultVsyncDen; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncDen;
-		}
 
 		private readonly int[] PaletteTMS9918 =
 		{

src/BizHawk.Emulation.Cores/Consoles/GCE/Vectrex/VectrexHawk.ICodeDataLog.cs

@@ -23,9 +23,7 @@ namespace BizHawk.Emulation.Cores.Consoles.Vectrex
 
 		[FeatureNotImplemented]
 		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-
-		}
 
 		private enum CDLog_AddrType
 		{

src/BizHawk.Emulation.Cores/Consoles/Intellivision/STIC.cs

@@ -83,16 +83,10 @@ namespace BizHawk.Emulation.Cores.Intellivision
 		public int BackgroundColor => 0;
 
 		public int VsyncNumerator
-		{
+			=> NullVideo.DefaultVsyncNum; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncNum;
-		}
 
 		public int VsyncDenominator
-		{
+			=> NullVideo.DefaultVsyncDen; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncDen;
-		}
 
 		public void Reset()
 		{

src/BizHawk.Emulation.Cores/Consoles/Magnavox/Odyssey2/O2Hawk.ICodeDataLog.cs

@@ -1,3 +1,4 @@
+using System;
 using System.IO;
 using BizHawk.Emulation.Common;
 using BizHawk.Emulation.Cores.Components.I8048;
@@ -31,9 +32,8 @@ namespace BizHawk.Emulation.Cores.Consoles.O2Hawk
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		public void SetCDL(I8048.eCDLogMemFlags flags, string type, int cdladdr)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBA/MGBAHawk.IDebuggable.cs

@@ -50,7 +50,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBA
 			}
 		}
 
-		[FeatureNotImplemented]
 		public IMemoryCallbackSystem MemoryCallbacks { get; }
 
 		public bool CanStep(StepType type) => false;

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBA/MGBAHawk.cs

@@ -175,7 +175,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBA
 			return _gpumem;
 		}
 
-		[FeatureNotImplemented]
 		public void SetScanlineCallback(Action callback, int scanline)
 		{
 			_scanlinecb = callback;

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBHawk/GBHawk.ICodeDataLog.cs

@@ -1,3 +1,4 @@
+using System;
 using System.IO;
 using BizHawk.Emulation.Common;
 using BizHawk.Emulation.Cores.Components.LR35902;
@@ -31,9 +32,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBHawk
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		public void SetCDL(LR35902.eCDLogMemFlags flags, string type, int cdladdr)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBHawkLink/GBHawkLink.ICodeDataLog.cs

@@ -1,3 +1,4 @@
+using System;
 using System.IO;
 
 using BizHawk.Emulation.Common;
@@ -32,9 +33,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBHawkLink
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		public void SetCDL(LR35902.eCDLogMemFlags flags, string type, int cdladdr)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBHawkLink3x/GBHawkLink3x.ICodeDataLog.cs

@@ -1,3 +1,4 @@
+using System;
 using System.IO;
 
 using BizHawk.Emulation.Common;
@@ -32,9 +33,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBHawkLink3x
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		public void SetCDL(LR35902.eCDLogMemFlags flags, string type, int cdladdr)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/GBHawkLink4x/GBHawkLink4x.ICodeDataLog.cs

@@ -1,3 +1,4 @@
+using System;
 using System.IO;
 
 using BizHawk.Emulation.Common;
@@ -32,9 +33,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.GBHawkLink4x
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		public void SetCDL(LR35902.eCDLogMemFlags flags, string type, int cdladdr)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/Gambatte.ICodeDataLog.cs

@@ -33,9 +33,8 @@ namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-		}
 
 		private ICodeDataLog _cdl;
 		private string _which;

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/GambatteLink.ICodeDataLog.cs

@@ -1,7 +1,8 @@
-using BizHawk.Emulation.Common;
+using System;
-
 using System.IO;
 
+using BizHawk.Emulation.Common;
+
 namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
 {
 	partial class GambatteLink : ICodeDataLogger
@@ -23,8 +24,9 @@ namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
 		}
 
 		[FeatureNotImplemented]
-		void ICodeDataLogger.DisassembleCDL(Stream s, ICodeDataLog cdl)
+		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
 		{
+			throw new NotImplementedException();
 			// this doesn't actually do anything
 			/*
 			for (int i = 0; i < _numCores; i++)

src/BizHawk.Emulation.Cores/Consoles/Nintendo/N64/N64.IInputPollable.cs

@@ -39,6 +39,6 @@ namespace BizHawk.Emulation.Cores.Nintendo.N64
 		}
 
 		// TODO: optimize managed to unmanaged using the ActiveChanged event
-		public IInputCallbackSystem InputCallbacks { [FeatureNotImplemented] get; }
+		public IInputCallbackSystem InputCallbacks { get; }
 	}
 }

src/BizHawk.Emulation.Cores/Consoles/Nintendo/NES/NES.ICodeDataLogger.cs

@@ -28,9 +28,7 @@ namespace BizHawk.Emulation.Cores.Nintendo.NES
 
 		[FeatureNotImplemented]
 		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-
-		}
 
 		public enum CDLog_AddrType
 		{

src/BizHawk.Emulation.Cores/Consoles/PC Engine/VDC.Render.cs

@@ -445,15 +445,9 @@ namespace BizHawk.Emulation.Cores.PCEngine
 		public int BackgroundColor => vce.Palette[256];
 
 		public int VsyncNumerator
-		{
+			=> NullVideo.DefaultVsyncNum; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncNum;
-		}
 
 		public int VsyncDenominator
-		{
+			=> NullVideo.DefaultVsyncDen; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncDen;
-		}
 	}
 }

src/BizHawk.Emulation.Cores/Consoles/PC Engine/VPC.cs

@@ -534,15 +534,9 @@ namespace BizHawk.Emulation.Cores.PCEngine
 		public int BackgroundColor => VCE.Palette[0];
 
 		public int VsyncNumerator
-		{
+			=> NullVideo.DefaultVsyncNum; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncNum;
-		}
 
 		public int VsyncDenominator
-		{
+			=> NullVideo.DefaultVsyncDen; //TODO precise numbers or confirm the default is okay
-			[FeatureNotImplemented]
-			get => NullVideo.DefaultVsyncDen;
-		}
 	}
 }

src/BizHawk.Emulation.Cores/Consoles/Sega/GGHawkLink/GGHawkLink.ICodeDataLog.cs

@@ -42,9 +42,7 @@ namespace BizHawk.Emulation.Cores.Sega.GGHawkLink
 
 		[FeatureNotImplemented]
 		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-
-		}
 
 		private enum CDLog_AddrType
 		{

src/BizHawk.Emulation.Cores/Consoles/Sega/SMS/SMS.ICodeDataLogger.cs

@@ -40,9 +40,7 @@ namespace BizHawk.Emulation.Cores.Sega.MasterSystem
 
 		[FeatureNotImplemented]
 		public void DisassembleCDL(Stream s, ICodeDataLog cdl)
-		{
+			=> throw new NotImplementedException();
-
-		}
 
 		public enum CDLog_AddrType
 		{

src/BizHawk.Emulation.Cores/Consoles/Sony/PSX/Octoshock.cs

@@ -918,7 +918,6 @@ namespace BizHawk.Emulation.Cores.Sony.PSX
 			get => throw new NotImplementedException();
 		}
 
-		[FeatureNotImplemented]
 		public bool DeterministicEmulation => true;
 
 		public int[] GetVideoBuffer() => frameBuffer;