In the code below, the last two references to key are flagged by Resharper, even though Resharper ought to know that key will not be null.
First, the string property is check for Null, Empty, or Whitespace and the block skipped if any of those conditions are true. The extension method, ToLowerNullSafe() will only return Null if the input is Null - and this is marked with an Annotation (2nd code block). Since we've already checked that the property is non-null and the extension method is marked to return non-null, I would expect Resharper to know that key is non-null.
var myObj = new { MyProperty = "some string" };
var myCache = new Dictionary<string, object>();
if (!string.IsNullOrWhiteSpace(myObj.MyProperty))
{
string key = myObj.MyProperty.ToLowerNullSafe();
lock (myCache)
{
if (!myCache.ContainsKey(key))
{
myCache.Add(key, myObj);
}
}
}
These two lines above are where key is flagged:
if (!myCache.ContainsKey(key))
and
myCache.Add(key, myObj);
Here is the ToLowerNullSafe method, with its annotation.
[ContractAnnotation("null => null; notnull => notnull")]
public static string ToLowerNullSafe(this string str)
{
return string.IsNullOrWhiteSpace(str) ? str : str.ToLower();
}
Any ideas on why Resharper appears to be ignoring the annotations? And how do I fix it?
Using:
- Visual Studio Ultimate 2013 Update 5
- Resharper 8.2.3
- Current Annotations from Nuget (both code and external) are applied to the project.
- .Net 4.5.2
EDIT
Redundant Null Check on myObj.MyProperty
Adding a redundant null check (myObj.MyProperty != null) either before or after the !string.IsNullOrWhiteSpace check clears the error.
if (myObj.MyProperty != null && !string.IsNullOrWhiteSpace(myObj.MyProperty))
{
string key = myObj.MyProperty.ToLowerNullSafe();
lock (myCache)
{
if (!myCache.ContainsKey(key))
{
myCache.Add(key, myObj);
}
}
}
Redundant Null Check on key
Adding a redundant check, assertion, or contract assumption can also clear the error.
Here I've added Contract.Assume(key != null); immediately after assigning a value to key.
if (!string.IsNullOrWhiteSpace(myObj.MyProperty))
{
string key = myObj.MyProperty.ToLowerNullSafe();
Contract.Assume(key != null);
lock (myCache)
{
if (!myCache.ContainsKey(key))
{
myCache.Add(key, myObj);
}
}
}
Not calling ToLowerNullSafe
This also clears the error. But since the ContainsKey method performs case-sensitive comparisons where I want a case-insensitive comparison, there could be problems.
if (!string.IsNullOrWhiteSpace(myObj.MyProperty))
{
string key = myObj.MyProperty;
lock (myCache)
{
if (!myCache.ContainsKey(key))
{
myCache.Add(key, myObj);
}
}
}
Calling the built-in string function ToLower
This also clears the error. But would make the code base inconsistent. We have many places in the code where null values can be expected and must be safely handled, so to avoid problems we've applied custom ...NullSafe extension methods throughout our code. I'm not sure if that matches a 'best practice', but it is what we've done on this project.
if (!string.IsNullOrWhiteSpace(myObj.MyProperty))
{
string key = myObj.MyProperty.ToLower();
lock (myCache)
{
if (!myCache.ContainsKey(key))
{
myCache.Add(key, myObj);
}
}
}
With all the working code examples, it seems that Resharper is simply ignoring the Contract Annotations on the ToLowerNullSafe extension method. However, in the first working example, it does recognize those annotations, but by using an explicit and redundant null check on the object's property.
