From 577f17eb1b63edea86b9d9326b956485712f9e10 Mon Sep 17 00:00:00 2001 From: goat Date: Thu, 11 Jul 2024 00:14:15 +0200 Subject: [PATCH] add solution-wide code analysis config Disabled for now, since it'll be a bit of a refactor to enable them --- Dalamud.sln | 3 ++ Dalamud/Utility/TaskExtensions.cs | 75 +++++++++++++++++++++++++++++++ Directory.Build.props | 12 +++++ lib/Directory.Build.props | 3 ++ tools/BannedSymbols.txt | 13 ++++++ tools/dalamud.ruleset | 6 +++ 6 files changed, 112 insertions(+) create mode 100644 Dalamud/Utility/TaskExtensions.cs create mode 100644 Directory.Build.props create mode 100644 lib/Directory.Build.props create mode 100644 tools/BannedSymbols.txt create mode 100644 tools/dalamud.ruleset diff --git a/Dalamud.sln b/Dalamud.sln index 320e1a7a2..22cc59a8d 100644 --- a/Dalamud.sln +++ b/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}" diff --git a/Dalamud/Utility/TaskExtensions.cs b/Dalamud/Utility/TaskExtensions.cs new file mode 100644 index 000000000..a65956325 --- /dev/null +++ b/Dalamud/Utility/TaskExtensions.cs @@ -0,0 +1,75 @@ +// Copyright (c) 2024 ppy Pty Ltd . +// 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; + +/// +/// Extension methods to make working with easier. +/// +public static class TaskExtensions +{ + /// + /// Safe alternative to Task.Wait which ensures the calling thread is not a thread pool thread. + /// + /// The task to be awaited. + 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 + } + + /// + /// Safe alternative to Task.Result which ensures the calling thread is not a thread pool thread. + /// + /// The target task. + /// The type of the result. + /// The result. + public static T GetResultSafely(this Task 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; + } +} diff --git a/Directory.Build.props b/Directory.Build.props new file mode 100644 index 000000000..109deba65 --- /dev/null +++ b/Directory.Build.props @@ -0,0 +1,12 @@ + + + + diff --git a/lib/Directory.Build.props b/lib/Directory.Build.props new file mode 100644 index 000000000..f719442cd --- /dev/null +++ b/lib/Directory.Build.props @@ -0,0 +1,3 @@ + + + diff --git a/tools/BannedSymbols.txt b/tools/BannedSymbols.txt new file mode 100644 index 000000000..89cf843f4 --- /dev/null +++ b/tools/BannedSymbols.txt @@ -0,0 +1,13 @@ +M:System.Object.Equals(System.Object,System.Object)~System.Boolean;Don't use object.Equals. Use IEquatable or EqualityComparer.Default instead. +M:System.Object.Equals(System.Object)~System.Boolean;Don't use object.Equals. Use IEquatable or EqualityComparer.Default instead. +M:System.ValueType.Equals(System.Object)~System.Boolean;Don't use object.Equals(Fallbacks to ValueType). Use IEquatable or EqualityComparer.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. diff --git a/tools/dalamud.ruleset b/tools/dalamud.ruleset new file mode 100644 index 000000000..f3abde04e --- /dev/null +++ b/tools/dalamud.ruleset @@ -0,0 +1,6 @@ + + + + + +