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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo refactorings replace iteration patterns with LINQ batch and streaming operations. ChangesBatch operations and streaming refactors
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 43.02% 43.03%
=======================================
Files 881 881
Lines 51437 51431 -6
Branches 4812 4810 -2
=======================================
Hits 22131 22131
+ Misses 28780 28774 -6
Partials 526 526
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
rlorenzo
force-pushed
the
codeql/14-effort-pdf-select
branch
from
dfdad52 to
f4e7d91
Compare
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.
rlorenzo
force-pushed
the
codeql/14-effort-pdf-select
branch
from
f4e7d91 to
d171ea3
Compare
There was a problem hiding this comment.
Pull request overview
Refactors a few simple “build-and-add inside foreach” patterns to use LINQ projections, addressing a subset of CodeQL cs/linq/missed-select alerts while keeping behavior the same.
Changes:
- Replaced
foreach+context.Add(...)withcontext.AddRange(...Select(...))when creatingSessionCompetencyrows. - Refactored email-host extraction to use a LINQ projection over the raw SQL results and set the last host if any rows are returned.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/Areas/Directory/Models/IndividualSearchResult.cs | Refactors email-host derivation from raw SQL results using a LINQ projection. |
| web/Areas/CTS/Controllers/CourseController.cs | Converts foreach entity-creation loops into AddRange with LINQ Select for session competencies. |
| var hosts = context.Database.SqlQueryRaw<string>(query) | ||
| .AsEnumerable() | ||
| .Select(r => r.Split("@")[^1]) | ||
| .ToList(); | ||
| if (hosts.Count > 0) | ||
| { | ||
| var parts = r.Split("@"); | ||
| EmailHost = parts[^1]; | ||
| EmailHost = hosts[^1]; |
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