Skip to content

Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates#1049

Merged
BenjaminMichaelis merged 3 commits intomainfrom
copilot/unsuppress-ca1848-editorconfig
May 2, 2026
Merged

Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates#1049
BenjaminMichaelis merged 3 commits intomainfrom
copilot/unsuppress-ca1848-editorconfig

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 2, 2026

CA1848 was suppressed to suggestion in .editorconfig, masking a pervasive pattern of suboptimal logging. This PR removes that suppression and fixes all violations by migrating to [LoggerMessage] source-generated delegates.

Approach

Every direct ILogger extension call (LogInformation, LogWarning, LogError, etc.) is replaced with a [LoggerMessage] partial method. Classes not already partial were made so.

// Before
logger.LogWarning("Chapter directory not found: {ChapterDirectory}", chapterDirectory);

// After
[LoggerMessage(Level = LogLevel.Warning, Message = "Chapter directory not found: {ChapterDirectory}")]
private static partial void LogChapterDirectoryNotFound(ILogger<ListingSourceCodeService> logger, string chapterDirectory);

// call site
LogChapterDirectoryNotFound(_Logger, chapterDirectory);

Files changed

  • .editorconfig — removed dotnet_diagnostic.CA1848.severity = suggestion
  • ServicesEmailSender, ListingSourceCodeService, McpRateLimiterPolicy, CaptchaService
  • ControllersChatController
  • Program.cs — rate-limit rejection handler, exception handler, CSP middleware, sitemap startup
  • Identity pagesLogin, Register, ExternalLogin, Logout, LoginWith2fa, LoginWithRecoveryCode, all Manage pages (ChangePassword, DeletePersonalData, Disable2fa, DownloadPersonalData, EnableAuthenticator, GenerateRecoveryCodes, ResetAuthenticator)
  • Identity servicesPwnedPasswordValidator
  • Helpers/ExtensionsFileInfoExtensions, SitemapXmlHelpers
  • Chat.SharedAISearchService, MarkdownChunkingService

Also fixed one LOGGEN008 error: removed a redundant "Error: " prefix in a log message template whose level already implies the severity.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.hcaptcha.com
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/120cf16fe2ba497ba2e0b3c576c12c16 (dns block)
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/f4a1d75f3ca54e5498eb8c5b68a70fbc ACCEPT (dns block)
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/40a09c9a64054f218b5236e9732db5a8 rvices/IGuidelinesService.cs rvices/CaptchaOptions.cs rvices/AuthMessageSenderOptions.cs rvices/McpApiTokenService.cs rvices/RouteConfigurationService.cs rvices/CaptchaService.cs rvices/GuidelinesService.cs rvic�� rvices/ISiteMappingService.cs rvices/SiteSettings.cs (dns block)
  • api.pwnedpasswords.com
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/120cf16fe2ba497ba2e0b3c576c12c16 (dns block)
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/f4a1d75f3ca54e5498eb8c5b68a70fbc ACCEPT (dns block)
    • Triggering command: /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests /home/REDACTED/work/EssentialCSharp.Web/EssentialCSharp.Web/EssentialCSharp.Web.Tests/bin/Release/net10.0/EssentialCSharp.Web.Tests --server dotnettestcli --dotnet-test-pipe /tmp/40a09c9a64054f218b5236e9732db5a8 rvices/IGuidelinesService.cs rvices/CaptchaOptions.cs rvices/AuthMessageSenderOptions.cs rvices/McpApiTokenService.cs rvices/RouteConfigurationService.cs rvices/CaptchaService.cs rvices/GuidelinesService.cs rvic�� rvices/ISiteMappingService.cs rvices/SiteSettings.cs (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Unsuppress CA1848 from .editorconfig Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates May 2, 2026
Copilot AI requested a review from BenjaminMichaelis May 2, 2026 07:11
@BenjaminMichaelis BenjaminMichaelis marked this pull request as ready for review May 2, 2026 10:35
Copilot AI review requested due to automatic review settings May 2, 2026 10:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the .editorconfig suppression of CA1848 and migrates existing logging from direct ILogger extension method calls to [LoggerMessage] source-generated delegates across the web app, identity pages, and shared chat services.

Changes:

  • Removed CA1848 severity override in .editorconfig.
  • Converted many types to partial and introduced [LoggerMessage] partial methods for structured logging.
  • Updated logging call sites throughout Program.cs, services, controllers, identity pages, and Chat.Shared.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.editorconfig Removes CA1848 suppression so violations surface during build/analysis.
EssentialCSharp.Web/Program.cs Replaces rate-limit/exception/CSP/sitemap logging with LoggerMessage delegates.
EssentialCSharp.Web/Services/McpRateLimiterPolicy.cs Migrates rejection logging to a LoggerMessage delegate.
EssentialCSharp.Web/Services/ListingSourceCodeService.cs Migrates file/directory warning logs to LoggerMessage delegates.
EssentialCSharp.Web/Services/EmailSender.cs Migrates Mailjet response logging to LoggerMessage delegates.
EssentialCSharp.Web/Services/CaptchaService.cs Migrates hostname mismatch and request-failure logging to LoggerMessage delegates.
EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs Migrates sitemap generation logging to a LoggerMessage delegate.
EssentialCSharp.Web/Extensions/FileInfoExtensions.cs Migrates guideline JSON read errors/info logging to LoggerMessage delegates.
EssentialCSharp.Web/Controllers/ChatController.cs Migrates streaming cancellation/error logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Services/PasswordValidators/PwnedPasswordValidator.cs Migrates fail-open warning log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Register.cshtml.cs Migrates registration/captcha logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Pages/Account/ExternalLogin.cshtml.cs Migrates external-login informational logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Pages/Account/LoginWithRecoveryCode.cshtml.cs Migrates recovery-code login logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Pages/Account/LoginWith2fa.cshtml.cs Migrates 2FA login logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Login.cshtml.cs Migrates login/lockout logs to LoggerMessage delegates.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Logout.cshtml.cs Migrates logout log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/ChangePassword.cshtml.cs Migrates change-password log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/DeletePersonalData.cshtml.cs Migrates delete-personal-data log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/Disable2fa.cshtml.cs Migrates disable-2FA log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/DownloadPersonalData.cshtml.cs Migrates download-personal-data log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/EnableAuthenticator.cshtml.cs Migrates enable-authenticator log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/GenerateRecoveryCodes.cshtml.cs Migrates generate-recovery-codes log to a LoggerMessage delegate.
EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/ResetAuthenticator.cshtml.cs Migrates reset-authenticator log to a LoggerMessage delegate.
EssentialCSharp.Chat.Shared/Services/AISearchService.cs Migrates token-expiry retry warning to a LoggerMessage delegate.
EssentialCSharp.Chat.Shared/Services/MarkdownChunkingService.cs Migrates missing-directory error log to a LoggerMessage delegate.

[GeneratedRegex(@"^(#{1,6}) +(.+)$")]
private static partial Regex HeadingRegex();

[LoggerMessage(Level = LogLevel.Error, Message = "Directory {directoryName} does not exist.")]
Comment on lines +55 to +58
[LoggerMessage(Level = LogLevel.Information, Message = "Email to queued successfully!")]
private static partial void LogEmailQueued(ILogger logger);

[LoggerMessage(Level = LogLevel.Error, Message = "Failure To Send Email")]
Comment on lines +58 to +62
@@ -59,19 +59,28 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)

