Add Analyzer to warn about LINQ calls on string receivers

.global.editorconfig.ini

@@ -47,6 +47,8 @@ dotnet_diagnostic.BHI1210.severity = error
 dotnet_diagnostic.BHI3100.severity = error
 # Use .Order()/.OrderDescending() shorthand
 dotnet_diagnostic.BHI3101.severity = warning
+# Prefer specialised methods over LINQ on string receivers
+dotnet_diagnostic.BHI3102.severity = error
 # Throw NotImplementedException from methods/props marked [FeatureNotImplemented]
 dotnet_diagnostic.BHI3300.severity = error
 

ExternalProjects/AnalyzersCommon/RoslynUtils.cs

@@ -1,8 +1,11 @@
 namespace BizHawk.Analyzers;
 
+using System.Threading;
+
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Operations;
 
 public static class RoslynUtils
 {
@@ -24,6 +27,12 @@ public static class RoslynUtils
 	public static ITypeSymbol? GetThrownExceptionType(this SemanticModel model, ThrowStatementSyntax tss)
 		=> model.GetThrownExceptionType(tss.Expression!);
 
+	public static TypeInfo GetTypeInfo(this SemanticModel model, IArgumentOperation argOp, CancellationToken cancellationToken)
+	{
+		var syn = argOp.Syntax;
+		return model.GetTypeInfo(syn is ArgumentSyntax argSyn ? argSyn.Expression : syn, cancellationToken);
+	}
+
 	public static bool Matches(this ISymbol expected, ISymbol? actual)
 		=> SymbolEqualityComparer.Default.Equals(expected, actual);
 

ExternalProjects/BizHawk.Analyzer/ExprBodiedMemberFlowAnalyzer.cs

@@ -115,7 +115,9 @@ public sealed class ExprBodiedMemberFlowAnalyzer : DiagnosticAnalyzer
 						Fail();
 						return;
 					}
+#pragma warning disable BHI3102 // LINQ `Contains(char)` is fine here
 					var hasLineBreakAfterArrow = aecs.ArrowToken.HasTrailingTrivia && aecs.ArrowToken.TrailingTrivia.ToFullString().Contains('\n');
+#pragma warning restore BHI3102
 					if ((hasLineBreakAfterArrow ? '\n' : ' ') != expectedWhitespace.After) Fail();
 				},
 				SyntaxKind.ArrowExpressionClause);

ExternalProjects/BizHawk.Analyzer/LINQOnStringsAnalyzer.cs

