[PR #12628] Fix some analyzer IDisposable warnings #13117

New issue

Open

opened 2025-12-22 09:40:48 +01:00 by backuprepo · 0 comments

backuprepo commented

2025-12-22 09:40:48 +01:00

Owner

📋 Pull Request Information

Original PR: https://github.com/jellyfin/jellyfin/pull/12628
Author: @fice-t
Created: 9/11/2024
Status: 🔄 Open

Base: master ← Head: disposable

📝 Commits (5)

51de8c2 Enable CA2213 analyzer rule as error
2e3946d Fix several small CA2000 violations
e9923da Pass null instead of empty array to HttpClient
d7b854a Fix several CA2000 violations in tests
5d1ad51 Enable more rules for IDisposable as errors

📊 Changes

14 files changed (+97 additions, -29 deletions)

View changed files

📝 Emby.Server.Implementations/ApplicationHost.cs (+2 -2)
📝 Jellyfin.Api/Controllers/VideosController.cs (+1 -1)
📝 MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs (+3 -2)
📝 jellyfin.ruleset (+6 -0)
📝 tests/Jellyfin.LiveTv.Tests/HdHomerunHostTests.cs (+17 -2)
📝 tests/Jellyfin.LiveTv.Tests/Listings/XmlTvListingsProviderTests.cs (+17 -2)
📝 tests/Jellyfin.MediaEncoding.Tests/Probing/ProbeExternalSourcesTests.cs (+1 -1)
📝 tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs (+1 -1)
📝 tests/Jellyfin.Server.Implementations.Tests/HttpServer/WebSocketConnectionTests.cs (+12 -4)
📝 tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs (+16 -9)
📝 tests/Jellyfin.Server.Implementations.Tests/Updates/InstallationManagerTests.cs (+18 -2)
📝 tests/Jellyfin.Server.Integration.Tests/AuthHelper.cs (+1 -1)
📝 tests/Jellyfin.Server.Integration.Tests/Controllers/DashboardControllerTests.cs (+1 -1)
📝 tests/Jellyfin.Server.Integration.Tests/Controllers/StartupControllerTests.cs (+1 -1)

📄 Description

I haven't had much experience with C#, so I decided to start with fixing some of what the IDE analyzers were yelling about.

There are still remaining CA2000 violations, which fall into several categories:

Required touching larger portions of code (such as SKBitMap)
Not sure if a simple (await) using is appropriate (such as CancellationTokenSource and the log Stream for FFmpeg)
Fixing the "recommended way" introduces an IDISP003 warning, though I believe it's an analyzer bug (FromBase64Transform, Socket)
False positives (analyzer bugs for CA2000) (such as not recognizing that ControllerBase.File disposes its Stream argument, and something weird with GetBitmapFromSvg being part of a conditional expression).

Changes

These bring down the number of CA2000 violations that VS reports from 47 to 22, as well as remove the only CA2213 violation and add CA2213+CA2216 as build errors.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/jellyfin/jellyfin/pull/12628 **Author:** [@fice-t](https://github.com/fice-t) **Created:** 9/11/2024 **Status:** 🔄 Open **Base:** `master` ← **Head:** `disposable` --- ### 📝 Commits (5) - [`51de8c2`](https://github.com/jellyfin/jellyfin/commit/51de8c2f567639aa57cc2a1441f56892c5ff9e6d) Enable CA2213 analyzer rule as error - [`2e3946d`](https://github.com/jellyfin/jellyfin/commit/2e3946d7febcd3855f5767fbdffc298508593d9b) Fix several small CA2000 violations - [`e9923da`](https://github.com/jellyfin/jellyfin/commit/e9923dab2a161e19c0aa24e0056733c6a3a4bbe7) Pass null instead of empty array to HttpClient - [`d7b854a`](https://github.com/jellyfin/jellyfin/commit/d7b854a3fa18bdbb7692d7b2041a402484172b95) Fix several CA2000 violations in tests - [`5d1ad51`](https://github.com/jellyfin/jellyfin/commit/5d1ad5198ecaf529b08390726dac0ad6403a0b44) Enable more rules for IDisposable as errors ### 📊 Changes **14 files changed** (+97 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `Emby.Server.Implementations/ApplicationHost.cs` (+2 -2) 📝 `Jellyfin.Api/Controllers/VideosController.cs` (+1 -1) 📝 `MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs` (+3 -2) 📝 `jellyfin.ruleset` (+6 -0) 📝 `tests/Jellyfin.LiveTv.Tests/HdHomerunHostTests.cs` (+17 -2) 📝 `tests/Jellyfin.LiveTv.Tests/Listings/XmlTvListingsProviderTests.cs` (+17 -2) 📝 `tests/Jellyfin.MediaEncoding.Tests/Probing/ProbeExternalSourcesTests.cs` (+1 -1) 📝 `tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs` (+1 -1) 📝 `tests/Jellyfin.Server.Implementations.Tests/HttpServer/WebSocketConnectionTests.cs` (+12 -4) 📝 `tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs` (+16 -9) 📝 `tests/Jellyfin.Server.Implementations.Tests/Updates/InstallationManagerTests.cs` (+18 -2) 📝 `tests/Jellyfin.Server.Integration.Tests/AuthHelper.cs` (+1 -1) 📝 `tests/Jellyfin.Server.Integration.Tests/Controllers/DashboardControllerTests.cs` (+1 -1) 📝 `tests/Jellyfin.Server.Integration.Tests/Controllers/StartupControllerTests.cs` (+1 -1) </details> ### 📄 Description  I haven't had much experience with C#, so I decided to start with fixing some of what the IDE analyzers were yelling about. There are still remaining CA2000 violations, which fall into several categories: * Required touching larger portions of code (such as `SKBitMap`) * Not sure if a simple `(await) using` is appropriate (such as `CancellationTokenSource` and the log `Stream` for FFmpeg) * Fixing the "recommended way" introduces an IDISP003 warning, though I believe it's an analyzer bug (`FromBase64Transform`, `Socket`) * False positives (analyzer bugs for CA2000) (such as not recognizing that `ControllerBase.File` disposes its `Stream` argument, and something weird with `GetBitmapFromSvg` being part of a conditional expression). **Changes**  These bring down the number of CA2000 violations that VS reports from 47 to 22, as well as remove the only CA2213 violation and add CA2213+CA2216 as build errors. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>