Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates#1049
Merged
BenjaminMichaelis merged 3 commits intomainfrom May 2, 2026
Merged
Conversation
Agent-Logs-Url: https://github.com/IntelliTect/EssentialCSharp.Web/sessions/e696ce7c-314a-4d73-82b9-749c1fd47833 Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Unsuppress CA1848 from .editorconfig
Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates
May 2, 2026
Contributor
There was a problem hiding this comment.
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
partialand introduced[LoggerMessage]partial methods for structured logging. - Updated logging call sites throughout
Program.cs, services, controllers, identity pages, andChat.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(); |
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>
Contributor
Author
All review feedback has been addressed in commit
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CA1848 was suppressed to
suggestionin.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
ILoggerextension call (LogInformation,LogWarning,LogError, etc.) is replaced with a[LoggerMessage]partial method. Classes not alreadypartialwere made so.Files changed
.editorconfig— removeddotnet_diagnostic.CA1848.severity = suggestionEmailSender,ListingSourceCodeService,McpRateLimiterPolicy,CaptchaServiceChatControllerProgram.cs— rate-limit rejection handler, exception handler, CSP middleware, sitemap startupLogin,Register,ExternalLogin,Logout,LoginWith2fa,LoginWithRecoveryCode, all Manage pages (ChangePassword, DeletePersonalData, Disable2fa, DownloadPersonalData, EnableAuthenticator, GenerateRecoveryCodes, ResetAuthenticator)PwnedPasswordValidatorFileInfoExtensions,SitemapXmlHelpersAISearchService,MarkdownChunkingServiceAlso 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/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)/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)/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/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)/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)/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: