Analyzers did become an integral part of the .NET ecosystem. Their main responsibility is to find potential code issues and warn you. Often times this comes even with potential fixes you can directly apply.
And Microsoft will continue this journey with the upcoming .NET 8 release. This blog post will show you potential candidates, which will make the cut. Sure .NET 8 is in the (maybe not so far) future, with an expected release date of November 2023, but anyway, that article can be of great value. All the potential issues those analyzers will detect, you might find them in your code today.
Enabling preview analyzers
To enable Analyzers you can edit your csproj-file and add the appropriate flag. If you download a preview version of .NET you can also enable the preview analyzers like that:
<PropertyGroup Label="Analyzer settings">
<AnalysisLevel>preview</AnalysisLevel>
</PropertyGroup>
Currently supported analyzers
You might be surprised to find out, that as of today we have over 100 rules in place, which will warn you in certain situations. For a comprehensive list, check out the GitHub page. You can see which additions come into with each new framework version.
The following sections will go over possible new additions, as usual, I will link to the GitHub tracker as well as explain what issue the new analyzer tries to solve.
Recommend string.ToLower().Contains/IndexOf/StartsWith("lowercasestring") be replaced by case-insensitive comparison
In its core it's about simple things like:
bool IsAdminUser(string userName) => userName.ToLowerCase() == "admin";
The problem is, that with ToLowerCase
, as well as ToUpperCase
we introduce new allocations. string
s are immutable so if you create a new representation, you have to create new memory blocks. So you should use something like this instead:
bool IsAdminUser(string userName)
=> string.Equals(userName,"admin", StringComparison.OrdinalIgnoreCase);
This is faster and uses no allocations. Here is a small example in action:
[MemoryDiagnoser]
[HideColumns(Column.Arguments)]
public class StringBenchmark
{
[Benchmark(Baseline = true)]
[Arguments(
"HellO WoRLD, how are you? You are doing good?",
"hElLO wOrLD, how Are you? you are doing good?")]
public bool AreEqualToLower(string a, string b) => a.ToLower() == b.ToLower();
[Benchmark(Baseline = false)]
[Arguments(
"HellO WoRLD, how are you? You are doing good?",
"hElLO wOrLD, how Are you? you are doing good?")]
public bool AreEqualStringComparison(string a, string b)
=> string.Equals(a, b, StringComparison.OrdinalIgnoreCase);
}
Results in:
| Method | Mean | Error | StdDev | Ratio | Gen0 | Allocated | Alloc Ratio |
|------------------------- |---------:|---------:|---------:|------:|-------:|----------:|------------:|
| AreEqualToLower | 60.93 ns | 1.008 ns | 0.943 ns | 1.00 | 0.0356 | 224 B | 1.00 |
| AreEqualStringComparison | 16.10 ns | 0.030 ns | 0.028 ns | 0.26 | - | - | 0.00 |
This will go into the performance section of the analyzers.
Link: GitHub
Using StartsWith instead of IndexOf == 0
The analyzer will flag code, which looks like this:
bool IndexOf(string haystack, string needle)
=> haystack.IndexOf(needle, StringComparison.OrdinalIgnoreCase) == 0;
Why is that bad? Well, the problem with IndexOf
is, that it will go through the whole string in the worst case. StartsWith
on the contrary, will directly abort one the first mismatch.
[Benchmark(Baseline = true)]
[Arguments("That is a sentence", "Thzt")]
public bool IndexOf(string haystack, string needle)
=> haystack.IndexOf(needle, StringComparison.OrdinalIgnoreCase) == 0;
[Benchmark]
[Arguments("That is a sentence", "Thzt")]
public bool StartsWith(string haystack, string needle)
=> haystack.StartsWith(needle, StringComparison.OrdinalIgnoreCase);
Results:
| Method | haystack | needle | Mean | Error | StdDev | Ratio |
|----------- |------------------- |------- |----------:|----------:|----------:|------:|
| IndexOf | That is a sentence | Thzt | 21.966 ns | 0.1584 ns | 0.1482 ns | 1.00 |
| StartsWith | That is a sentence | Thzt | 3.066 ns | 0.0142 ns | 0.0126 ns | 0.14 |
You guessed it, it also goes into the performance category.
Link: GitHub
Prefer .Length/Count/IsEmpty over Any()
Another one for the performance category. If you have a collection, which is strongly-typed as ICollection<T>
or has a property like Length
(T[]
) or Count
(List<T>
), then you are better off to use those properties in contrast to Any()
.
List<string> names = GetNamesFromStroage();
// This would be flagged by the new analyzer
var isNotEmpty = names.Any();
// This would not get flagged and is "preferred"
var isNotEmpty = names.Count != 0;
You might wonder, why should this be faster. After all, it is in the performance category. Well, two things to consider: 1. Any
does type-checking, which costs CPU cycles. Length
or Count
are just easy properties to evaluate against. 2. Any
will not be inlined, Length != 0
might get inlined. Yes in tight loops, that can make a difference.
Now this one is a subjective one. I know a lot of people that would prefer Any
anyday over Length != 0
. So I do think that this hint might be set to quite in the beginning and will not actively warn you. Let's see, time will tell!
Link: GitHub
Prevent wrong usage of string Split
I will just quote directly from the GitHub-Tracker as it is easily described there:
Consider the following usage of Split on a string where you want to split on space and tab:
var split = s.Split(' ', '\t');
(which works fine)
If you now get to the conclusion to remove empty entries, it is tempting to do this:
var split = s.Split(' ', '\t', StringSplitOptions.RemoveEmptyEntries);
But this won't give the expected result and will result in a bug in the application because the second char
is converted to an int
and used as the count argument for this overload:
public string[] Split(char separator, int count, StringSplitOptions options = StringSplitOptions.None);
Link: GitHub
Hoist explicit calls to HttpContext.RequestServices.GetService
to route handler parameters
This one is from the aspnet core team.Instead of using the HttpContext
for retrieving from the container, one should use the parameters and let ASP.NET Core take care of it.
This violates:
app.MapGet("/", (HttpContext context) =>
{
var service = context.RequestServices.GetRequiredService<ITodoService>();
return service.GetTodos();
});
This would fix it:
app.MapGet("/", (ITodoService service) =>
{
return service.GetTodos();
});
Link: GitHub
Warnings for in
/readonly ref
For primitive value types like int
, double
, byte
passing by the in
or readonly ref
keyword is probable slower than just creating the simple copy in the register.
Link: GitHub
NonCopyable structs attribute and analyzer
For this I personally waited a long time and I am looking forward to this one. Normally if you pass value types to a function, these value types get copied. Okay fair enough, so what is the problem? Let's assume you write some high performance API (like the ValueStringBuilder I wrote a while back) are defined as (ref) struct to avoid GC heap allocations. Consumers of those types have to be careful that the object doesn't get copied all over the place. And that is where the new attribute comes into play.
[NonCopyable]
public ref struct ValueStringBuilder
{
...
}
ValueStringBuilder vsb = new ValueStringBuilder();
f(vsb); // Error. ValueStringBuilder must by passed by reference
Conclusion
There you have it folks. Some of the new analyzers planned for .NET 8. I hope you are excited as I am. At least some of the new analyzers and their implications were new to me.