dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

Analyzer to prefer equality operator over object.equals calls for enums #41567

Open Mrnikbobjeff opened 4 years ago

Mrnikbobjeff commented 4 years ago

Enums shoud always be compared by using == when testing for equality. In Xamarin.Essentials I saw the use of .Equals calls to compare enums for equality, which does the same but incurs a box for enums. A roslyn analyzer can easily detect calls to object.Equals calls where the argument is an enum. I already have a working implementation of this analyzer if this is a desirable feature.

Dotnet-GitSync-Bot commented 4 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 4 years ago

Tagging subscribers to this area: @CoffeeFlux See info in area-owners.md if you want to be subscribed.

CoffeeFlux commented 4 years ago

What is the exact suggestion here—that Roslyn add an analyzer for this or that Xamarin.Essentials switch what they're using? In either case, I think this issue is better filed in the respective repo.

Mrnikbobjeff commented 4 years ago

Xamarin.Essentials was the first place where I found this pattern. As there are other analyzer suggestions listed here such as #33773 and the board I thought this might be the place for it.

jeffhandley commented 4 years ago

Thanks for this feature idea and the offer to contribute an analyzer, @Mrnikbobjeff.

I'm going to move this into our Future milestone as we wouldn't be able to include this in 5.0 at this point. @terrajobst / @bartonjs, would we want to do a full API review for this one, or do you think this is straightforward enough to give the go-ahead for a PR into dotnet/roslyn-analyzers (for after 5.0)?