How to properly fix this code to support nullable references without warnings

user_622140 0
Viewed 303

Using the following project file settings:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net5.0</TargetFrameworks>
    <LangVersion>9.0</LangVersion>
    <Nullable>enable</Nullable>
    <EnableNETAnalyzers>true</EnableNETAnalyzers>
    <AnalysisLevel>latest</AnalysisLevel>
    <EnableNETAnalyzers>true</EnableNETAnalyzers>
    <AnalysisMode>AllEnabledByDefault</AnalysisMode>
  </PropertyGroup>
</Project>

with the following code:

using System;

namespace TestArrayValueHashCodeNullability
{
    public static class Test
    {
        /// <summary>
        /// Gets a hash code for the array based on the values in the array.
        /// </summary>
        /// <typeparam name="TYPE">The type of item in the array.</typeparam>
        /// <param name="array">The array to get the value hash code for.</param>
        /// <returns>A hash code based on the values in the array.</returns>
        public static int ValueHashCode<TYPE>(this TYPE[] array)
        {
            if (array == null) throw new ArgumentNullException(nameof(array));
            int code = array.Length;
            // loop through each element and compare
            for (int offset = 0; offset < array.Length; ++offset)
            {
                int elemhashcode;
                elemhashcode = array[offset]?.GetHashCode() ?? 0;   // either this line should work
                elemhashcode = array[offset].GetHashCode();         // or this one should
                code ^= (elemhashcode >> (32 - (offset % 32)) ^ (elemhashcode << (offset % 32)) ^ 0x1A7FCA3B);
            }
            return code;
        }
    }
}

I get the the following warnings on the two alternative elemhashcode = lines, which appear to be in direct conflict with each other. I believe that one of the lines should compile without a warning, as array[offset] is possibly null on that line or it is not--it cannot be both "possibly null" and "never null".

Test.cs(21,32,21,45): warning CA1508: 'array[offset]' is never 'null'. Remove or refactor the condition(s) to avoid dead code.
Test.cs(21,32,21,60): warning CA1508: 'array[offset]?.GetHashCode()' is never 'null'. Remove or refactor the condition(s) to avoid dead code.
Test.cs(22,32,22,45): warning CS8602: Dereference of a possibly null reference.

I can get the warning to go away using the following code:

                elemhashcode = array[offset]!.GetHashCode();

but that shouldn't be necessary. One or the other should be technically correct. Is this a compiler bug, or am I missing some subtle solution to this?

c#-9.0 c#
1 Answers

This seems to me to be a disparity between the Code Analysis feature, and the compiler itself.

Given the code you posted, the error on line 22 is the critical one. You've done nothing to constrain the type parameter, and so indeed it is possible that a nullable reference type could be used, and so you are dereferencing a potentially null reference.

The warnings on line 21 appear to me to be erroneous. However, note that they show up only when you have included <AnalysisMode>AllEnabledByDefault</AnalysisMode> in the project file. Indeed, it seems that there are known false-positive results for CA1508:

mavasani commented on Mar 28, 2019

This analyzer has known false positives and hence is turned off by default. I would recommend you remove the ruleset entry that is turning on this rule. When the false positives are fixed, we will enable this rule by default.

Which version of the code is correct depends not on the warnings you get, but on what your intent for the code actually is. If you expect for the type used for the type parameter to always be non-nullable, then you should add the constraint where TYPE : notnull to the method declaration, and keep the elemhashcode = array[offset].GetHashCode(); statement. With the proper constraint reflecting your intended use of the generic method, that warning will go away.

On the other hand, if you want the TYPE type parameter to allow nullable types, you need to protect against null values as in the first statement, i.e. elemhashcode = array[offset]?.GetHashCode() ?? 0;. In which case, having all of the rules enabled will result in that known false-positive.

In that case, if you feel you really need to retain AllEnabledByDefault for the project's Code Analysis settings, you can suppress that specific warning in a variety of ways, such as a #pragma in the code, a source- or assembly-level [SuppressMessage] attribute, or a code-analysis configuration file. See How to suppress code analysis warnings for more details about that.

user_35380120
Related