demo(12-missing-null-guard): 12 — New public method takes a nullable string with no guard#161
demo(12-missing-null-guard): 12 — New public method takes a nullable string with no guard#161
Conversation
…string with no guard See scenarios/12-missing-null-guard/README.md for the expected verdict.
|
|
||
| public sealed class CustomerNoteFormatter | ||
| { | ||
| public string Format(string? note) |
There was a problem hiding this comment.
GCI0006 — Edge Case Handling
New method parameter(s) added without apparent null/range validation in src/OrderService/Pricing/CustomerNoteFormatter.cs
Evidence:
Line 5: public string Format(string? note)
💡 Suggested action: Add ArgumentNullException.ThrowIfNull() or similar guard at the top of the method.
Confidence: Medium | Severity: Warn
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0613fdb15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| public string Format(string? note) | ||
| { | ||
| return note!.Trim().ToUpperInvariant(); |
There was a problem hiding this comment.
Guard nullable note before calling Trim
Format declares note as nullable (string?) but immediately dereferences it via note! and calls Trim, so Format(null) will throw a NullReferenceException from inside Trim instead of failing fast with a clear argument validation error. Since this is a public method, callers are allowed by the signature to pass null; add an explicit null guard (or fallback behavior) at the method boundary.
Useful? React with 👍 / 👎.
12 — New public method takes a nullable string with no guard
Expected verdict: ❌ Fails — GauntletCI should fire GCI0006 (edge case handling).
What changed
A new
CustomerNoteFormatterexposes a public method that accepts anullable
string?and immediately dereferences it without anyruntime check:
The compiler is silenced with
!(so the build still succeeds), butthe runtime behaviour is unchanged — calling
Format(null)throwsNullReferenceExceptiondeep insideTrim.Why this is risky
null(string?), so callers areentitled to pass it. Nothing in the body translates that into a
documented
ArgumentNullExceptionor a sensible default.NullReferenceExceptionthrown from a transitive callee is one ofthe worst error shapes to debug: the stack trace points at
Trim,not at the caller that handed in
null.ArgumentNullException.ThrowIfNull(note)(or a??fallback) makes the contract explicit.
What GauntletCI catches
GCI0006 Edge Case Handling— a public method whose signaturedeclares a nullable reference parameter (
string?/object?) andwhose first few lines contain no
nullcheck,throw, orArgumentNullExceptionguard.