CodeQL 13: chore: additional style fixes (ternary, complex-block, implicit-conversion)#199
CodeQL 13: chore: additional style fixes (ternary, complex-block, implicit-conversion)#199rlorenzo wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR refactors control flow across six files by converting if/else conditionals to ternary expressions, replacing manual loops with LINQ, removing redundant ChangesCode Simplification Refactoring
🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 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 #199 +/- ##
=======================================
Coverage 42.96% 42.97%
=======================================
Files 877 877
Lines 51468 51450 -18
Branches 4802 4797 -5
=======================================
- Hits 22113 22109 -4
+ Misses 28831 28817 -14
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. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/Areas/Effort/Services/MeritMultiYearService.cs`:
- Around line 868-875: The code builds the responses list using
Enumerable.Repeat with n1..n5 but doesn't validate those counts; negative counts
will throw ArgumentOutOfRangeException. Before calling Enumerable.Repeat (the
block that constructs responses), clamp or sanitize the n1, n2, n3, n4, n5
values to a non-negative integer (e.g., Math.Max(0, nX) or a small helper
NormalizeCount) and then use those sanitized variables when creating responses
so Enumerable.Repeat(...) never receives a negative count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 079a90be-124e-4667-b457-197ab51abdda
📒 Files selected for processing (6)
VueApp/src/Effort/__tests__/course-import-dialog.test.tsweb/Areas/ClinicalScheduler/Controllers/RotationsController.csweb/Areas/Effort/Services/EffortAuditService.csweb/Areas/Effort/Services/MeritMultiYearService.csweb/Areas/RAPS/Services/UinformService.csweb/Areas/Students/Services/GradYearClassLevel.cs
| if (n1 + n2 + n3 + n4 + n5 == 0) return null; | ||
|
|
||
| var responses = Enumerable.Repeat(1m, n1) | ||
| .Concat(Enumerable.Repeat(2m, n2)) | ||
| .Concat(Enumerable.Repeat(3m, n3)) | ||
| .Concat(Enumerable.Repeat(4m, n4)) | ||
| .Concat(Enumerable.Repeat(5m, n5)) | ||
| .ToList(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In .NET, does Enumerable.Repeat<T>(element, count) throw an exception when count is negative? Please cite the official Microsoft docs.
💡 Result:
Yes, in.NET, the Enumerable.Repeat(TResult, Int32) method throws an ArgumentOutOfRangeException if the count parameter is less than 0 [1][2][3]. According to the official Microsoft documentation, the count argument represents the number of times to repeat the value in the generated sequence, and it must be a non-negative integer [1][2][3]. If a negative value is provided, the method explicitly throws an ArgumentOutOfRangeException [1][2][3].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.repeat?view=net-9.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.repeat?view=net-10.0
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.repeat?view=net-6.0
🏁 Script executed:
cd web/Areas/Effort/Services && wc -l MeritMultiYearService.csRepository: ucdavis/VIPER
Length of output: 87
🏁 Script executed:
cd web/Areas/Effort/Services && sed -n '850,900p' MeritMultiYearService.csRepository: ucdavis/VIPER
Length of output: 1732
🏁 Script executed:
cd web/Areas/Effort/Services && sed -n '1,50p' MeritMultiYearService.cs | head -50Repository: ucdavis/VIPER
Length of output: 1734
🏁 Script executed:
cd web/Areas/Effort/Services && rg 'CalculateMedian' -nRepository: ucdavis/VIPER
Length of output: 648
🏁 Script executed:
cd web/Areas/Effort/Services && sed -n '1,900p' MeritMultiYearService.cs | rg -B 10 'CalculateMedian\(' | head -100Repository: ucdavis/VIPER
Length of output: 923
🏁 Script executed:
cd web/Areas/Effort/Services && sed -n '420,460p' MeritMultiYearService.csRepository: ucdavis/VIPER
Length of output: 1648
🏁 Script executed:
cd web/Areas/Effort/Services && sed -n '300,450p' MeritMultiYearService.cs | grep -n -B 30 'CalculateMedian'Repository: ucdavis/VIPER
Length of output: 1296
🏁 Script executed:
cd web && find . -name '*.cs' -type f | xargs rg 'class EvalsMultiyearRow|record EvalsMultiyearRow|struct EvalsMultiyearRow' -A 20Repository: ucdavis/VIPER
Length of output: 2139
🏁 Script executed:
cd web && rg 'EvalsMultiyearRow' --type cs | grep -i 'select\|new EvalsMultiyearRow'Repository: ucdavis/VIPER
Length of output: 150
🏁 Script executed:
cd web && sed -n '1,1000p' Areas/Effort/Services/MeritMultiYearService.cs | grep -n -A 30 'new EvalsMultiyearRow'Repository: ucdavis/VIPER
Length of output: 2066
Guard against negative bucket counts before Enumerable.Repeat.
Enumerable.Repeat(value, count) throws ArgumentOutOfRangeException when count < 0. The N1–N5 values come from database reads with NULL→0 conversion but no range validation. If the database contains corrupted negative counts, this refactor will crash instead of silently handling them. Clamp counts before calling Enumerable.Repeat():
Proposed fix
- if (n1 + n2 + n3 + n4 + n5 == 0) return null;
-
- var responses = Enumerable.Repeat(1m, n1)
- .Concat(Enumerable.Repeat(2m, n2))
- .Concat(Enumerable.Repeat(3m, n3))
- .Concat(Enumerable.Repeat(4m, n4))
- .Concat(Enumerable.Repeat(5m, n5))
+ n1 = Math.Max(0, n1);
+ n2 = Math.Max(0, n2);
+ n3 = Math.Max(0, n3);
+ n4 = Math.Max(0, n4);
+ n5 = Math.Max(0, n5);
+
+ if (n1 + n2 + n3 + n4 + n5 == 0) return null;
+
+ var responses = Enumerable.Repeat(1m, n1)
+ .Concat(Enumerable.Repeat(2m, n2))
+ .Concat(Enumerable.Repeat(3m, n3))
+ .Concat(Enumerable.Repeat(4m, n4))
+ .Concat(Enumerable.Repeat(5m, n5))
.ToList();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (n1 + n2 + n3 + n4 + n5 == 0) return null; | |
| var responses = Enumerable.Repeat(1m, n1) | |
| .Concat(Enumerable.Repeat(2m, n2)) | |
| .Concat(Enumerable.Repeat(3m, n3)) | |
| .Concat(Enumerable.Repeat(4m, n4)) | |
| .Concat(Enumerable.Repeat(5m, n5)) | |
| .ToList(); | |
| n1 = Math.Max(0, n1); | |
| n2 = Math.Max(0, n2); | |
| n3 = Math.Max(0, n3); | |
| n4 = Math.Max(0, n4); | |
| n5 = Math.Max(0, n5); | |
| if (n1 + n2 + n3 + n4 + n5 == 0) return null; | |
| var responses = Enumerable.Repeat(1m, n1) | |
| .Concat(Enumerable.Repeat(2m, n2)) | |
| .Concat(Enumerable.Repeat(3m, n3)) | |
| .Concat(Enumerable.Repeat(4m, n4)) | |
| .Concat(Enumerable.Repeat(5m, n5)) | |
| .ToList(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/Areas/Effort/Services/MeritMultiYearService.cs` around lines 868 - 875,
The code builds the responses list using Enumerable.Repeat with n1..n5 but
doesn't validate those counts; negative counts will throw
ArgumentOutOfRangeException. Before calling Enumerable.Repeat (the block that
constructs responses), clamp or sanitize the n1, n2, n3, n4, n5 values to a
non-negative integer (e.g., Math.Max(0, nX) or a small helper NormalizeCount)
and then use those sanitized variables when creating responses so
Enumerable.Repeat(...) never receives a negative count.
2 participants
Summary
Final sweep of CodeQL style alerts that earlier PRs left as wontfix:
course-import-dialog.test.ts:72,96- narrow the union check by combiningtypeof === 'object'withinstanceof Error, then explicitas Errorcast on the truthy branch.MeritMultiYearService.CalculateMedian- five for-loops appending to a List collapsed into chainedEnumerable.Repeat(...).Concat(...).GradYearClassLevel.cs:63- case-4 if-else assignmentRotationsController.cs:63-serviceId.HasValue ? GetRotationsByServiceAsync : GetRotationsAsyncUinformService.cs:247-response.IsSuccessStatusCode ? Deserialize : new ErrorResponseEffortAuditService.cs:605- termCode-keyed IQueryable selectionuseless-tostringfix onGradYearClassLevel.cs:57,60,69(no-op vs PR CodeQL 12: chore: style sweep for remaining CodeQL alerts #198 which has the same change).Context
Thirteenth in the
CodeQL N:cleanup series.Test plan
npm run test:backend- 1946 tests passing