Repository navigation
Report a null pair key in ContiguousMap and InsertionOrderMap as "item" or "dictionary" [patch] - #165
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
…m" or "dictionary" [patch] Contains(KeyValuePair) and Remove(KeyValuePair) now check the key with NullKey.ThrowIfNullItem, and the IDictionary constructors check each source key with NullKey.ThrowIfNullIn, as OrderedMap already does. Before, the name "key" leaked from the inner Dictionary or from Add. Fixes #164 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3faRnes8WmPvx9zps
Contains(KeyValuePair) and Remove(KeyValuePair) were the same code in both maps, which SonarCloud flagged as 48% duplication on new code. They now call one PairLookup.Contains that checks the key with NullKey.ThrowIfNullItem and compares the value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3faRnes8WmPvx9zps
|
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 #164
What was wrong
In
ContiguousMapandInsertionOrderMap, a null key passed toContains(KeyValuePair)orRemove(KeyValuePair), or enumerated by the dictionary passed to anIDictionaryconstructor, threwArgumentNullExceptionwithParamName == "key". None of those methods has a parameter calledkey. The name came from the innerDictionary.TryGetValueor from the constructors'Add(key, value).OrderedMapalready reports"item"and"dictionary"here.Change
PairLookup.Contains(map, item). It checks the key withNullKey.ThrowIfNullItem(item.Key), asOrderedMapdoes, then compares the value.Contains(KeyValuePair)andRemove(KeyValuePair)in both maps now call it. They were the same code in both maps, and SonarCloud flagged the first version of this PR for 48% duplication on new code.IDictionaryconstructors in both maps callNullKey.ThrowIfNullIn(pair.Key)for each source pair, asOrderedMap.BuildSorteddoes.ArgumentNullException.Add(KeyValuePair)still reports"key", as the issue asks. That matches all three maps,OrderedMapincluded.Tests
NullKeyParamNameTestsnow coversContains/Remove(KeyValuePair)forContiguousMapandInsertionOrderMap. New testsContiguousMap_DictionaryConstructorWithNullKey_ReportsDictionaryandInsertionOrderMap_DictionaryConstructorWithNullKey_ReportsDictionarycover the constructors.expected "item"/"dictionary",actual "key").🤖 Generated with Claude Code
https://claude.ai/code/session_017vfJW3faRnes8WmPvx9zps