dotnet / roslyn-analyzers

MIT License
1.59k stars 465 forks source link

False positives for RS1024 (Compare symbols correctly) for symbols without nullability #4410

Open KrisVandermotten opened 3 years ago

KrisVandermotten commented 3 years ago

Comparing instances of ITypeSymbol using their IEquatable<ISymbol> interface could be a bug, because of NRT. RS1024 was introduced to discover those possible bugs early, and force the author to be explicit about the symbol equality comparer to be used. Obviously, the rule should apply when comparing ITypeSymbol instances, but also when comparing ISymbol instances, since those just might be ITypeSymbol instances.

However, the rule should not apply to types implementing IEquatable<ISymbol> that do not inherit from ITypeSymbol. Examples include IAssemblySymbol, IParameterSymbol and others.

Steps To Reproduce

Consider this code:

    private void M(SyntaxNodeAnalysisContext context, ISymbol symbol)
    {
        if (symbol.ContainingAssembly.Equals(context.Compilation.Assembly))
        {
            // ...
        }
    }

Expected behavior

No diagnostic is reported, since there is no ambiguity in comparing IAssemblySymbol instances.

Actual behavior

warning RS1024: Compare symbols correctly
sharwell commented 3 years ago

@333fred @mavasani Do you know which symbols can never have nullability information?

sharwell commented 3 years ago

It looks this applies to all member symbols (IFieldSymbol, IEventSymbol, IMethodSymbol, IParameterSymbol, etc.). This narrows down the potential false positives to:

mavasani commented 3 years ago

Tagging @Evangelink