Skip to content

Commit d265f8f

Browse files
CopilotBenjaminMichaelis
Copilot
and
BenjaminMichaelis
authored
Unsuppress CA1848: replace direct logger calls with LoggerMessage delegates (#1049)
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. ```csharp // 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` - **Services** — `EmailSender`, `ListingSourceCodeService`, `McpRateLimiterPolicy`, `CaptchaService` - **Controllers** — `ChatController` - **`Program.cs`** — rate-limit rejection handler, exception handler, CSP middleware, sitemap startup - **Identity pages** — `Login`, `Register`, `ExternalLogin`, `Logout`, `LoginWith2fa`, `LoginWithRecoveryCode`, all Manage pages (ChangePassword, DeletePersonalData, Disable2fa, DownloadPersonalData, EnableAuthenticator, GenerateRecoveryCodes, ResetAuthenticator) - **Identity services** — `PwnedPasswordValidator` - **Helpers/Extensions** — `FileInfoExtensions`, `SitemapXmlHelpers` - **Chat.Shared** — `AISearchService`, `MarkdownChunkingService` Also fixed one LOGGEN008 error: removed a redundant `"Error: "` prefix in a log message template whose level already implies the severity. > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### 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: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/IntelliTect/EssentialCSharp.Web/settings/copilot/coding_agent) (admins only) > > </details> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
1 parent 7d7d906 commit d265f8f

25 files changed

Lines changed: 230 additions & 83 deletions

‎.editorconfig‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,6 @@ csharp_style_prefer_top_level_statements = true:silent
155155
csharp_style_expression_bodied_lambdas = true:silent
156156
csharp_style_expression_bodied_local_functions = false:silent
157157

158-
# CA1848: Use the LoggerMessage delegates
159-
dotnet_diagnostic.CA1848.severity = suggestion
160158
# Test files - allow underscore-separated test method names (CA1707)
161159
[{EssentialCSharp.Web.Tests,EssentialCSharp.Chat.Tests}/**]
162160
dotnet_diagnostic.CA1707.severity = none

‎EssentialCSharp.Chat.Shared/Services/AISearchService.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace EssentialCSharp.Chat.Common.Services;
88

9-
public class AISearchService(
9+
public partial class AISearchService(
1010
VectorStore vectorStore,
1111
EmbeddingService embeddingService,
1212
ILogger<AISearchService> logger)
@@ -49,10 +49,13 @@ public async Task<IReadOnlyList<VectorSearchResult<BookContentChunk>>> ExecuteVe
4949
// needed (clearing would evict all healthy connections, hurting concurrent users).
5050
// The retry opens a fresh physical connection, which calls UsePasswordProvider
5151
// and gets a new token from DefaultAzureCredential.
52-
logger.LogWarning(ex, "Entra ID token expired on pooled connection (SqlState 28000); retrying once.");
52+
LogEntraIdTokenExpired(logger, ex);
5353
}
5454
}
5555

5656
throw new UnreachableException("Retry loop exited without returning or throwing.");
5757
}
58+
59+
[LoggerMessage(Level = LogLevel.Warning, Message = "Entra ID token expired on pooled connection (SqlState 28000); retrying once.")]
60+
private static partial void LogEntraIdTokenExpired(ILogger<AISearchService> logger, Exception exception);
5861
}

‎EssentialCSharp.Chat.Shared/Services/MarkdownChunkingService.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public async Task<List<FileChunkingResult>> ProcessMarkdownFilesAsync(
2626
// Validate input parameters
2727
if (!directory.Exists)
2828
{
29-
logger.LogError("Error: Directory {DirectoryName} does not exist.", directory.FullName);
29+
LogDirectoryDoesNotExist(logger, directory.FullName);
3030
throw new InvalidOperationException($"Error: Directory '{directory.FullName}' does not exist.");
3131
}
3232

@@ -177,4 +177,7 @@ public FileChunkingResult ProcessSingleMarkdownFile(
177177

178178
[GeneratedRegex(@"^(#{1,6}) +(.+)$")]
179179
private static partial Regex HeadingRegex();
180+
181+
[LoggerMessage(Level = LogLevel.Error, Message = "Directory {DirectoryName} does not exist.")]
182+
private static partial void LogDirectoryDoesNotExist(ILogger<MarkdownChunkingService> logger, string directoryName);
180183
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/ExternalLogin.cshtml.cs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account;
1515

1616
[AllowAnonymous]
17-
public class ExternalLoginModel(
17+
public partial class ExternalLoginModel(
1818
SignInManager<EssentialCSharpWebUser> signInManager,
1919
UserManager<EssentialCSharpWebUser> userManager,
2020
IUserStore<EssentialCSharpWebUser> userStore,
@@ -78,7 +78,7 @@ public async Task<IActionResult> OnGetCallbackAsync(string? returnUrl = null, st
7878
Microsoft.AspNetCore.Identity.SignInResult result = await signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: false, bypassTwoFactor: true);
7979
if (result.Succeeded)
8080
{
81-
logger.LogInformation("{Name} logged in with {LoginProvider} provider.", info.Principal.Identity?.Name, info.LoginProvider);
81+
LogUserLoggedInWithProvider(logger, info.Principal.Identity?.Name, info.LoginProvider);
8282
// Ensure referral ID is set for the user
8383
var user = await userManager.FindByLoginAsync(info.LoginProvider, info.ProviderKey);
8484
if (user != null)
@@ -140,7 +140,7 @@ public async Task<IActionResult> OnPostConfirmationAsync(string? returnUrl = nul
140140
result = await userManager.AddLoginAsync(user, info);
141141
if (result.Succeeded)
142142
{
143-
logger.LogInformation("User created an account using {Name} provider.", info.LoginProvider);
143+
LogUserCreatedWithProvider(logger, info.LoginProvider);
144144
return await SendConfirmationEmail(returnUrl, info, user);
145145
}
146146
}
@@ -214,4 +214,10 @@ private EssentialCSharpWebUser CreateUser()
214214
$"override the external login page in /Areas/Identity/Pages/Account/ExternalLogin.cshtml", innerException);
215215
}
216216
}
217+
218+
[LoggerMessage(Level = LogLevel.Information, Message = "{Name} logged in with {LoginProvider} provider.")]
219+
private static partial void LogUserLoggedInWithProvider(ILogger<ExternalLoginModel> logger, string? name, string loginProvider);
220+
221+
[LoggerMessage(Level = LogLevel.Information, Message = "User created an account using {Name} provider.")]
222+
private static partial void LogUserCreatedWithProvider(ILogger<ExternalLoginModel> logger, string name);
217223
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/Login.cshtml.cs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account;
1313

14-
public class LoginModel(SignInManager<EssentialCSharpWebUser> signInManager, UserManager<EssentialCSharpWebUser> userManager, ILogger<LoginModel> logger, IReferralService referralService, ICaptchaService captchaService, IOptions<CaptchaOptions> optionsAccessor) : PageModel
14+
public partial class LoginModel(SignInManager<EssentialCSharpWebUser> signInManager, UserManager<EssentialCSharpWebUser> userManager, ILogger<LoginModel> logger, IReferralService referralService, ICaptchaService captchaService, IOptions<CaptchaOptions> optionsAccessor) : PageModel
1515
{
1616
private InputModel? _Input;
1717
[BindProperty]
@@ -102,7 +102,7 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)
102102
}
103103
if (result.Succeeded)
104104
{
105-
logger.LogInformation("User logged in.");
105+
LogUserLoggedIn(logger);
106106
return LocalRedirect(returnUrl);
107107
}
108108
if (result.RequiresTwoFactor)
@@ -111,7 +111,7 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)
111111
}
112112
if (result.IsLockedOut)
113113
{
114-
logger.LogWarning("User account locked out.");
114+
LogUserAccountLockedOut(logger);
115115
return RedirectToPage("./Lockout");
116116
}
117117
else
@@ -124,4 +124,10 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)
124124
// If we got this far, something failed, redisplay form
125125
return Page();
126126
}
127+
128+
[LoggerMessage(Level = LogLevel.Information, Message = "User logged in.")]
129+
private static partial void LogUserLoggedIn(ILogger<LoginModel> logger);
130+
131+
[LoggerMessage(Level = LogLevel.Warning, Message = "User account locked out.")]
132+
private static partial void LogUserAccountLockedOut(ILogger<LoginModel> logger);
127133
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/LoginWith2fa.cshtml.cs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account;
88

9-
public class LoginWith2faModel(
9+
public partial class LoginWith2faModel(
1010
SignInManager<EssentialCSharpWebUser> signInManager,
1111
UserManager<EssentialCSharpWebUser> userManager,
1212
ILogger<LoginWith2faModel> logger) : PageModel
@@ -68,19 +68,28 @@ public async Task<IActionResult> OnPostAsync(bool rememberMe, string? returnUrl
6868

6969
if (result.Succeeded)
7070
{
71-
logger.LogInformation("User with ID '{UserId}' logged in with 2fa.", user.Id);
71+
LogUserLoggedInWith2fa(logger, user.Id);
7272
return LocalRedirect(returnUrl);
7373
}
7474
else if (result.IsLockedOut)
7575
{
76-
logger.LogWarning("User with ID '{UserId}' account locked out.", user.Id);
76+
LogUserAccountLockedOut2fa(logger, user.Id);
7777
return RedirectToPage("./Lockout");
7878
}
7979
else
8080
{
81-
logger.LogWarning("Invalid authenticator code entered for user with ID '{UserId}'.", user.Id);
81+
LogInvalidAuthenticatorCode(logger, user.Id);
8282
ModelState.AddModelError(string.Empty, "Invalid authenticator code.");
8383
return Page();
8484
}
8585
}
86+
87+
[LoggerMessage(Level = LogLevel.Information, Message = "User with ID '{UserId}' logged in with 2fa.")]
88+
private static partial void LogUserLoggedInWith2fa(ILogger<LoginWith2faModel> logger, string userId);
89+
90+
[LoggerMessage(Level = LogLevel.Warning, Message = "User with ID '{UserId}' account locked out.")]
91+
private static partial void LogUserAccountLockedOut2fa(ILogger<LoginWith2faModel> logger, string userId);
92+
93+
[LoggerMessage(Level = LogLevel.Warning, Message = "Invalid authenticator code entered for user with ID '{UserId}'.")]
94+
private static partial void LogInvalidAuthenticatorCode(ILogger<LoginWith2faModel> logger, string userId);
8695
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/LoginWithRecoveryCode.cshtml.cs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
using Microsoft.AspNetCore.Mvc.RazorPages;
66
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account;
77

8-
public class LoginWithRecoveryCodeModel(
8+
public partial class LoginWithRecoveryCodeModel(
99
SignInManager<EssentialCSharpWebUser> signInManager,
10-
UserManager<EssentialCSharpWebUser> userManager,
1110
ILogger<LoginWithRecoveryCodeModel> logger) : PageModel
1211
{
1312
private InputModel? _Input;
@@ -55,23 +54,30 @@ public async Task<IActionResult> OnPostAsync(string? returnUrl = null)
5554

5655
Microsoft.AspNetCore.Identity.SignInResult result = await signInManager.TwoFactorRecoveryCodeSignInAsync(recoveryCode);
5756

58-
string userId = await userManager.GetUserIdAsync(user);
59-
6057
if (result.Succeeded)
6158
{
62-
logger.LogInformation("User with ID '{UserId}' logged in with a recovery code.", user.Id);
59+
LogUserLoggedInWithRecoveryCode(logger, user.Id);
6360
return LocalRedirect(returnUrl ?? Url.Content("~/"));
6461
}
6562
if (result.IsLockedOut)
6663
{
67-
logger.LogWarning("User account locked out.");
64+
LogUserAccountLockedOutRecovery(logger);
6865
return RedirectToPage("./Lockout");
6966
}
7067
else
7168
{
72-
logger.LogWarning("Invalid recovery code entered for user with ID '{UserId}' ", user.Id);
69+
LogInvalidRecoveryCode(logger, user.Id);
7370
ModelState.AddModelError(string.Empty, "Invalid recovery code entered.");
7471
return Page();
7572
}
7673
}
74+
75+
[LoggerMessage(Level = LogLevel.Information, Message = "User with ID '{UserId}' logged in with a recovery code.")]
76+
private static partial void LogUserLoggedInWithRecoveryCode(ILogger<LoginWithRecoveryCodeModel> logger, string userId);
77+
78+
[LoggerMessage(Level = LogLevel.Warning, Message = "User account locked out.")]
79+
private static partial void LogUserAccountLockedOutRecovery(ILogger<LoginWithRecoveryCodeModel> logger);
80+
81+
[LoggerMessage(Level = LogLevel.Warning, Message = "Invalid recovery code entered for user with ID '{UserId}'.")]
82+
private static partial void LogInvalidRecoveryCode(ILogger<LoginWithRecoveryCodeModel> logger, string userId);
7783
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/Logout.cshtml.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@
55

66
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account;
77

8-
public class LogoutModel(SignInManager<EssentialCSharpWebUser> signInManager, ILogger<LogoutModel> logger) : PageModel
8+
public partial class LogoutModel(SignInManager<EssentialCSharpWebUser> signInManager, ILogger<LogoutModel> logger) : PageModel
99
{
1010
public async Task<IActionResult> OnPost(string? returnUrl = null)
1111
{
1212
await signInManager.SignOutAsync();
13-
logger.LogInformation("User logged out.");
14-
// This needs to be a redirect so that the browser performs a new
15-
// request and the identity for the user gets updated.
16-
return returnUrl is not null ? LocalRedirect(returnUrl) : RedirectToPage();
13+
LogUserLoggedOut(logger);
14+
// This needs to be a redirect so that the browser performs a new
15+
// request and the identity for the user gets updated.
16+
return returnUrl is not null ? LocalRedirect(returnUrl) : RedirectToPage();
1717
}
18+
19+
[LoggerMessage(Level = LogLevel.Information, Message = "User logged out.")]
20+
private static partial void LogUserLoggedOut(ILogger<LogoutModel> logger);
1821
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/ChangePassword.cshtml.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account.Manage;
88

9-
public class ChangePasswordModel(
9+
public partial class ChangePasswordModel(
1010
UserManager<EssentialCSharpWebUser> userManager,
1111
SignInManager<EssentialCSharpWebUser> signInManager,
1212
ILogger<ChangePasswordModel> logger) : PageModel
@@ -106,9 +106,12 @@ public async Task<IActionResult> OnPostAsync()
106106
}
107107

108108
await signInManager.RefreshSignInAsync(user);
109-
logger.LogInformation("User changed their password successfully.");
109+
LogUserChangedPassword(logger);
110110
StatusMessage = "Your password has been changed.";
111111

112112
return RedirectToPage();
113113
}
114+
115+
[LoggerMessage(Level = LogLevel.Information, Message = "User changed their password successfully.")]
116+
private static partial void LogUserChangedPassword(ILogger<ChangePasswordModel> logger);
114117
}

‎EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/DeletePersonalData.cshtml.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
namespace EssentialCSharp.Web.Areas.Identity.Pages.Account.Manage;
99

10-
public class DeletePersonalDataModel(
10+
public partial class DeletePersonalDataModel(
1111
UserManager<EssentialCSharpWebUser> userManager,
1212
SignInManager<EssentialCSharpWebUser> signInManager,
1313
ILogger<DeletePersonalDataModel> logger) : PageModel
@@ -79,8 +79,11 @@ public async Task<IActionResult> OnPostAsync()
7979

8080
await signInManager.SignOutAsync();
8181

82-
logger.LogInformation("User with ID '{UserId}' deleted themselves.", userId);
82+
LogUserDeletedThemselves(logger, userId);
8383

8484
return Redirect("~/");
8585
}
86+
87+
[LoggerMessage(Level = LogLevel.Information, Message = "User with ID '{UserId}' deleted themselves.")]
88+
private static partial void LogUserDeletedThemselves(ILogger<DeletePersonalDataModel> logger, string userId);
8689
}

0 commit comments

Comments
 (0)