Fix false positive for CA1854 across instances #7309
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7309 +/- ##
==========================================
- Coverage 96.50% 96.50% -0.01%
==========================================
Files 1448 1448
Lines 346869 346889 +20
Branches 11402 11404 +2
==========================================
+ Hits 334739 334753 +14
- Misses 9243 9248 +5
- Partials 2887 2888 +1 |
| string syntax1 = instance1.Syntax | ||
| .WithoutTrivia() | ||
| .ToString() | ||
| .Replace("this.", string.Empty) | ||
| .Replace("Me.", string.Empty); | ||
| string syntax2 = instance2.Syntax | ||
| .WithoutTrivia() | ||
| .ToString() | ||
| .Replace("this.", string.Empty) | ||
| .Replace("Me.", string.Empty); |
There was a problem hiding this comment.
NIT: SyntaxNode.ToString() doc says Returns the string representation of this node, not including its leading and trailing trivia.
| string syntax1 = instance1.Syntax | |
| .WithoutTrivia() | |
| .ToString() | |
| .Replace("this.", string.Empty) | |
| .Replace("Me.", string.Empty); | |
| string syntax2 = instance2.Syntax | |
| .WithoutTrivia() | |
| .ToString() | |
| .Replace("this.", string.Empty) | |
| .Replace("Me.", string.Empty); | |
| string syntax1 = instance1.Syntax | |
| .ToString() | |
| .Replace("this.", string.Empty) | |
| .Replace("Me.", string.Empty); | |
| string syntax2 = instance2.Syntax | |
| .ToString() | |
| .Replace("this.", string.Empty) | |
| .Replace("Me.", string.Empty); |
| case IPropertyReferenceOperation source when targetReference is IPropertyReferenceOperation target: | ||
| return target.Property.Equals(source.Property, SymbolEqualityComparer.Default); | ||
| return target.Property.Equals(source.Property, SymbolEqualityComparer.Default) && AreInstancesEqual(source, target); |
There was a problem hiding this comment.
I would expect to add something like && source.Instance == target.Instance, but it doesn't work.
Comparing string representation looks bit weird, though don't see any other way (no action needed here)
| } | ||
|
|
||
| [Fact, WorkItem(7295, "https://github.com/dotnet/roslyn-analyzers/issues/7295")] | ||
| public Task WhenDifferentLocalInstancesWithThisQualifierContainingDictionary_NoDiagnostic() |
There was a problem hiding this comment.
NIT: The test name doesn't match the with the test, seems forgot to update the name or the test, maybe Local part should be removed and NoDiagnostic => Diagnostic
|
Thanks for the review, I have applied your remarks! |
Affected analyzer: PreferDictionaryTryMethodsOverContainsKeyGuardAnalyzer
Affected diagnostic IDs: CA1854
This PR fixes an issue where the analyzer considers two dictionaries equal when they come from different instances of a type.
Fixes #7295