@@ -0,0 +1,137 @@
+namespace BizHawk.Analyzers;
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class LINQOnStringsAnalyzer : DiagnosticAnalyzer
+{
+	private static readonly DiagnosticDescriptor DiagLINQOnStrings = new(
+		id: "BHI3102",
+		title: "Prefer specialised methods over LINQ on string receivers",
+		messageFormat: "{0}",
+		category: "Usage",
+		defaultSeverity: DiagnosticSeverity.Warning,
+		isEnabledByDefault: true);
+
+	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagLINQOnStrings);
+
+	public override void Initialize(AnalysisContext context)
+	{
+		context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+		context.EnableConcurrentExecution();
+		context.RegisterCompilationStartAction(initContext =>
+		{
+			const string CHAR_ZERO_WARNING = " (but did you intend to get `default(char)`?)";
+			static string DisambigNameFor(IMethodSymbol sym)
+				=> $"{sym.Name}#{sym.Parameters.Length}";
+//			var hasHawkStringExtensions = initContext.Compilation.GetTypeByMetadataName("BizHawk.Common.StringExtensions.StringExtensions") is not null;
+			var hasStringContainsChar = initContext.Compilation.GetSpecialType(SpecialType.System_String)
+				.GetMembers("Contains").Length > 1;
+			var linqMethodSyms = initContext.Compilation.GetTypeByMetadataName("System.Linq.Enumerable")!.GetMembers();
+			Dictionary<string, (IMethodSymbol SymToMatch, DiagnosticSeverity Level, string MsgFmtStr)> banned = new();
+			foreach (var (identifier, msgFmtStr) in new[]
+			{
+				("Append", "Use concatenation or interpolation"),
+				("Concat", "Use concatenation operator"), // we have special handling for when the "other" string is only `IEnumerable<char>`
+				("ElementAt", "Use `({0})[i]`"),
+				("ElementAtOrDefault", "Use `i < str.Length ? str[i] : default`" + CHAR_ZERO_WARNING),
+				("Prepend", "Use concatenation or interpolation"),
+				("Skip", "Use `({0}).Substring(offset)` (or `.AsSpan(offset)`)"),
+				("Take", "Use `({0}).Substring(0, count)` (or `.AsSpan(0, count)`)"),
+				("ToArray", "Use `({0}).ToCharArray()`"),
+			})
+			{
+				// only one overload, simply add to list
+				var foundSym = linqMethodSyms.OfType<IMethodSymbol>().FirstOrDefault(sym => sym.Name == identifier);
+				if (foundSym is not null) banned[DisambigNameFor(foundSym)] = (foundSym, DiagnosticSeverity.Warning, msgFmtStr);
+			}
+			foreach (var (identifier, msgFmtStr) in new[]
+			{
+				("Any", "Use `({0}).Length is not 0`"),
+				("Count", "Use `({0}).Length`"),
+				("First", "Use `({0})[0]`"),
+				("FirstOrDefault", "Use `str.Length is 0 ? default : str[0]`" + CHAR_ZERO_WARNING),
+				("Last", "Use `({0})[^1]`"),
+				("LastOrDefault", "Use `str.Length is 0 ? default : str[^1]`" + CHAR_ZERO_WARNING),
+				("LongCount", "Use `(long) ({0}).Length` (`String` cannot exceed `int.MaxValue` chars)"),
+				("Single", "Use `str[0]`, asserting `str.Length is 1` beforehand if desired"),
+				("SingleOrDefault", "Use `str.Length is 1 ? str[0] : default`" + CHAR_ZERO_WARNING),
+			})
+			{
+				// for these, the overload with a delegate param (combined `Where`) is allowed, and the overload without is banned
+				var foundSym = linqMethodSyms.OfType<IMethodSymbol>()
+					.FirstOrDefault(sym => sym.Parameters.Length is 1 && sym.Name == identifier);
+				if (foundSym is not null) banned[DisambigNameFor(foundSym)] = (foundSym, DiagnosticSeverity.Error, msgFmtStr);
+			}
+			var linqBinaryContainsSym = linqMethodSyms.OfType<IMethodSymbol>()
+				.FirstOrDefault(static sym => sym.Parameters.Length is 2 && sym.Name is "Contains");
+			if (linqBinaryContainsSym is not null)
+			{
+				banned[DisambigNameFor(linqBinaryContainsSym)] = (
+					linqBinaryContainsSym,
+					DiagnosticSeverity.Error,
+					"Call the instance method from `String` (why would you do this)");
+			}
+			foreach (var sym in linqMethodSyms.OfType<IMethodSymbol>().Where(static sym => sym.Name is "DefaultIfEmpty"))
+			{
+				banned[DisambigNameFor(sym)] = sym.Parameters.Length is 2
+					? (sym, DiagnosticSeverity.Warning, "Use `str.Length is 0 ? [ defaultValue ] : str`")
+					: (sym, DiagnosticSeverity.Error, "Use `str.Length is 0 ? [ default(char) ] : str`" + CHAR_ZERO_WARNING);
+			}
+			var linqReverseSym = linqMethodSyms.OfType<IMethodSymbol>().FirstOrDefault(static sym => sym.Name is "Reverse");
+			if (linqReverseSym is not null)
+			{
+				banned[DisambigNameFor(linqReverseSym)] = (
+					linqReverseSym,
+					DiagnosticSeverity.Error,
+					"This will reverse the order of `char`s, which are NOT Unicode graphemes or even codepoints, and thus the result may be malformed;"
+						+ " to reverse text, use `str.EnumerateRunes().Reverse()` (.NET Core) or `Strings.StrReverse(str)` (from VB.NET helpers),"
+						+ " and to reverse a list of 16-bit values, use `Array.Reverse` on a `char[]`/`ushort[]`");
+			}
+			initContext.RegisterOperationAction(
+				oac =>
+				{
+					var operation = (IInvocationOperation) oac.Operation;
+					var calledSym = operation.TargetMethod.ConstructedFrom;
+					var disambigName = DisambigNameFor(calledSym);
+					if (!banned.TryGetValue(disambigName, out var tuple)) return;
+					var (symToMatch, level, msgFmtStr) = tuple;
+					if (!symToMatch.Matches(calledSym)) return;
+					var receiverExpr = operation.Arguments[0];
+					var receiverExprType = operation.SemanticModel!.GetTypeInfo(receiverExpr, oac.CancellationToken).Type!;
+					if (receiverExprType.SpecialType is not SpecialType.System_String) return;
+					switch (disambigName)
+					{
+						case "Concat#2" /*when operation.SemanticModel!
+								.GetTypeInfo(operation.Arguments[1], oac.CancellationToken)
+								.Type!.SpecialType is not SpecialType.System_String*/:
+							// other string is only `IEnumerable<char>`
+							level = DiagnosticSeverity.Warning;
+							msgFmtStr = "Use concatenation if both are strings and the value is being enumerated to a new string";
+							break;
+						case "Contains#1" when !hasStringContainsChar:
+							level = DiagnosticSeverity.Warning;
+							var arg = operation.Arguments[1].ConstantValue;
+							msgFmtStr = arg.HasValue
+								? $"Use `({{0}}).Contains(\"{arg.Value}\")`"
+								: "Use `str.Contains(c.ToString())`";
+							break;
+					}
+					oac.ReportDiagnostic(Diagnostic.Create(
+						DiagLINQOnStrings,
+						operation.Syntax.GetLocation(),
+						level,
+						additionalLocations: null,
+						properties: null,
+						messageArgs: string.Format(msgFmtStr, receiverExpr.Syntax)));
+				},
+				OperationKind.Invocation);
+		});
+	}
+}

