CodeQL 12: chore: style sweep for remaining CodeQL alerts#198
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 #198 +/- ##
==========================================
+ Coverage 42.96% 42.98% +0.02%
==========================================
Files 877 877
Lines 51468 51433 -35
Branches 4802 4804 +2
==========================================
- Hits 22113 22109 -4
+ Misses 28831 28800 -31
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. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- UserHelper.cs: capture user into 'fallbackUser' local before
GetOrCreate lambda so ReSharper AccessToModifiedClosure is silenced
- HttpHelper.cs: drop stale '<param name="memoryCache">' XML tag now
that the other params don't have matching tags (was creating 5
InvalidXmlDocComment warnings since the rename)
- test/ClinicalScheduler/{EmailNotificationTest,TestDataBuilder}.cs:
replace '(double)(-7 * x)' with '-7.0 * x' — same intent (force the
multiplication into double per cs/loss-of-precision) but no
RedundantCast warning from ReSharper
3 participants
Summary
Sweep PR covering the remaining straightforward CodeQL style alerts that earlier PRs deferred.
Closes
cs/useless-tostring-call (7):
HomeController.cs:315-doc.ToString()in string concat → dropGradYearClassLevel.cs:57,60,69-.ToString()afterintin"V" + ...UinformService.cs:274-epochTime.ToString()in concatLdapUserContact.cs:39-v.ToString()in concatTermCodeService.cs:39-year.ToString()instring.Formatcs/missed-ternary-operator (6 / 17):
HttpHelper.cs:51-HttpContextgetter collapsed to expression-bodied?.HttpContextRAPSController.cs:315,359-if/else return View(...)→ ternaryCMSController.cs:28-if/else return cms.X(...)→ ternaryVerificationService.cs:773-dueDateif/else assignment → ternaryUserHelper.cs:68,152,280,345- fourif (x != null) return x; else return Y;blocks →x ?? Y(in CodeQL 12 part 1 before this PR was scoped - folded in here)cs/local-shadows-member (3):
HttpHelper.ConfigureparametershttpContextAccessor,authorizationService,dataProtectionProviderrenamed to non-shadowing names (contextAccessor,authzService,dataProtection); the assignments to private static fields no longer need theHttpHelper.qualifier.cs/loss-of-precision (3):
test/ClinicalScheduler/EmailNotificationTest.cs:122-123andTestDataBuilder.cs:179- explicit(double)casts around theintarithmetic flowing intoDateTime.AddDays(double).Not addressed
Remaining
cs/missed-ternary-operator(11) where the branches contain multi-statement bodies that don't fit ternary cleanly (UinformService.cs:247 deserialize path, EffortAuditService.cs:605 IQueryable build, etc.) - converting them would hurt readability. Same forcs/complex-condition(17) which are subjective rewrites,cs/complex-block(1),cs/nested-if-statements(2). The twojs/implicit-operand-conversionalerts incourse-import-dialog.test.ts:72,96are intentional test code simulating non-Error throws.Context
Twelfth in the
CodeQL N:cleanup series.Test plan
npm run test:backend- 1946 tests passing