Skip to content

Report the parameter's name for a null map key [patch] - #147

Open
matt-edmondson wants to merge 2 commits into
mainfrom
fix/null-key-param-name-146
Open

matt-edmondson wants to merge 2 commits into
mainfrom
fix/null-key-param-name-146

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #146

What changed

Ensure.NotNull (Polyfill) takes its name from [CallerArgumentExpression], so the (object?) cast in front of every key check ended up in ParamName. A null key passed to ContiguousMap, InsertionOrderMap or OrderedMap reported "(object?)key" where Dictionary<TKey, TValue> reports "key".

All 26 call sites now pass the name explicitly:

  • "key" for the indexer, Add, Remove, ContainsKey and TryGetValue on all three maps
  • "item" for Keys.Contains, and for OrderedMap.Contains(KeyValuePair) and Remove(KeyValuePair)
  • "dictionary" for the OrderedMap constructors that take a dictionary

Tests

Containers.Test/NullKeyParamNameTests.cs checks ParamName for every one of those members on each map, plus both OrderedMap dictionary constructors. The constructors are fed through a small IDictionary stub, because Dictionary can't hold a null key.

  • With the library change reverted, all 4 new tests fail on ParamName
  • With the change, the full suite passes: 520/520 on net10.0

Generated by Claude Code

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(); }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

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.

A null key throws ArgumentNullException with ParamName "(object?)key" instead of "key" in ContiguousMap, InsertionOrderMap and OrderedMap

2 participants