Skip to content

CodeQL 11: chore(tests): replace Returns((T?)null) casts with ReturnsNull()#197

Open
rlorenzo wants to merge 2 commits into
mainfrom
codeql/11-nsubstitute-returns
Open

CodeQL 11: chore(tests): replace Returns((T?)null) casts with ReturnsNull()#197
rlorenzo wants to merge 2 commits into
mainfrom
codeql/11-nsubstitute-returns

Conversation

@rlorenzo
Copy link
Copy Markdown
Contributor

@rlorenzo rlorenzo commented May 13, 2026

Summary

Closes ~19 cs/useless-upcast alerts in test/** by replacing the NSubstitute pattern .Returns((SomeType?)null) with the dedicated .ReturnsNull() extension from NSubstitute.ReturnsExtensions.

Why

The explicit (T?)null cast existed because the bare .Returns(null) call is ambiguous between NSubstitute's Returns<T>(T) and Returns<T>(Func<CallInfo, T>) overloads, but the cast itself trips the cs/useless-upcast rule (CodeQL sees the receiver's return type, so the cast is "redundant from the type system's POV"). .ReturnsNull() is the canonical NSubstitute idiom for this case - it disambiguates without a cast and makes the test intent clearer.

Files (18)

  • ClinicalSchedulerTestBase, IntegrationTestBase
  • CourseRelationshipsControllerTests, CoursesControllerTests, EffortIntegrationTestBase, EffortRecordsControllerTests, EffortTypesControllerTests, InstructorsControllerTests, PercentagesControllerTests, PercentAssignTypesControllerTests, ReportsControllerTests, UnitsControllerTests, VerificationControllerTests, VerificationServiceTests
  • RolesTests
  • EmailServiceTests
  • EmergencyContactControllerTests, PhotoGalleryControllerTest

Each file got a using NSubstitute.ReturnsExtensions; added.

Context

Eleventh in the CodeQL N: cleanup series. Earlier task #9 had marked these as wontfix-by-design - that was wrong. ReturnsNull() is the correct fix.

Test plan

  • npm run test:backend - 1946 tests passing
  • Pre-commit lint+test+verify all passed
  • CodeQL workflow on this PR shows the test/Effort/cs/useless-upcast alerts closed

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • review-ready

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 47aed254-d511-4cef-b0ab-c6c3c8215ff6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codeql/11-nsubstitute-returns

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

Bundle Report

Bundle size has no change ✅

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.96%. Comparing base (38de1ad) to head (183283a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #197   +/-   ##
=======================================
  Coverage   42.96%   42.96%           
=======================================
  Files         877      877           
  Lines       51468    51468           
  Branches     4802     4802           
=======================================
  Hits        22113    22113           
  Misses      28831    28831           
  Partials      524      524           
Flag Coverage Δ
backend 43.04% <ø> (ø)
frontend 41.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…051)

PR-scoped gate flagged line 183 as a new xUnit1051 warning because
'.ReturnsNull()' shifted git blame to this commit. The underlying
FindAsync(object?[]?) call lacked a CancellationToken arg. Use the
overload that accepts both, matched via Arg.Any<CancellationToken>().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants