refactor!: drop redundant/ambiguous dictionary extensions from TryExtensions - #82
Conversation
…ensions Remove GetOrNull (redundant with BCL GetValueOrDefault), GetOrDefault (same), and all IDictionary/IList overloads that cause ambiguous invocation on types implementing both mutable and read-only interfaces. Kept: GetValueOrNull (struct, no BCL equivalent), GetOrElse (lazy factory), all TryParse methods, collection safe-access on IReadOnlyList. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review Summary by CodeRabbit
WalkthroughThe extension API surface in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (7 passed)
Review rate limit: 3/5 reviews remaining, refill in 23 minutes and 59 seconds. Comment |
There was a problem hiding this comment.
Pull request overview
Removes redundant and ambiguity-prone extension methods from TryExtensions, narrowing dictionary helpers to IReadOnlyDictionary (to match BCL scoping) and trimming list helpers to IReadOnlyList to avoid ambiguous overload resolution on types implementing multiple collection interfaces.
Changes:
- Removed
GetOrNullandGetOrDefaultdictionary extensions (redundant with BCLGetValueOrDefault). - Removed
IDictionary(andIList) overloads to eliminate ambiguous invocation on dual-interface types. - Retained
GetValueOrNull(value types),GetOrElse(lazy factory),TryParse*helpers, andIReadOnlyListsafe-access helpers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request refactors TryExtensions.cs by removing extension methods targeting mutable interfaces like IDictionary and IList, favoring IReadOnlyDictionary and IReadOnlyList instead. Specifically, it removes GetOrNull for reference types, GetOrDefault for dictionaries, and several IList extensions, while updating the documentation to match the updated API. I have no feedback to provide.
Summary
GetOrNull(redundant with BCLGetValueOrDefault) andGetOrDefault(same)IDictionary/IListoverloads that cause ambiguous invocation on types implementing both mutable and read-only interfaces (mirrors BCL's design of scoping toIReadOnlyDictionaryonly)GetValueOrNull(struct, returnsNullable<T>— no BCL equivalent),GetOrElse(lazy factory), allTryParse*, collection safe-access onIReadOnlyListTest plan
AnalyzerConfigOptionsProviderExtensions.csunaffected (uses its ownGetValueOrNullforAnalyzerConfigOptions, not the dictionary extension)🤖 Generated with Claude Code