if (result.Succeeded)
{
logger.LogInformation("User with ID '{UserId}' logged in with a recovery code.", user.Id);
LogUserLoggedInWithRecoveryCode(logger, user.Id);
[LoggerMessage(Level = LogLevel.Warning, Message = "User account locked out.")]
private static partial void LogUserAccountLockedOutRecovery(ILogger<LoginWithRecoveryCodeModel> logger);

[LoggerMessage(Level = LogLevel.Warning, Message = "Invalid recovery code entered for user with ID '{UserId}' ")]
Comment on lines 14 to 16
// This needs to be a redirect so that the browser performs a new
// request and the identity for the user gets updated.
return returnUrl is not null ? LocalRedirect(returnUrl) : RedirectToPage();
@BenjaminMichaelis
Copy link
Copy Markdown
Member

@copilot any feedback we need to address?

…tion, unused variable

Agent-Logs-Url: https://github.com/IntelliTect/EssentialCSharp.Web/sessions/89025ed7-2a22-4675-845d-14bc7cfdf263

Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 2, 2026

@copilot any feedback we need to address?

All review feedback has been addressed in commit cd3a79d:

  • MarkdownChunkingService.cs: Restored {DirectoryName} (PascalCase) in the log template to preserve backwards compatibility with existing log queries.
  • EmailSender.cs: Cleaned up awkward log messages — "Email to queued successfully!" → "Email queued successfully." and "Failure To Send Email" → "Failed to send email."
  • LoginWithRecoveryCode.cshtml.cs: Removed the unused userId variable and the now-unused userManager constructor parameter; logging now consistently uses user.Id. Also removed the trailing space from the "Invalid recovery code" message template.
  • Logout.cshtml.cs: Fixed the over-indented comment and return statement.

@BenjaminMichaelis BenjaminMichaelis merged commit d265f8f into main May 2, 2026
7 checks passed
@BenjaminMichaelis BenjaminMichaelis deleted the copilot/unsuppress-ca1848-editorconfig branch May 2, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsupress CA1848 from .editorconfig

3 participants