From 8f6e5c475465d826204f5308f4b431c4a52db253 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 19:14:18 +0800 Subject: [PATCH 01/11] Convert legacy ruleset to globalconfig --- .globalconfig | 8 +++ CodeAnalysis/osu.ruleset | 58 ------------------- Directory.Build.props | 1 - .../CodeAnalysis.tests.globalconfig | 7 +++ osu.Game.Tests/osu.Game.Tests.csproj | 6 +- osu.Game.Tests/tests.ruleset | 6 -- osu.sln | 2 +- 7 files changed, 19 insertions(+), 69 deletions(-) delete mode 100644 CodeAnalysis/osu.ruleset create mode 100644 osu.Game.Tests/CodeAnalysis.tests.globalconfig delete mode 100644 osu.Game.Tests/tests.ruleset diff --git a/.globalconfig b/.globalconfig index a4d4707f9b..c5723daed6 100644 --- a/.globalconfig +++ b/.globalconfig @@ -46,6 +46,14 @@ dotnet_diagnostic.IDE0130.severity = warning # IDE1006: Naming style dotnet_diagnostic.IDE1006.severity = warning +dotnet_diagnostic.CA1309.severity = warning + +# CA2007: Consider calling ConfigureAwait on the awaited task +dotnet_diagnostic.CA2007.severity = warning + +# CA2201: Do not raise reserved exception types +dotnet_diagnostic.CA2201.severity = warning + #Disable operator overloads requiring alternate named methods dotnet_diagnostic.CA2225.severity = none diff --git a/CodeAnalysis/osu.ruleset b/CodeAnalysis/osu.ruleset deleted file mode 100644 index 6a99e230d1..0000000000 --- a/CodeAnalysis/osu.ruleset +++ /dev/null @@ -1,58 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/Directory.Build.props b/Directory.Build.props index 5ba12b845b..e7e5e4e831 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -20,7 +20,6 @@ - $(MSBuildThisFileDirectory)CodeAnalysis\osu.ruleset true diff --git a/osu.Game.Tests/CodeAnalysis.tests.globalconfig b/osu.Game.Tests/CodeAnalysis.tests.globalconfig new file mode 100644 index 0000000000..3f039736ec --- /dev/null +++ b/osu.Game.Tests/CodeAnalysis.tests.globalconfig @@ -0,0 +1,7 @@ +# Higher global_level has higher priority, the default global_level +# is 100 for root .globalconfig and 0 for others +# https://learn.microsoft.com/dotnet/fundamentals/code-analysis/configuration-files#precedence +is_global = true +global_level = 101 + +dotnet_diagnostic.CA2007.severity = none diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 28a1d4d021..01d2241650 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -12,9 +12,9 @@ WinExe net8.0 - - tests.ruleset - + + + diff --git a/osu.Game.Tests/tests.ruleset b/osu.Game.Tests/tests.ruleset deleted file mode 100644 index a0abb781d3..0000000000 --- a/osu.Game.Tests/tests.ruleset +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/osu.sln b/osu.sln index 829e43fc65..2d9a4e86d0 100644 --- a/osu.sln +++ b/osu.sln @@ -60,7 +60,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Directory.Build.props = Directory.Build.props osu.Android.props = osu.Android.props osu.iOS.props = osu.iOS.props - CodeAnalysis\osu.ruleset = CodeAnalysis\osu.ruleset + global.json = global.json osu.sln.DotSettings = osu.sln.DotSettings osu.TestProject.props = osu.TestProject.props EndProjectSection From c57ace0b5f184491890ab566084e4a5b9ec9d790 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 19:40:09 +0800 Subject: [PATCH 02/11] Enable recommended rules for documentation --- Directory.Build.props | 3 +++ osu.Game/Database/RealmObjectExtensions.cs | 22 ++++++++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index e7e5e4e831..60e0334f97 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -20,6 +20,9 @@ + Default + Default + Recommended true diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index 2fa3b8a880..df725505fc 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -248,29 +248,30 @@ namespace osu.Game.Database return new RealmLive(realmObject, realm); } +#pragma warning disable RS0030 // mentioning banned symbols in documentation /// - /// Register a callback to be invoked each time this changes. + /// Register a callback to be invoked each time this changes. /// /// /// /// This adds osu! specific thread and managed state safety checks on top of . /// /// - /// The first callback will be invoked with the initial after the asynchronous query completes, + /// The first callback will be invoked with the initial after the asynchronous query completes, /// and then called again after each write transaction which changes either any of the objects in the collection, or /// which objects are in the collection. The changes parameter will /// be null the first time the callback is invoked with the initial results. For each call after that, /// it will contain information about which rows in the results were added, removed or modified. /// /// - /// If a write transaction did not modify any objects in this , the callback is not invoked at all. + /// If a write transaction did not modify any objects in this , the callback is not invoked at all. /// If an error occurs the callback will be invoked with null for the sender parameter and a non-null error. - /// Currently the only errors that can occur are when opening the on the background worker thread. + /// Currently the only errors that can occur are when opening the on the background worker thread. /// /// - /// At the time when the block is called, the object will be fully evaluated + /// At the time when the block is called, the object will be fully evaluated /// and up-to-date, and as long as you do not perform a write transaction on the same thread - /// or explicitly call , accessing it will never perform blocking work. + /// or explicitly call , accessing it will never perform blocking work. /// /// /// Notifications are delivered via the standard event loop, and so can't be delivered while the event loop is blocked by other activity. @@ -279,13 +280,14 @@ namespace osu.Game.Database /// /// /// The to observe for changes. - /// The callback to be invoked with the updated . + /// The callback to be invoked with the updated . /// /// A subscription token. It must be kept alive for as long as you want to receive change notifications. - /// To stop receiving notifications, call . + /// To stop receiving notifications, call . /// - /// - /// + /// + /// +#pragma warning restore RS0030 public static IDisposable QueryAsyncWithNotifications(this IRealmCollection collection, NotificationCallbackDelegate callback) where T : RealmObjectBase { From cd9b5927eba9b19b4b66382aaedd25298a91a4df Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 19:46:25 +0800 Subject: [PATCH 03/11] Enable recommended rules for globalization --- .globalconfig | 8 +++++++- CodeAnalysis/BannedSymbols.txt | 4 ---- Directory.Build.props | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.globalconfig b/.globalconfig index c5723daed6..d6c657bad0 100644 --- a/.globalconfig +++ b/.globalconfig @@ -46,11 +46,17 @@ dotnet_diagnostic.IDE0130.severity = warning # IDE1006: Naming style dotnet_diagnostic.IDE1006.severity = warning -dotnet_diagnostic.CA1309.severity = warning +# CA1305: Specify IFormatProvider +# Too many noisy warnings for parsing/formatting numbers +dotnet_diagnostic.CA1305.severity = none # CA2007: Consider calling ConfigureAwait on the awaited task dotnet_diagnostic.CA2007.severity = warning +# CA2101: Specify marshaling for P/Invoke string arguments +# Reports warning for all non-UTF16 usages on DllImport; consider migrating to LibraryImport +dotnet_diagnostic.CA2101.severity = none + # CA2201: Do not raise reserved exception types dotnet_diagnostic.CA2201.severity = warning diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt index 3c60b28765..550f7c8e11 100644 --- a/CodeAnalysis/BannedSymbols.txt +++ b/CodeAnalysis/BannedSymbols.txt @@ -14,10 +14,6 @@ M:Realms.CollectionExtensions.SubscribeForNotifications`1(System.Collections.Gen 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. M:Humanizer.InflectorExtensions.Pascalize(System.String);Humanizer's .Pascalize() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToPascalCase() instead. M:Humanizer.InflectorExtensions.Camelize(System.String);Humanizer's .Camelize() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToCamelCase() instead. M:Humanizer.InflectorExtensions.Underscore(System.String);Humanizer's .Underscore() extension method changes behaviour depending on CultureInfo.CurrentCulture. Use StringDehumanizeExtensions.ToSnakeCase() instead. diff --git a/Directory.Build.props b/Directory.Build.props index 60e0334f97..f89696d867 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -23,6 +23,8 @@ Default Default Recommended + Recommended + Recommended true From 13d7c6a2d863b9bae4ae024e4fd59ac2f895c292 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 19:55:03 +0800 Subject: [PATCH 04/11] Enable recommended rules for maintainability --- Directory.Build.props | 2 ++ osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs | 2 ++ .../API/Requests/Responses/APIUserMostPlayedBeatmap.cs | 2 ++ .../Notifications/WebSocket/Events/NewChatMessageData.cs | 2 ++ osu.Game/Utils/SpecialFunctions.cs | 5 +---- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index f89696d867..7cd02a72d4 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -25,6 +25,8 @@ Recommended Recommended Recommended + Recommended + Default true diff --git a/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs b/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs index 583def8eda..8e4cc387ed 100644 --- a/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs +++ b/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs @@ -45,7 +45,9 @@ namespace osu.Game.Online.API.Requests.Responses public KudosuAction Action; +#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty("action")] +#pragma warning restore CA1507 private string action { set diff --git a/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs b/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs index 6d5fd59f9c..38ad2bd02d 100644 --- a/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs +++ b/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs @@ -15,7 +15,9 @@ namespace osu.Game.Online.API.Requests.Responses [JsonProperty("count")] public int PlayCount { get; set; } +#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty("beatmap")] +#pragma warning restore CA1507 private APIBeatmap beatmap { get; set; } public APIBeatmap BeatmapInfo diff --git a/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs b/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs index ff9f5ee9f7..677286bb8a 100644 --- a/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs +++ b/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs @@ -19,7 +19,9 @@ namespace osu.Game.Online.Notifications.WebSocket.Events [JsonProperty(@"messages")] public List Messages { get; set; } = null!; +#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty(@"users")] +#pragma warning restore CA1507 private List users { get; set; } = null!; [OnDeserialized] diff --git a/osu.Game/Utils/SpecialFunctions.cs b/osu.Game/Utils/SpecialFunctions.cs index 0b0f0598bb..795a84a973 100644 --- a/osu.Game/Utils/SpecialFunctions.cs +++ b/osu.Game/Utils/SpecialFunctions.cs @@ -666,10 +666,7 @@ namespace osu.Game.Utils { // 2020-10-07 jbialogrodzki #730 Since this is public API we should probably // handle null arguments? It doesn't seem to have been done consistently in this class though. - if (coefficients == null) - { - throw new ArgumentNullException(nameof(coefficients)); - } + ArgumentNullException.ThrowIfNull(coefficients); // 2020-10-07 jbialogrodzki #730 Zero polynomials need explicit handling. // Without this check, we attempted to peek coefficients at negative indices! From f5a77165096fd395a2df7d8e3656bf794a7db2aa Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 20:37:53 +0800 Subject: [PATCH 05/11] Apply minor performance rules --- .globalconfig | 16 ++++++++++++++++ Directory.Build.props | 1 + .../TestSceneMultiplayerParticipantsList.cs | 2 +- .../UserInterface/TestSceneDeleteLocalScore.cs | 2 +- .../Extensions/StringDehumanizeExtensions.cs | 2 +- .../Overlays/Chat/ChannelList/ChannelList.cs | 11 ++--------- osu.Game/Overlays/ChatOverlay.cs | 3 +-- osu.Game/Overlays/Comments/CommentsContainer.cs | 2 +- osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs | 4 ++-- .../Objects/Legacy/ConvertHitObjectParser.cs | 2 +- .../Ranking/Statistics/User/OverallRanking.cs | 4 ++-- osu.Game/Skinning/SkinImporter.cs | 2 +- osu.Game/Storyboards/Storyboard.cs | 3 ++- .../Visual/Spectator/TestSpectatorClient.cs | 4 ++-- 14 files changed, 34 insertions(+), 24 deletions(-) diff --git a/.globalconfig b/.globalconfig index d6c657bad0..e218f46cd2 100644 --- a/.globalconfig +++ b/.globalconfig @@ -50,6 +50,22 @@ dotnet_diagnostic.IDE1006.severity = warning # Too many noisy warnings for parsing/formatting numbers dotnet_diagnostic.CA1305.severity = none +# CA1806: Do not ignore method results +# The usages for numeric parsing are explicitly optional +dotnet_diagnostic.CA1806.severity = suggestion + +# CA1822: Mark members as static +# Potential false positive around reflection/too much noise +dotnet_diagnostic.CA1822.severity = none + +# CA1859: Use concrete types when possible for improved performance +# Involves design considerations +dotnet_diagnostic.CA1859.severity = none + +# CA1861: Avoid constant arrays as arguments +# Outdated with collection expressions +dotnet_diagnostic.CA1861.severity = none + # CA2007: Consider calling ConfigureAwait on the awaited task dotnet_diagnostic.CA2007.severity = warning diff --git a/Directory.Build.props b/Directory.Build.props index 7cd02a72d4..0f982d496e 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -27,6 +27,7 @@ Recommended Recommended Default + Minimum true diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs index 95ae4c5e80..d88741ec0c 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs @@ -198,7 +198,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("make second user host", () => MultiplayerClient.TransferHost(3)); - AddUntilStep("kick buttons not visible", () => this.ChildrenOfType().Count(d => d.IsPresent) == 0); + AddUntilStep("kick buttons not visible", () => !this.ChildrenOfType().Any(d => d.IsPresent)); AddStep("make local user host again", () => MultiplayerClient.TransferHost(API.LocalUser.Value.Id)); diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs index e2fe10fa74..f7bdda6b57 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs @@ -151,7 +151,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("click delete option", () => { - InputManager.MoveMouseTo(contextMenuContainer.ChildrenOfType().First(i => i.Item.Text.Value.ToString().ToLowerInvariant() == "delete")); + InputManager.MoveMouseTo(contextMenuContainer.ChildrenOfType().First(i => string.Equals(i.Item.Text.Value.ToString(), "delete", System.StringComparison.OrdinalIgnoreCase))); InputManager.Click(MouseButton.Left); }); diff --git a/osu.Game/Extensions/StringDehumanizeExtensions.cs b/osu.Game/Extensions/StringDehumanizeExtensions.cs index 6f0d7622d3..5993f83b55 100644 --- a/osu.Game/Extensions/StringDehumanizeExtensions.cs +++ b/osu.Game/Extensions/StringDehumanizeExtensions.cs @@ -60,7 +60,7 @@ namespace osu.Game.Extensions public static string ToCamelCase(this string input) { string word = input.ToPascalCase(); - return word.Length > 0 ? word.Substring(0, 1).ToLowerInvariant() + word.Substring(1) : word; + return word.Length > 0 ? char.ToLowerInvariant(word[0]) + word.Substring(1) : word; } /// diff --git a/osu.Game/Overlays/Chat/ChannelList/ChannelList.cs b/osu.Game/Overlays/Chat/ChannelList/ChannelList.cs index fc0060d86a..39860b5e03 100644 --- a/osu.Game/Overlays/Chat/ChannelList/ChannelList.cs +++ b/osu.Game/Overlays/Chat/ChannelList/ChannelList.cs @@ -120,10 +120,9 @@ namespace osu.Game.Overlays.Chat.ChannelList public void RemoveChannel(Channel channel) { - if (!channelMap.ContainsKey(channel)) + if (!channelMap.TryGetValue(channel, out var item)) return; - ChannelListItem item = channelMap[channel]; FillFlowContainer flow = getFlowForChannel(channel); channelMap.Remove(channel); @@ -132,13 +131,7 @@ namespace osu.Game.Overlays.Chat.ChannelList updateVisibility(); } - public ChannelListItem GetItem(Channel channel) - { - if (!channelMap.ContainsKey(channel)) - throw new ArgumentOutOfRangeException(); - - return channelMap[channel]; - } + public ChannelListItem GetItem(Channel channel) => channelMap[channel]; public void ScrollChannelIntoView(Channel channel) => scroll.ScrollIntoView(GetItem(channel)); diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index b11483e678..a00414522d 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -386,9 +386,8 @@ namespace osu.Game.Overlays { channelList.RemoveChannel(channel); - if (loadedChannels.ContainsKey(channel)) + if (loadedChannels.TryGetValue(channel, out var loaded)) { - DrawableChannel loaded = loadedChannels[channel]; loadedChannels.Remove(channel); // DrawableChannel removed from cache must be manually disposed loaded.Dispose(); diff --git a/osu.Game/Overlays/Comments/CommentsContainer.cs b/osu.Game/Overlays/Comments/CommentsContainer.cs index 921c1682f5..5e277357a9 100644 --- a/osu.Game/Overlays/Comments/CommentsContainer.cs +++ b/osu.Game/Overlays/Comments/CommentsContainer.cs @@ -244,7 +244,7 @@ namespace osu.Game.Overlays.Comments protected void OnSuccess(CommentBundle response) { commentCounter.Current.Value = response.Total; - newCommentEditor.CommentableMeta.Value = response.CommentableMeta.SingleOrDefault(m => m.Id == id.Value && m.Type == type.Value.ToString().ToSnakeCase().ToLowerInvariant()); + newCommentEditor.CommentableMeta.Value = response.CommentableMeta.SingleOrDefault(m => m.Id == id.Value && string.Equals(m.Type, type.Value.ToString().ToSnakeCase(), StringComparison.OrdinalIgnoreCase)); if (!response.Comments.Any()) { diff --git a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs index 4ca937bf86..fb056b457b 100644 --- a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs +++ b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs @@ -173,10 +173,10 @@ namespace osu.Game.Rulesets.Mods }; drawable.OnRevertResult += (_, result) => { - if (!ratesForRewinding.ContainsKey(result.HitObject)) return; + if (!ratesForRewinding.TryGetValue(result.HitObject, out double rewindValue)) return; if (!shouldProcessResult(result)) return; - recentRates.Insert(0, ratesForRewinding[result.HitObject]); + recentRates.Insert(0, rewindValue); ratesForRewinding.Remove(result.HitObject); recentRates.RemoveAt(recentRates.Count - 1); diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index f8bc0ce112..0162c8017b 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -243,7 +243,7 @@ namespace osu.Game.Rulesets.Objects.Legacy return PathType.CATMULL; case 'B': - if (input.Length > 1 && int.TryParse(input.Substring(1), out int degree) && degree > 0) + if (input.Length > 1 && int.TryParse(input.AsSpan(1), out int degree) && degree > 0) return PathType.BSpline(degree); return PathType.BEZIER; diff --git a/osu.Game/Screens/Ranking/Statistics/User/OverallRanking.cs b/osu.Game/Screens/Ranking/Statistics/User/OverallRanking.cs index 171a3f0f65..9f5afea6f0 100644 --- a/osu.Game/Screens/Ranking/Statistics/User/OverallRanking.cs +++ b/osu.Game/Screens/Ranking/Statistics/User/OverallRanking.cs @@ -59,14 +59,14 @@ namespace osu.Game.Screens.Ranking.Statistics.User new SimpleStatisticTable.Spacer(), new PerformancePointsChangeRow { StatisticsUpdate = { BindTarget = StatisticsUpdate } }, }, - new Drawable[] { }, + [], new Drawable[] { new MaximumComboChangeRow { StatisticsUpdate = { BindTarget = StatisticsUpdate } }, new SimpleStatisticTable.Spacer(), new AccuracyChangeRow { StatisticsUpdate = { BindTarget = StatisticsUpdate } }, }, - new Drawable[] { }, + [], new Drawable[] { new RankedScoreChangeRow { StatisticsUpdate = { BindTarget = StatisticsUpdate } }, diff --git a/osu.Game/Skinning/SkinImporter.cs b/osu.Game/Skinning/SkinImporter.cs index 59c7f0ba26..70d3195ecd 100644 --- a/osu.Game/Skinning/SkinImporter.cs +++ b/osu.Game/Skinning/SkinImporter.cs @@ -37,7 +37,7 @@ namespace osu.Game.Skinning protected override string[] HashableFileTypes => new[] { ".ini", ".json" }; - protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == @".osk"; + protected override bool ShouldDeleteArchive(string path) => string.Equals(Path.GetExtension(path), @".osk", StringComparison.OrdinalIgnoreCase); protected override SkinInfo CreateModel(ArchiveReader archive, ImportParameters parameters) => new SkinInfo { Name = archive.Name ?? @"No name" }; diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index 8c43b99702..5120757f3d 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -89,7 +90,7 @@ namespace osu.Game.Storyboards // Importantly, do this after the NullOrEmpty because EF may have stored the non-nullable value as null to the database, bypassing compile-time constraints. backgroundPath = backgroundPath.ToLowerInvariant(); - return GetLayer("Background").Elements.Any(e => e.Path.ToLowerInvariant() == backgroundPath); + return GetLayer("Background").Elements.Any(e => string.Equals(e.Path, backgroundPath, StringComparison.OrdinalIgnoreCase)); } } diff --git a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs index c27e7f15ca..5d33afd288 100644 --- a/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs +++ b/osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs @@ -78,12 +78,12 @@ namespace osu.Game.Tests.Visual.Spectator /// The spectator state to end play with. public void SendEndPlay(int userId, SpectatedUserState state = SpectatedUserState.Quit) { - if (!userBeatmapDictionary.ContainsKey(userId)) + if (!userBeatmapDictionary.TryGetValue(userId, out int beatmapId)) return; ((ISpectatorClient)this).UserFinishedPlaying(userId, new SpectatorState { - BeatmapID = userBeatmapDictionary[userId], + BeatmapID = beatmapId, RulesetID = 0, Mods = userModsDictionary[userId], State = state From 5b63d725c507b7e12cf111e3b3db9faf20cd0725 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 22:13:59 +0800 Subject: [PATCH 06/11] Mark CA1826/CA1860 as suggestion --- .globalconfig | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.globalconfig b/.globalconfig index e218f46cd2..7673e5afb0 100644 --- a/.globalconfig +++ b/.globalconfig @@ -58,13 +58,19 @@ dotnet_diagnostic.CA1806.severity = suggestion # Potential false positive around reflection/too much noise dotnet_diagnostic.CA1822.severity = none +# CA1826: Do not use Enumerable method on indexable collections +dotnet_diagnostic.CA1826.severity = suggestion + # CA1859: Use concrete types when possible for improved performance # Involves design considerations -dotnet_diagnostic.CA1859.severity = none +dotnet_diagnostic.CA1859.severity = suggestion + +# CA1860: Avoid using 'Enumerable.Any()' extension method +dotnet_diagnostic.CA1860.severity = suggestion # CA1861: Avoid constant arrays as arguments # Outdated with collection expressions -dotnet_diagnostic.CA1861.severity = none +dotnet_diagnostic.CA1861.severity = suggestion # CA2007: Consider calling ConfigureAwait on the awaited task dotnet_diagnostic.CA2007.severity = warning From 0a8ec4db2b6fc80cdcf6c9d5dc190b4e31a140d1 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 22:20:36 +0800 Subject: [PATCH 07/11] Enable recommended rules for reliability --- .globalconfig | 8 ++++++++ Directory.Build.props | 1 + 2 files changed, 9 insertions(+) diff --git a/.globalconfig b/.globalconfig index 7673e5afb0..7eec612775 100644 --- a/.globalconfig +++ b/.globalconfig @@ -75,6 +75,14 @@ dotnet_diagnostic.CA1861.severity = suggestion # CA2007: Consider calling ConfigureAwait on the awaited task dotnet_diagnostic.CA2007.severity = warning +# CA2016: Forward the 'CancellationToken' parameter to methods +# Some overloads are having special handling for debugger +dotnet_diagnostic.CA2016.severity = suggestion + +# CA2021: Do not call Enumerable.Cast or Enumerable.OfType with incompatible types +# Causing a lot of false positives with generics +dotnet_diagnostic.CA2021.severity = none + # CA2101: Specify marshaling for P/Invoke string arguments # Reports warning for all non-UTF16 usages on DllImport; consider migrating to LibraryImport dotnet_diagnostic.CA2101.severity = none diff --git a/Directory.Build.props b/Directory.Build.props index 0f982d496e..0dcbffd0dc 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -28,6 +28,7 @@ Recommended Default Minimum + Recommended true From fced2545944f401fe6cf7a8429eb97bc774824fc Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 28 Nov 2024 22:32:55 +0800 Subject: [PATCH 08/11] Enable selected rules for usage --- .globalconfig | 7 +++++-- Directory.Build.props | 2 ++ .../Argon/ManiaArgonSkinTransformer.cs | 20 +++++++++---------- osu.Game/Online/Leaderboards/Leaderboard.cs | 2 +- osu.Game/Online/OnlineViewContainer.cs | 2 +- osu.Game/Overlays/AccountCreationOverlay.cs | 2 +- .../Overlays/Toolbar/ToolbarUserButton.cs | 2 +- 7 files changed, 21 insertions(+), 16 deletions(-) diff --git a/.globalconfig b/.globalconfig index 7eec612775..e2cd1926f0 100644 --- a/.globalconfig +++ b/.globalconfig @@ -90,8 +90,11 @@ dotnet_diagnostic.CA2101.severity = none # CA2201: Do not raise reserved exception types dotnet_diagnostic.CA2201.severity = warning -#Disable operator overloads requiring alternate named methods -dotnet_diagnostic.CA2225.severity = none +# CA2208: Instantiate argument exceptions correctly +dotnet_diagnostic.CA2208.severity = suggestion + +# CA2242: Test for NaN correctly +dotnet_diagnostic.CA2242.severity = warning # Banned APIs dotnet_diagnostic.RS0030.severity = error diff --git a/Directory.Build.props b/Directory.Build.props index 0dcbffd0dc..0ab41d27a0 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -29,6 +29,8 @@ Default Minimum Recommended + Default + Default true diff --git a/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs index afccb2e568..c37c18081a 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Argon/ManiaArgonSkinTransformer.cs @@ -164,7 +164,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 1: return colour_cyan; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 3: @@ -176,7 +176,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 2: return colour_cyan; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 4: @@ -190,7 +190,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 3: return colour_purple; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 5: @@ -206,7 +206,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 4: return colour_cyan; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 6: @@ -224,7 +224,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 5: return colour_pink; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 7: @@ -244,7 +244,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 6: return colour_pink; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 8: @@ -266,7 +266,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 7: return colour_purple; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 9: @@ -290,7 +290,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 8: return colour_purple; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } case 10: @@ -316,7 +316,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 9: return colour_purple; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } } @@ -339,7 +339,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Argon case 5: return colour_green; - default: throw new ArgumentOutOfRangeException(); + default: throw new ArgumentOutOfRangeException(nameof(columnIndex)); } } } diff --git a/osu.Game/Online/Leaderboards/Leaderboard.cs b/osu.Game/Online/Leaderboards/Leaderboard.cs index 0fd9597ac0..d76da54adf 100644 --- a/osu.Game/Online/Leaderboards/Leaderboard.cs +++ b/osu.Game/Online/Leaderboards/Leaderboard.cs @@ -363,7 +363,7 @@ namespace osu.Game.Online.Leaderboards return null; default: - throw new ArgumentOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(state)); } } diff --git a/osu.Game/Online/OnlineViewContainer.cs b/osu.Game/Online/OnlineViewContainer.cs index 824da152b2..ce55b50d94 100644 --- a/osu.Game/Online/OnlineViewContainer.cs +++ b/osu.Game/Online/OnlineViewContainer.cs @@ -87,7 +87,7 @@ namespace osu.Game.Online break; default: - throw new ArgumentOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(state.NewValue)); } }); diff --git a/osu.Game/Overlays/AccountCreationOverlay.cs b/osu.Game/Overlays/AccountCreationOverlay.cs index 82fc5508f1..55cba33153 100644 --- a/osu.Game/Overlays/AccountCreationOverlay.cs +++ b/osu.Game/Overlays/AccountCreationOverlay.cs @@ -126,7 +126,7 @@ namespace osu.Game.Overlays break; default: - throw new ArgumentOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(state.NewValue)); } } } diff --git a/osu.Game/Overlays/Toolbar/ToolbarUserButton.cs b/osu.Game/Overlays/Toolbar/ToolbarUserButton.cs index 96c0b15c44..787c525566 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarUserButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarUserButton.cs @@ -130,7 +130,7 @@ namespace osu.Game.Overlays.Toolbar break; default: - throw new ArgumentOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(state.NewValue)); } }); } From 68f21709a8c314488d5c50e1502cc122ac8c756f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 30 Nov 2024 02:32:09 +0800 Subject: [PATCH 09/11] Fix CA1865 --- .../Visual/Gameplay/TestScenePlayerLocalScoreImport.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs index 1660f93384..046ae6d953 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs @@ -221,7 +221,7 @@ namespace osu.Game.Tests.Visual.Gameplay string? filePath = null; // Files starting with _ are temporary, created by CreateFileSafely call. - AddUntilStep("wait for export file", () => filePath = LocalStorage.GetFiles("exports").SingleOrDefault(f => !Path.GetFileName(f).StartsWith("_", StringComparison.Ordinal)), () => Is.Not.Null); + AddUntilStep("wait for export file", () => filePath = LocalStorage.GetFiles("exports").SingleOrDefault(f => !Path.GetFileName(f).StartsWith('_')), () => Is.Not.Null); AddUntilStep("filesize is non-zero", () => { try From 68f4fa5a5781d1f6d2344cf6041d39c51ef9bf05 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 3 Dec 2024 00:00:31 +0800 Subject: [PATCH 10/11] Turn off CA1507 --- .globalconfig | 4 ++++ osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs | 2 -- .../Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs | 2 -- .../Notifications/WebSocket/Events/NewChatMessageData.cs | 2 -- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.globalconfig b/.globalconfig index e2cd1926f0..ca7b86c778 100644 --- a/.globalconfig +++ b/.globalconfig @@ -50,6 +50,10 @@ dotnet_diagnostic.IDE1006.severity = warning # Too many noisy warnings for parsing/formatting numbers dotnet_diagnostic.CA1305.severity = none +# CA1507: Use nameof to express symbol names +# Flaggs serialization name attributes +dotnet_diagnostic.CA1507.severity = suggestion + # CA1806: Do not ignore method results # The usages for numeric parsing are explicitly optional dotnet_diagnostic.CA1806.severity = suggestion diff --git a/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs b/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs index 8e4cc387ed..583def8eda 100644 --- a/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs +++ b/osu.Game/Online/API/Requests/Responses/APIKudosuHistory.cs @@ -45,9 +45,7 @@ namespace osu.Game.Online.API.Requests.Responses public KudosuAction Action; -#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty("action")] -#pragma warning restore CA1507 private string action { set diff --git a/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs b/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs index 38ad2bd02d..6d5fd59f9c 100644 --- a/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs +++ b/osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs @@ -15,9 +15,7 @@ namespace osu.Game.Online.API.Requests.Responses [JsonProperty("count")] public int PlayCount { get; set; } -#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty("beatmap")] -#pragma warning restore CA1507 private APIBeatmap beatmap { get; set; } public APIBeatmap BeatmapInfo diff --git a/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs b/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs index 677286bb8a..ff9f5ee9f7 100644 --- a/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs +++ b/osu.Game/Online/Notifications/WebSocket/Events/NewChatMessageData.cs @@ -19,9 +19,7 @@ namespace osu.Game.Online.Notifications.WebSocket.Events [JsonProperty(@"messages")] public List Messages { get; set; } = null!; -#pragma warning disable CA1507 // Happens to name the same because of casing preference [JsonProperty(@"users")] -#pragma warning restore CA1507 private List users { get; set; } = null!; [OnDeserialized] From 7ece8ec1dc466a5f9676b888746c4c33e3140e0f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 3 Dec 2024 00:03:59 +0800 Subject: [PATCH 11/11] Update for suggestions --- osu.Game/Overlays/ChatOverlay.cs | 3 +-- osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index a00414522d..c49afa3a66 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -386,9 +386,8 @@ namespace osu.Game.Overlays { channelList.RemoveChannel(channel); - if (loadedChannels.TryGetValue(channel, out var loaded)) + if (loadedChannels.Remove(channel, out var loaded)) { - loadedChannels.Remove(channel); // DrawableChannel removed from cache must be manually disposed loaded.Dispose(); } diff --git a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs index fb056b457b..83a48599ca 100644 --- a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs +++ b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs @@ -173,10 +173,10 @@ namespace osu.Game.Rulesets.Mods }; drawable.OnRevertResult += (_, result) => { - if (!ratesForRewinding.TryGetValue(result.HitObject, out double rewindValue)) return; + if (!ratesForRewinding.TryGetValue(result.HitObject, out double rate)) return; if (!shouldProcessResult(result)) return; - recentRates.Insert(0, rewindValue); + recentRates.Insert(0, rate); ratesForRewinding.Remove(result.HitObject); recentRates.RemoveAt(recentRates.Count - 1);