Closed craigktreasure closed 2 years ago
However, you can't fix the warning by using
SymbolEqualityComparer.Default
because it causes the wrong type to be returned.
You can, if you take advantage of contravariance of IEqualityComparer
:
symbols.ToDictionary(s => s, s => s.ToDisplayString(), (IEqualityComparer<ITypeSymbol>)SymbolEqualityComparer.Default);
Though it's probably better to take advantage of C# 10 and instead explicitly specify the return type of the first lambda:
symbols.ToDictionary(ITypeSymbol (s) => s, s => s.ToDisplayString(), SymbolEqualityComparer.Default);
IDictionary<ITypeSymbol, string> symbolNameMap = symbols.ToDictionary(s => s, s => s.ToDisplayString(), TypeSymbolEqualityComparer.Default);
But again, the analyzer isn't satisfied.
This one seems like a bug to me. A custom equality comparer shouldn't violate RS1024. Other than that, I agree with @svick
@svick Thanks! I didn't think to cast the silly comparer. That seems to be the only solution for extensions like .Distinct(...)
and .Union(...)
. I just updated to .NET 6, so I can use the C# 10 trick you suggested for .ToDictionary(...)
.
As @Youssef1313 said, I still think it's reasonable to be able to use a custom comparer.
I believe my issue is related: I'm facing a RS1024
when passing a custom IEqualityComparer<ISymbol>
to HashSet`1
.
_ = {|#0:new HashSet<ISymbol>(SymbolNameComparer.Instance)|};
internal sealed class SymbolNameComparer : EqualityComparer<ISymbol>
{
private SymbolNameComparer() { }
internal static IEqualityComparer<ISymbol> Instance { get; } = new SymbolNameComparer();
public override bool Equals(ISymbol? x, ISymbol? y)
{
Debug.Assert(x is not null, $"{nameof(x)} not expected to be null");
Debug.Assert(y is not null, $"{nameof(y)} not expected to be null");
return x.Name.Equals(y.Name, StringComparison.Ordinal);
}
public override int GetHashCode([DisallowNull] ISymbol obj)
=> obj.Name.GetHashCode();
}
Location 0: Warning | RS1024 | Use 'SymbolEqualityComparer' when comparing symbols
<!-- .csproj -->
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.3" PrivateAssets="all" />
</ItemGroup>
@Flash0ver https://github.com/dotnet/roslyn-analyzers/pull/5807 should fix it.
@Youssef1313 oh, wow - super quick, thank you. Is there perhaps a pre-release package I may try? If so, I'm afraid I'm unaware of the feed.
Links for pre-releases are in the README.
You can try witg the next prerelease after the PR gets merged.
Thanks - fixed my issue.
Analyzer
Diagnostic ID: RS1024:
Compare symbols correctly
Analyzer source
SDK: Built-in CA analyzers in .NET 6 SDK or later
Version: SDK 6.0.100
AND
NuGet Package: Microsoft.CodeAnalysis.Analyzers
Version: 3.3.3 (Latest)
Describe the bug
When using the
Enumerable.ToDictionary(...)
extension method on something like anIEnumerable<ITypeSymbol>
, it is not possible (from what I can tell) to satisfy the analyzer.Steps To Reproduce
Consider the following example code:
However, you can't fix the warning by using
SymbolEqualityComparer.Default
because it causes the wrong type to be returned. In this case anIDictionary<ISymbol, string>
instead of an<IDictionary<ITypeSymbol, string>
.So, I figured I'd create an implementation of an
IEqualityComparer<ITypeSymbol?>
, since one doesn't already exist, using something like the following:Then, back in our code, we'd write the following:
But again, the analyzer isn't satisfied.
Expected behavior
I can cleanly compare symbols properly using the
Enumerable.ToDictionary(...)
extension without getting an RS1024 warning.Actual behavior
I can't cleanly compare symbols without getting the RS1024 warning, so I disable the warning.
Additional information
The same appears to be true for other extension methods like
Enumerable.Union(...)
.