Fix RS1024: Do not report on overloads not comparing symbols#4477
Fix RS1024: Do not report on overloads not comparing symbols#4477ryzngard merged 6 commits intodotnet:masterfrom
Conversation
| var destinationTypeIndex = method.TypeParameters | ||
| .Select((type, index) => type.Name == "TKey" ? index : -1) | ||
| .FirstOrDefault(x => x >= 0); |
There was a problem hiding this comment.
/!\ This is a bit hacky. /!\
I am not sure whether we can safely rely on a generic type parameter name.
As far as I can tell, the non-hacky solution forces us to resolves all overloads, and map them with the index we should look at.
There was a problem hiding this comment.
I'll let @ryzngard review/approve this change.
There was a problem hiding this comment.
Agree this does seem hacky, but maybe no more hacky than depending on ordering. At the end of the day we're at least working on a known set of functions that we don't expect to change drastically, so if it works for the known set maybe it's good enough.
I was looking at the test you wrote without the change, and a surprising piece for me: the type arguments are not in the order I would expect.
Method display is showing ILookup<string, IFieldSymbol> but type arguments are { IFieldSymbol, string }. Going to try and find documentation to see if that's expected.
There was a problem hiding this comment.
I can but this will add some complexity to the current solution. Let me know what's your preference.
Codecov Report
@@ Coverage Diff @@
## master #4477 +/- ##
=======================================
Coverage 95.67% 95.68%
=======================================
Files 1193 1193
Lines 271525 271559 +34
Branches 16413 16415 +2
=======================================
+ Hits 259789 259839 +50
Misses 9620 9620
+ Partials 2116 2100 -16 |
src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs
Outdated
Show resolved
Hide resolved
…eSymbolsCorrectlyAnalyzer.cs Co-authored-by: Manish Vasani <mavasani@microsoft.com>
# Conflicts: # src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs # src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs
src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs
Show resolved
Hide resolved
# Conflicts: # src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs
Fix #4413