add solution-wide code analysis config

Disabled for now, since it'll be a bit of a refactor to enable them

Dalamud.sln

@@ -8,6 +8,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 		.gitignore = .gitignore
 		targets\Dalamud.Plugin.Bootstrap.targets = targets\Dalamud.Plugin.Bootstrap.targets
 		targets\Dalamud.Plugin.targets = targets\Dalamud.Plugin.targets
+		Directory.Build.props = Directory.Build.props
+		tools\BannedSymbols.txt = tools\BannedSymbols.txt
+		tools\dalamud.ruleset = tools\dalamud.ruleset
 	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "build", "build\build.csproj", "{94E5B016-02B1-459B-97D9-E783F28764B2}"

Dalamud/Utility/TaskExtensions.cs

@@ -0,0 +1,75 @@
+// Copyright (c) 2024 ppy Pty Ltd <contact@ppy.sh>.
+// Copyright (c) 2024 XIVLauncher & Dalamud contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Dalamud.Utility;
+
+/// <summary>
+/// Extension methods to make working with <see cref="Task"/> easier.
+/// </summary>
+public static class TaskExtensions
+{
+    /// <summary>
+    /// Safe alternative to Task.Wait which ensures the calling thread is not a thread pool thread.
+    /// </summary>
+    /// <param name="task">The task to be awaited.</param>
+    public static void WaitSafely(this Task task)
+    {
+        if (!IsWaitingValid(task))
+            throw new InvalidOperationException($"Can't use {nameof(WaitSafely)} from inside an async operation.");
+
+#pragma warning disable RS0030
+        task.Wait();
+#pragma warning restore RS0030
+    }
+
+    /// <summary>
+    /// Safe alternative to Task.Result which ensures the calling thread is not a thread pool thread.
+    /// </summary>
+    /// <param name="task">The target task.</param>
+    /// <typeparam name="T">The type of the result.</typeparam>
+    /// <returns>The result.</returns>
+    public static T GetResultSafely<T>(this Task<T> task)
+    {
+        // We commonly access `.Result` from within `ContinueWith`, which is a safe usage (the task is guaranteed to be completed).
+        // Unfortunately, the only way to allow these usages is to check whether the task is completed or not here.
+        // This does mean that there could be edge cases where this safety is skipped (ie. if the majority of executions complete
+        // immediately).
+        if (!task.IsCompleted && !IsWaitingValid(task))
+            throw new InvalidOperationException($"Can't use {nameof(GetResultSafely)} from inside an async operation.");
+
+#pragma warning disable RS0030
+        return task.Result;
+#pragma warning restore RS0030
+    }
+
+    private static bool IsWaitingValid(Task task)
+    {
+        // In the case the task has been started with the LongRunning flag, it will not be in the TPL thread pool and we can allow waiting regardless.
+        if (task.CreationOptions.HasFlag(TaskCreationOptions.LongRunning))
+            return true;
+
+        // Otherwise only allow waiting from a non-TPL thread pool thread.
+        return !Thread.CurrentThread.IsThreadPoolThread;
+    }
+}

Directory.Build.props

@@ -0,0 +1,12 @@
+<!-- Code analysis settings for all Dalamud projects. -->
+<Project>
+    <!--
+    <ItemGroup Label="Code Analysis">
+        <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.4" PrivateAssets="All" />
+        <AdditionalFiles Include="$(MSBuildThisFileDirectory)tools\BannedSymbols.txt" />
+    </ItemGroup>
+    <PropertyGroup Label="Code Analysis">
+        <CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)tools\dalamud.ruleset</CodeAnalysisRuleSet>
+    </PropertyGroup>
+    -->
+</Project>

lib/Directory.Build.props

@@ -0,0 +1,3 @@
+<!-- This is left empty on purpose to avoid having Directory.Build.props in the root of the repository leak into dependencies. -->
+<Project>
+</Project>

tools/BannedSymbols.txt

@@ -0,0 +1,13 @@
+M:System.Object.Equals(System.Object,System.Object)~System.Boolean;Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
+M:System.Object.Equals(System.Object)~System.Boolean;Don't use object.Equals. Use IEquatable<T> or EqualityComparer<T>.Default instead.
+M:System.ValueType.Equals(System.Object)~System.Boolean;Don't use object.Equals(Fallbacks to ValueType). Use IEquatable<T> or EqualityComparer<T>.Default instead.
+M:System.Nullable`1.Equals(System.Object)~System.Boolean;Use == instead.
+T:System.IComparable;Don't use non-generic IComparable. Use generic version instead.
+M:System.Guid.#ctor;Probably meaning to use Guid.NewGuid() instead. If actually wanting empty, use Guid.Empty.
+M:System.Threading.Tasks.Task.Wait();Don't use Task.Wait. Use Task.WaitSafely() to ensure we avoid deadlocks.
+P:System.Threading.Tasks.Task`1.Result;Don't use Task.Result. Use Task.GetResultSafely() to ensure we avoid deadlocks.
+M:System.Threading.ManualResetEventSlim.Wait();Specify a timeout to avoid waiting forever.
+M:System.Char.ToLower(System.Char);char.ToLower() changes behaviour depending on CultureInfo.CurrentCulture. Use char.ToLowerInvariant() instead. If wanting culture-sensitive behaviour, explicitly provide CultureInfo.CurrentCulture.
+M:System.Char.ToUpper(System.Char);char.ToUpper() changes behaviour depending on CultureInfo.CurrentCulture. Use char.ToUpperInvariant() instead. If wanting culture-sensitive behaviour, explicitly provide CultureInfo.CurrentCulture.
+M:System.String.ToLower();string.ToLower() changes behaviour depending on CultureInfo.CurrentCulture. Use string.ToLowerInvariant() instead. If wanting culture-sensitive behaviour, explicitly provide CultureInfo.CurrentCulture or use LocalisableString.
+M:System.String.ToUpper();string.ToUpper() changes behaviour depending on CultureInfo.CurrentCulture. Use string.ToUpperInvariant() instead. If wanting culture-sensitive behaviour, explicitly provide CultureInfo.CurrentCulture or use LocalisableString.

tools/dalamud.ruleset

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<RuleSet Name="Dalamud Rule Set" Description=" " ToolsVersion="16.0">
+    <Rules AnalyzerId="Microsoft.CodeQuality.Analyzers" RuleNamespace="Microsoft.CodeQuality.Analyzers">
+        <Rule Id="CA2007" Action="Warning" /> <!-- Do not directly await a Task -->
+    </Rules>
+</RuleSet>