ExternalProjects/BizHawk.AnalyzersTests/BizHawk.Analyzer/LINQOnStringsAnalyzerTests.cs

@@ -0,0 +1,91 @@
+namespace BizHawk.Tests.Analyzers;
+
+using System.Threading.Tasks;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<
+	BizHawk.Analyzers.LINQOnStringsAnalyzer,
+	Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
+
+[TestClass]
+public sealed class LINQOnStringsAnalyzerTests
+{
+	[TestMethod]
+	public Task CheckMisuseOfLINQOnStrings()
+		=> Verify.VerifyAnalyzerAsync("""
+			using System.Collections.Generic;
+			using System.Linq;
+			public static class Cases {
+				private static bool DummyPredicate(char c)
+					=> c is >= 'a' and <= 'z';
+				private static char ZQ(string str)
+					=> str.SingleOrDefault(DummyPredicate);
+				private static char ZR(string str)
+					=> str.Single(DummyPredicate);
+				private static long ZS(string str)
+					=> str.LongCount(DummyPredicate);
+				private static char ZT(string str)
+					=> str.LastOrDefault(DummyPredicate);
+				private static char ZU(string str)
+					=> str.Last(DummyPredicate);
+				private static char ZV(string str)
+					=> str.FirstOrDefault(DummyPredicate);
+				private static char ZW(string str)
+					=> str.First(DummyPredicate);
+				private static int ZX(string str)
+					=> str.Count(DummyPredicate);
+				private static bool ZY(string str)
+					=> str.Any(DummyPredicate);
+				private static bool ZZ(string str)
+					=> str.All(DummyPredicate);
+				private static bool AA(string str)
+					=> {|BHI3102:str.Any()|};
+				private static IEnumerable<char> AB(string str)
+					=> {|BHI3102:str.Append('.')|};
+				private static IEnumerable<char> AC(string str)
+					=> {|BHI3102:str.Concat("-_-")|};
+				private static IEnumerable<char> AD(string str)
+					=> {|BHI3102:str.Concat(new[] { '-', '_', '-' })|};
+				private static bool AE(string str)
+					=> {|BHI3102:Enumerable.Contains(str, '.')|};
+				private static int AF(string str)
+					=> {|BHI3102:str.Count()|};
+				private static IEnumerable<char> AG(string str)
+					=> {|BHI3102:str.DefaultIfEmpty()|};
+				private static IEnumerable<char> AH(string str)
+					=> {|BHI3102:str.DefaultIfEmpty('.')|};
+				private static char AI(string str)
+					=> {|BHI3102:str.ElementAt(2)|};
+				private static char AJ(string str)
+					=> {|BHI3102:str.ElementAtOrDefault(2)|};
+				private static char AK(string str)
+					=> {|BHI3102:str.First()|};
+				private static char AL(string str)
+					=> {|BHI3102:str.FirstOrDefault()|};
+				private static char AM(string str)
+					=> {|BHI3102:str.Last()|};
+				private static char AN(string str)
+					=> {|BHI3102:str.LastOrDefault()|};
+				private static long AO(string str)
+					=> {|BHI3102:str.LongCount()|};
+				private static IEnumerable<char> AP(string str)
+					=> {|BHI3102:str.Prepend('.')|};
+				private static IEnumerable<char> AQ(string str)
+					=> {|BHI3102:str.Reverse()|};
+				private static char AR(string str)
+					=> {|BHI3102:str.Single()|};
+				private static char AS(string str)
+					=> {|BHI3102:str.SingleOrDefault()|};
+				private static IEnumerable<char> AT(string str)
+					=> {|BHI3102:str.Skip(2)|};
+				private static IEnumerable<char> AU(string str)
+					=> {|BHI3102:str.Take(2)|};
+				private static char[] AV(string str)
+					=> {|BHI3102:str.ToArray()|};
+			}
+			namespace BizHawk.Common.StringExtensions {
+				public static class StringExtensions {} // Analyzer does more checks if this exists
+			}
+		""");
+}

