fix(quality): clear ReSharper PR-gate + CodeQL review findings

- Real cleanups across CMS / ClinicalScheduler / Effort / RAPS / Students:
  drop dead null-checks Roslyn flow analysis can prove unreachable,
  collapse a duplicated return path, simplify the cache lookup that
  was capturing a never-assigned outer variable, fix the stale XML doc
  param tag left behind by the IEnumerable materialisation rename,
  remove a virtual call from a DTO constructor, and split a SqlCommand
  object initializer outside the using statement.
- Replace anonymous-type byte[]? gymnastics in PhotoExportService with
  a named record so CodeQL stops flagging the casts as redundant.
- Gate now supports `--exclude-rule` and ships defaults for rules that
  fire false positives on ASP.NET Core / EF surfaces (DTO accessors
  bound at runtime by JSON / MVC, record positional properties used
  via record pattern Equals, and the EF nav-property NRT contract that
  Roslyn intentionally distrusts because Include() can be missing).

Author: rlorenzo

scripts/audit-resharper-regression.js

@@ -12,6 +12,7 @@
 //                                              [--sarif inspect-report/inspect.sarif]
 //                                              [--skip-scan]
 //                                              [--staged]
+//                                              [--exclude-rule <id> ...]
 //
 // --skip-scan reuses the SARIF from a prior `audit:resharper` run, useful in
 // CI where the full scan and the gate are split into separate steps so the
@@ -20,6 +21,11 @@
 // --staged filters findings to staged C# lines (`git diff --cached`) instead of
 // PR-diff lines. Pair with --skip-scan for fast iterative pre-commit checks
 // against an existing SARIF report.
+//
+// --exclude-rule adds a ReSharper rule id to skip (repeatable). The default
+// list covers rules known to misfire on ASP.NET Core / EF DTO patterns where
+// public surface looks unused to static analysis but is wired up at runtime
+// (JSON serialization, MVC model binding, Razor views, EF projections).
 
 const fs = require("node:fs")
 const path = require("node:path")
@@ -34,8 +40,30 @@ const MAX_BUFFER_BYTES = 268_435_456
 // Cap how many findings we print per rule before summarising the rest.
 const MAX_FINDINGS_PER_RULE = 5
 
