dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.73k stars 3.99k forks source link

Nullable: review usages of `!=`, `==` and `Equals` on `TypeSymbol` and `TypeSymbolWithAnnotations` #31742

Open jcouv opened 5 years ago

jcouv commented 5 years ago

As pointed out by https://github.com/dotnet/roslyn/issues/31740, some binding code uses strict type comparison, but should be using nullable-insensitive comparison.

Update: I've converted usages of == and != to explicit comparison with ConsiderEverything2. I've yet to track down other implicit comparisons (such as Dictionary<TypeSymbol, ...> or HashSet<TypeSymbol>).

Pilchie commented 5 years ago

If only we had a framework for doing static analysis to avoid mistakes like this 😉

jcouv commented 5 years ago

Indeed. I'm planning to create analyzer/fixer for this. The trouble is that when you switch to .Equals, we should make the comparison option explicit, so that is still a human decision.

CyrusNajmabadi commented 5 years ago

Note: having these comparison options be available at the ISymbol layer would be very helpful. IDE needs to make these sorts of decisions as well. And we've gone about it by trying to reeimplement the compiler rules.

alrz commented 5 years ago

Could we use some Obsolete tricks to forbid those?

jcouv commented 5 years ago

@alrz Yes, that could help, but the main issue is dealing with existing usages (there's many).