src/BizHawk.Client.Common/cheats/GbaGameSharkDecoder.cs

@@ -1,5 +1,4 @@
 using System.Globalization;
-using System.Linq;
 
 #pragma warning disable MA0089
 namespace BizHawk.Client.Common.cheats
@@ -64,7 +63,7 @@ namespace BizHawk.Client.Common.cheats
 
 			var result = new DecodeResult
 			{
-				Size = code.First() switch
+				Size = code[0] switch
 				{
 					'0' => WatchSize.Byte,
 					'1' => WatchSize.Word,

src/BizHawk.Client.Common/lua/LuaDocumentation.cs

@@ -146,7 +146,7 @@ namespace BizHawk.Client.Common
 
 				var sb = new StringBuilder();
 
-				if (f.ParameterList.Any())
+				if (f.ParameterList.Length is not 0)
 				{
 					sb
 						.Append($"{f.Library}.{f.Name}(");

src/BizHawk.Client.Common/movie/import/DsmImport.cs

@@ -1,5 +1,5 @@
 using System.Globalization;
-using System.Linq;
+
 using BizHawk.Common;
 using BizHawk.Common.StringExtensions;
 using BizHawk.Emulation.Common;
@@ -109,7 +109,7 @@ namespace BizHawk.Client.Common
 
 			if (sections.Length > 1)
 			{
-				var mnemonics = sections[1].Take(_buttons.Length).ToList();
+				var mnemonics = sections[1].AsSpan(start: 0, length: _buttons.Length);
 
 				controller["Right"] = mnemonics[0] != '.';
 				controller["Left"] = mnemonics[1] != '.';

src/BizHawk.Client.EmuHawk/tools/HexEditor/HexEditor.cs

@@ -1246,7 +1246,7 @@ namespace BizHawk.Client.EmuHawk
 				}
 
 				var parts = line.Split('=');
-				_textTable.Add(int.Parse(parts[0], NumberStyles.HexNumber), parts[1].First());
+				_textTable.Add(int.Parse(parts[0], NumberStyles.HexNumber), parts[1][0]);
 			}
 
 			return true;

src/BizHawk.Client.EmuHawk/tools/Lua/LuaConsole.cs

@@ -1205,11 +1205,8 @@ namespace BizHawk.Client.EmuHawk
 		private void ConsoleContextMenu_Opening(object sender, CancelEventArgs e)
 		{
 			RegisteredFunctionsContextItem.Enabled = LuaImp.RegisteredFunctions.Any();
-			CopyContextItem.Enabled = OutputBox.SelectedText.Any();
-			ClearConsoleContextItem.Enabled =
-				SelectAllContextItem.Enabled =
-				OutputBox.Text.Any();
-
+			CopyContextItem.Enabled = OutputBox.SelectedText.Length is not 0;
+			ClearConsoleContextItem.Enabled = SelectAllContextItem.Enabled = OutputBox.Text.Length is not 0;
 			ClearRegisteredFunctionsLogContextItem.Enabled =
 				LuaImp.RegisteredFunctions.Any();
 		}

src/BizHawk.Emulation.Cores/vpads_schemata/PSXSchema.cs

@@ -1,6 +1,5 @@
 using System.Collections.Generic;
 using System.Drawing;
-using System.Linq;
 
 using BizHawk.Common;
 using BizHawk.Emulation.Common;
@@ -48,7 +47,7 @@ namespace BizHawk.Emulation.Cores
 			{
 				foreach (var result in nyma.ActualPortData)
 				{
-					var num = int.Parse(result.Port.ShortName.Last().ToString());
+					var num = int.Parse(result.Port.ShortName[^1].ToString());
 					var device = result.Device.ShortName;
 					if (device is "none") continue;
 					yield return device switch

src/BizHawk.Emulation.Cores/vpads_schemata/PceSchema.cs

@@ -87,7 +87,7 @@ namespace BizHawk.Emulation.Cores
 		{
 			foreach (NymaCore.PortResult result in nyma.ActualPortData)
 			{
-				var num = int.Parse(result.Port.ShortName.Last().ToString());
+				var num = int.Parse(result.Port.ShortName[^1].ToString());
 				var device = result.Device.ShortName;
 				if (device == "gamepad")
 				{

src/BizHawk.Emulation.Cores/vpads_schemata/PcfxSchema.cs

@@ -1,6 +1,5 @@
 using System.Collections.Generic;
 using System.Drawing;
-using System.Linq;
 
 using BizHawk.Common;
 using BizHawk.Emulation.Common;
@@ -21,7 +20,7 @@ namespace BizHawk.Emulation.Cores
 		{
 			foreach (NymaCore.PortResult result in nyma.ActualPortData)
 			{
-				var num = int.Parse(result.Port.ShortName.Last().ToString());
+				var num = int.Parse(result.Port.ShortName[^1].ToString());
 				var device = result.Device.ShortName;
 				if (device == "gamepad")
 				{

src/BizHawk.Emulation.Cores/vpads_schemata/SaturnSchema.cs

@@ -19,7 +19,7 @@ namespace BizHawk.Emulation.Cores
 			foreach (var result in nyma.ActualPortData
 				.Where(r => r.Port.ShortName != "builtin"))
 			{
-				var num = int.Parse(result.Port.ShortName.Last().ToString());
+				var num = int.Parse(result.Port.ShortName[^1].ToString());
 				var device = result.Device.ShortName;
 				var schema = GenerateSchemaForPort(device, num, showMessageBox);
 				if (schema != null)

src/BizHawk.Emulation.Cores/vpads_schemata/SnesSchema.cs

@@ -176,7 +176,7 @@ namespace BizHawk.Emulation.Cores
 		{
 			foreach (NymaCore.PortResult result in nyma.ActualPortData)
 			{
-				var num = int.Parse(result.Port.ShortName.Last().ToString());
+				var num = int.Parse(result.Port.ShortName[^1].ToString());
 				var device = result.Device.ShortName;
 				if (device == "gamepad")
 				{