+// Rules excluded by default because they fire false positives on the kinds of
+// public surface ASP.NET Core / EF wires up at runtime (DTO/binding/EF
+// projection types) or where ReSharper's NRT contract analysis disagrees
+// with Roslyn's flow analysis (EF nav-property dereferences after `?.`).
+const DEFAULT_EXCLUDED_RULES = new Set([
+    "UnusedAutoPropertyAccessor.Global",
+    "UnusedAutoPropertyAccessor.Local",
+    "NotAccessedPositionalProperty.Local",
+    "S3260", // SonarLint sealed-record rule, low actionable value here
+    // ReSharper trusts the NRT annotation on EF nav properties (`Rotation` is
+    // declared non-null with `null!` default), but Roslyn rightly insists on
+    // `?.` because the runtime can produce null when Include() is missing.
+    // Keep the runtime-safe `?.Nav?.Member` style and silence the ReSharper rule.
+    "ConditionalAccessQualifierIsNonNullableAccordingToAPIContract",
+])
+
 function parseArgs(argv) {
-    const args = { base: "origin/main", sarif: DEFAULT_SARIF, skipScan: false, staged: false }
+    const args = {
+        base: "origin/main",
+        sarif: DEFAULT_SARIF,
+        skipScan: false,
+        staged: false,
+        excludedRules: new Set(DEFAULT_EXCLUDED_RULES),
+    }
     const remaining = [...argv]
     while (remaining.length > 0) {
         const flag = remaining.shift()
@@ -47,6 +75,8 @@ function parseArgs(argv) {
             args.skipScan = true
         } else if (flag === "--staged") {
             args.staged = true
+        } else if (flag === "--exclude-rule") {
+            args.excludedRules.add(remaining.shift())
         } else {
             console.error(`Unknown arg: ${flag}`)
             process.exit(2)
@@ -130,7 +160,7 @@ function normalizeUri(uri) {
     return s
 }
 
-function findRegressions(sarifPath, changedLines) {
+function findRegressions(sarifPath, changedLines, excludedRules) {
     const sarif = JSON.parse(fs.readFileSync(sarifPath, "utf8"))
     const results = sarif.runs?.[0]?.results ?? []
 
@@ -142,6 +172,9 @@ function findRegressions(sarifPath, changedLines) {
     const regressions = []
     for (const r of results) {
         const ruleId = r.ruleId ?? "?"
+        if (excludedRules.has(ruleId)) {
+            continue
+        }
         for (const loc of r.locations ?? []) {
             const uri = loc.physicalLocation?.artifactLocation?.uri
             const line = loc.physicalLocation?.region?.startLine
@@ -186,7 +219,11 @@ if (changed.size === 0) {
     process.exit(0)
 }
 
-const regressions = findRegressions(args.sarif, changed)
+if (args.excludedRules.size > 0) {
+    const sortedRules = [...args.excludedRules].toSorted()
+    console.log(`Excluding ${sortedRules.length} rule(s) from gate: ${sortedRules.join(", ")}`)
+}
+const regressions = findRegressions(args.sarif, changed, args.excludedRules)
 if (regressions.length === 0) {
     console.log(`✅ No new ReSharper warnings at ${touchedLabel} lines.`)
     process.exit(0)

web/Areas/CMS/Data/CMS.cs

@@ -525,9 +525,6 @@ public IActionResult ProvideFile(Controller controller, string id, string friend
                 bytes = DecryptFile(bytes, file.Key);
             }
 
-            if (bytes == null)
-                return controller.NotFound();
-
             string extension = file.FilePath[(file.FilePath.LastIndexOf('.') + 1)..];
             controller.Response.Headers["Content-Disposition"] = "inline; filename=" + friendlyName;
 

web/Areas/CMS/Data/Codecs.cs

@@ -1,6 +1,5 @@
 // FROM https://rextester.com/TGN19503
 
-using System.IO;
 using System.Text;
 
 namespace Viper.Areas.CMS.Data

web/Areas/ClinicalScheduler/Controllers/CliniciansController.cs

@@ -372,7 +372,7 @@ public async Task<IActionResult> GetClinicianRotations(string mothraId)
                         RotationName = r.Name,
                         r.Abbreviation,
                         r.ServiceId,
-                        r.Service!.ServiceName
+                        r.Service.ServiceName
                     })
                     .OrderBy(r => r.ServiceName)
                     .ThenBy(r => r.RotationName)
@@ -529,7 +529,8 @@ private static object CreateDefaultClinicianInfo(string mothraId, string? role =
         /// Filter clinicians based on user permissions.
         /// Users with EditOwnSchedule permission should only see themselves.
         /// </summary>
-        /// <param name="clinicians">List of all available clinicians</param>
+        /// <param name="cliniciansSource">List of all available clinicians</param>
+        /// <param name="viewContext">Optional view context label used for log messages</param>
         /// <returns>Filtered list of clinicians based on user permissions</returns>
         private List<ClinicianSummary> FilterCliniciansByPermissions(IEnumerable<ClinicianSummary> cliniciansSource, string? viewContext = null)
         {

web/Areas/ClinicalScheduler/Services/EvaluationPolicyService.cs

@@ -37,7 +37,9 @@ public bool RequiresPrimaryEvaluator(
                 return false;
             }
 
-            // Validate input data
+            // Defensive: contract says non-null but tests cover the NRT-violating
+            // path explicitly (reflection / interop callers).
+            // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
             if (rotationWeeks == null)
             {
                 return false;

web/Areas/ClinicalScheduler/Services/ScheduleEditService.cs

@@ -130,7 +130,7 @@ public async Task<List<InstructorSchedule>> AddInstructorAsync(
 
 
                 // Use Serializable isolation level to prevent race conditions
-                var result = await ExecuteInTransactionAsync(async cancellationToken =>
+                var result = await ExecuteInTransactionAsync(async _ =>
                 {
                     var createdSchedules = new List<InstructorSchedule>();
                     var removedPrimarySchedules = new Dictionary<int, List<InstructorSchedule>>(); // WeekId -> removed schedules
@@ -312,7 +312,7 @@ await _auditService.LogPrimaryEvaluatorSetAsync(
                     }
                 }
 
-                await ExecuteInTransactionAsync(async cancellationToken =>
+                await ExecuteInTransactionAsync(async _ =>
                 {
                     // Remove the schedule
                     _context.InstructorSchedules.Remove(schedule);
@@ -391,7 +391,7 @@ await _auditService.LogInstructorRemovedAsync(
                     return (true, null);
                 }
 
-                var result = await ExecuteInTransactionAsync(async cancellationToken =>
+                var result = await ExecuteInTransactionAsync(async _ =>
                 {
                     string? previousPrimaryName = null;
                     List<InstructorSchedule> removedPrimarySchedules = [];

web/Areas/Effort/Services/ClinicalEffortService.cs

@@ -2,7 +2,6 @@
 using ClosedXML.Excel;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore;
-using QuestPDF;
 using QuestPDF.Fluent;
 using QuestPDF.Helpers;
 using QuestPDF.Infrastructure;

web/Areas/Effort/Services/ClinicalScheduleService.cs

@@ -1,7 +1,6 @@
 using ClosedXML.Excel;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore;
-using QuestPDF;
 using QuestPDF.Fluent;
 using QuestPDF.Helpers;
 using QuestPDF.Infrastructure;
@@ -144,7 +143,8 @@ private async Task<List<CliWeeksRawRow>> QueryClinicalScheduleAsync(
         await using var connection = new SqlConnection(connectionString);
         await connection.OpenAsync(ct);
 
-        await using var command = new SqlCommand { Connection = connection };
+        await using var command = new SqlCommand();
+        command.Connection = connection;
         var paramNames = new List<string>(clinicalJobCodes.Count);
         for (var i = 0; i < clinicalJobCodes.Count; i++)
         {

web/Areas/Effort/Services/DeptSummaryService.cs

@@ -1,5 +1,4 @@
 using ClosedXML.Excel;
-using QuestPDF;
 using QuestPDF.Fluent;
 using QuestPDF.Helpers;
 using QuestPDF.Infrastructure;

web/Areas/Effort/Services/EvaluationReportService.cs

@@ -1,7 +1,6 @@
 using ClosedXML.Excel;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore;
-using QuestPDF;
 using QuestPDF.Fluent;
 using QuestPDF.Helpers;
 using QuestPDF.Infrastructure;