dotnet / runtime

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

Operations Which Have a Default StringComparer Should Strongly Suggest Explicitly Specifying the StringComparer #47775

Open aolszowka opened 3 years ago

aolszowka commented 3 years ago

Brief description: Background on the issue here: https://github.com/dotnet/runtime/issues/43802

The default string comparer can have different results based on the running environment (such as platform or culture). Here's an earlier instance of this expected behavior, specifically about Windows vs. macOS/Linux and the ICU library: dotnet/roslyn#20109.

It's generally best practice to avoid using string comparison defaults so you don't run into this kind of issue: https://docs.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings#specifying-string-comparisons-explicitly

This is a real gotcha in my mind for anyone new to the ecosystem.

Historically I have used this excellent Roslyn Analyzer https://github.com/meziantou/Meziantou.Analyzer to catch similar issues, however I feel this is important enough to be something that should be out of the box. I have filed a request with that project as well here https://github.com/meziantou/Meziantou.Analyzer/issues/196.

Languages applicable: All Languages

Code example that the analyzer should report: SortedDictionary<string, int> sortedDictionary = new SortedDictionary<string, int>();

Is one such example that show throw a warning to indicate that you have not specified the StringComparer

The linked repository in the issue (https://github.com/aolszowka/DotNetCoreSortedDictionaryCrossPlatform) shows what happens cross platform when this is not followed.

dotnet-issue-labeler[bot] commented 3 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.