CodeQL 14: refactor: convert foreach-add patterns to LINQ Select#200
CodeQL 14: refactor: convert foreach-add patterns to LINQ Select#200rlorenzo wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #200 +/- ##
=======================================
Coverage 42.96% 42.96%
=======================================
Files 877 877
Lines 51468 51462 -6
Branches 4802 4800 -2
=======================================
Hits 22113 22113
+ Misses 28831 28825 -6
Partials 524 524
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
S2971: SqlQueryRaw().ToList().Select(...) materializes the entire result set into a List just to throw it away. Replace with AsEnumerable() — same effect of switching to LINQ-to-objects so the Split isn't translated to SQL, without the extra allocation.
2 participants
Summary
Closes 3 of the 28
cs/linq/missed-selectalerts where the foreach body is a simple build-and-add into a context/list:CourseController.AddSessionCompetency(line 227) -foreach (var levelId in LevelIds) { var x = new SessionCompetency(...); context.Add(x); }→context.AddRange(LevelIds.Select(levelId => new SessionCompetency(...))).CourseController.UpdateSessionCompetency(line 273, same pattern).IndividualSearchResult(line 130) -foreach (var r in results) { EmailHost = r.Split("@")[^1]; }→Select(...)to materialize the list, then guard on count before assigning the last entry. Semantically identical.Not addressed (still open)
The remaining 25
cs/linq/missed-selectalerts are all in PDF/Excel cell-generation loops in Effort report services where the foreach body calls side-effectfulQuestPDF.table.Cell()…Text(…)orClosedXML.ws.Cell()…Value. Forcing those intoSelectjust renames a single local without removing the iteration or its side effects, so the resulting code is the same length with worse readability. Those should be dismissed at the dashboard.Context
Fourteenth in the
CodeQL N:cleanup series.Test plan
npm run test:backend- 1946 tests passing