fix: replace Dictionary with List for wildcard-compatible rule storage#75
Closed
mfogliatto-dev-agent wants to merge 1 commit into
Closed
fix: replace Dictionary with List for wildcard-compatible rule storage#75mfogliatto-dev-agent wants to merge 1 commit into
mfogliatto-dev-agent wants to merge 1 commit into
Conversation
The rules dictionary used PatternMatchComparer as its equality comparer, but PatternMatchComparer.GetHashCode used the default string.GetHashCode, violating the IEqualityComparer contract (equal objects must have equal hash codes). This made Dictionary lookups silently broken for wildcard patterns. Since the rules collection is only iterated (never looked up by key), replace it with List<KeyValuePair<>> which correctly reflects its usage. Duplicate detection now uses the comparer's Equals method directly instead of relying on Dictionary.Add's broken hash-based check. The exactMatchRules dictionary (using StringComparer.InvariantCulture) and patternRules list in the experimental path are unaffected — they already use correct comparers for their respective purposes. Fixes #70
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
1 participant
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.
Summary
Replaces the
Dictionary<string, Rule>fieldrulesinAssemblyNameViolationDetectorwithList<KeyValuePair<string, Rule>>to fix a brokenIEqualityComparercontract.Problem
PatternMatchComparer.GetHashCodeuses the defaultstring.GetHashCode(), which violates theIEqualityComparer<string>contract: whenEquals("*", "SomeAssembly")returnstrue, their hash codes differ. This means:Dictionaryusing this comparer cannot perform correct hash-based lookups for wildcard keysDictionary.Addwas also broken (two patterns the comparer considers equal but with different hashes would not be detected as duplicates)GetViolationsFromiterates the dictionary viaforeach, bypassing hash lookups entirelyFix
Since
rulesis only iterated (never looked up by key), replace it with aList<KeyValuePair<>>— this correctly reflects the actual usage pattern and eliminates the broken hash dependency.Duplicate detection now uses the comparer's
Equalsmethod directly viaList.Any(), which is correct for all comparer types including wildcard-aware ones.The
exactMatchRulesdictionary (usingStringComparer.InvariantCulture) andpatternRuleslist in the experimental path are unaffected.Testing
All 18
AssemblyNameViolationDetectorTestspass. The 8 pre-existing failures inProjectPathProviderTestsare unrelated (Windows path tests on Linux).Fixes #70