Repository navigation
Report the parameter's name for a null map key [patch] - #147
Open
matt-edmondson wants to merge 2 commits into
Open
matt-edmondson wants to merge 2 commits into
matt-edmondson wants to merge 2 commits into
Conversation
Ensure.NotNull takes its name from [CallerArgumentExpression], so the (object?) cast in front of each key check ended up in ParamName: ContiguousMap, InsertionOrderMap and OrderedMap reported "(object?)key" where Dictionary reports "key". Pass the name explicitly at all 26 call sites: "key", "item" for Keys.Contains and the KeyValuePair overloads, and "dictionary" for the OrderedMap constructors that take one. Fixes #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3SsCYipEZieVhB1tG
| { | ||
| private readonly List<KeyValuePair<string, int>> pairs = [new(null!, 1)]; | ||
|
|
||
| public int this[string key] { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } |
Contributor
Author
There was a problem hiding this comment.
This is intentional. NullKeyDictionary is a test-only stub with one job: to enumerate a pair with a null key into the OrderedMap constructors, because a real Dictionary can't hold one. The constructors only read it through Count and enumeration, so every other member, the indexer setter included, throws NotSupportedException instead of pretending to work. Leaving it as is.
Generated by Claude Code
Sonar S3236 flags each explicit name passed to Ensure.NotNull's [CallerArgumentExpression] parameter, and KTSU0003 rules out ArgumentNullException.ThrowIfNull. NullKey's checks take the key through a parameter named key, item or dictionary, so the expression Ensure.NotNull records is that name and no call site passes it explicitly. The reported ParamName is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3SsCYipEZieVhB1tG
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #146
What changed
Ensure.NotNull(Polyfill) takes its name from[CallerArgumentExpression], so the(object?)cast in front of every key check ended up inParamName. A null key passed toContiguousMap,InsertionOrderMaporOrderedMapreported"(object?)key"whereDictionary<TKey, TValue>reports"key".All 26 call sites now pass the name explicitly:
"key"for the indexer,Add,Remove,ContainsKeyandTryGetValueon all three maps"item"forKeys.Contains, and forOrderedMap.Contains(KeyValuePair)andRemove(KeyValuePair)"dictionary"for theOrderedMapconstructors that take a dictionaryTests
Containers.Test/NullKeyParamNameTests.cschecksParamNamefor every one of those members on each map, plus bothOrderedMapdictionary constructors. The constructors are fed through a smallIDictionarystub, becauseDictionarycan't hold a null key.ParamNameGenerated by Claude Code