¿Por qué este código da una advertencia del compilador "Retorno de referencia nulo posible"?

71

Considere el siguiente código:

using System;

#nullable enable

namespace Demo
{
    public sealed class TestClass
    {
        public string Test()
        {
            bool isNull = _test == null;

            if (isNull)
                return "";
            else
                return _test; // !!!
        }

        readonly string _test = "";
    }
}

Cuando construyo esto, la línea marcada con !!!da una advertencia del compilador: warning CS8603: Possible null reference return..

Esto me parece un poco confuso, dado que _testes de solo lectura e inicializado como no nulo.

Si cambio el código a lo siguiente, la advertencia desaparece:

        public string Test()
        {
            // bool isNull = _test == null;

            if (_test == null)
                return "";
            else
                return _test;
        }

¿Alguien puede explicar este comportamiento?

Matthew Watson
fuente
1
Debug.Assert es irrelevante porque es una verificación de tiempo de ejecución, mientras que la advertencia del compilador es una verificación de tiempo de compilación. El compilador no tiene acceso al comportamiento de tiempo de ejecución.
Polyfun
55
The Debug.Assert is irrelevant because that is a runtime check- Se es relevante porque si comentar que la línea de salida, la advertencia desaparece.
Matthew Watson
1
@Polyfun: El compilador puede saber potencialmente (a través de atributos) que Debug.Assertarrojará una excepción si la prueba falla.
Jon Skeet
2
He estado agregando muchos casos diferentes aquí, y hay algunos resultados realmente interesantes. Escribiré una respuesta más tarde, trabajo por hacer por ahora.
Jon Skeet
2
@EricLippert: Debug.Assertahora tiene una anotación ( src ) de DoesNotReturnIf(false)para el parámetro de condición.
Jon Skeet

Respuestas:

39

El análisis de flujo anulable rastrea el estado nulo de las variables, pero no rastrea otro estado, como el valor de una boolvariable (como isNullarriba), y no rastrea la relación entre el estado de variables separadas (por ejemplo, isNully_test ).

Un motor de análisis estático real probablemente haría esas cosas, pero también sería "heurístico" o "arbitrario" hasta cierto punto: no necesariamente podría decir las reglas que estaba siguiendo, y esas reglas podrían incluso cambiar con el tiempo.

Eso no es algo que podamos hacer directamente en el compilador de C #. Las reglas para las advertencias anulables son bastante sofisticadas (como lo muestra el análisis de Jon), pero son reglas y se pueden razonar.

A medida que implementamos la función, parece que en su mayoría alcanzamos el equilibrio correcto, pero hay algunos lugares que resultan incómodos, y volveremos a visitarlos para C # 9.0.

Mads Torgersen - MSFT
fuente
3
Sabes que quieres poner la teoría de la red en la especificación; ¡La teoría de la red es increíble y nada confusa! ¡Hazlo! :)
Eric Lippert
77
¡Sabes que tu pregunta es legítima cuando el administrador del programa para C # responde!
Sam Rueby
1
@TanveerBadar: la teoría de celosía trata sobre el análisis de conjuntos de valores que tienen un orden parcial; los tipos son un buen ejemplo; si un valor de tipo X es asignable a una variable de tipo Y, entonces eso implica que Y es "lo suficientemente grande" para contener X, y que es suficiente para formar una red, lo que luego nos dice que se podría redactar la verificación de la asignabilidad en el compilador en la especificación en términos de teoría de la red. Esto es relevante para el análisis estático porque una gran cantidad de temas de interés para un analizador además de la asignación de tipos también se pueden expresar en términos de redes.
Eric Lippert
1
@TanveerBadar: lara.epfl.ch/w/_media/sav08:schwartzbach.pdf tiene algunos buenos ejemplos introductorios de cómo los motores de análisis estático usan la teoría de la red.
Eric Lippert
1
@EricLippert Awesome no comienza a describirte. Ese enlace va a mi lista de lectura obligatoria de inmediato.
Tanveer Badar
56

Puedo hacer una suposición razonable sobre lo que está sucediendo aquí, pero todo es un poco complicado :) Involucra el estado nulo y el seguimiento nulo descrito en el borrador de la especificación . Básicamente, en el punto donde queremos regresar, el compilador advertirá si el estado de la expresión es "quizás nulo" en lugar de "no nulo".

Esta respuesta está en forma narrativa en lugar de simplemente "aquí están las conclusiones" ... Espero que sea más útil de esa manera.

Voy a simplificar un poco el ejemplo al eliminar los campos y considerar un método con una de estas dos firmas:

public static string M(string? text)
public static string M(string text)

En las implementaciones a continuación, le he dado a cada método un número diferente para poder referirme a ejemplos específicos sin ambigüedades. También permite que todas las implementaciones estén presentes en el mismo programa.

En cada uno de los casos descritos a continuación, haremos varias cosas, pero terminaremos intentando regresar text, por lo que es el estado nulo detext que es importante el .

Retorno incondicional

Primero, intentemos devolverlo directamente:

public static string M1(string? text) => text; // Warning
public static string M2(string text) => text;  // No warning

Hasta ahora, muy simple. El estado anulable del parámetro al comienzo del método es "tal vez nulo" si es de tipo string?y "no nulo" si es de tipo string.

Retorno condicional simple

Ahora verifiquemos si hay nulo dentro de la ifcondición de la declaración. (Usaría el operador condicional, que creo tendrá el mismo efecto, pero quería estar más fiel a la pregunta).

public static string M3(string? text)
{
    if (text is null)
    {
        return "";
    }
    else
    {
        return text; // No warning
    }
}

public static string M4(string text)
{
    if (text is null)
    {
        return "";
    }
    else
    {
        return text; // No warning
    }
}

Genial, por lo que parece dentro de una ifdeclaración donde la condición misma verifica la nulidad, el estado de la variable dentro de cada rama de la ifdeclaración puede ser diferente: dentro del elsebloque, el estado "no es nulo" en ambas partes del código. Entonces, en particular, en M3 el estado cambia de "quizás nulo" a "no nulo".

Retorno condicional con una variable local

Ahora intentemos elevar esa condición a una variable local:

public static string M5(string? text)
{
    bool isNull = text is null;
    if (isNull)
    {
        return "";
    }
    else
    {
        return text; // Warning
    }
}

public static string M6(string text)
{
    bool isNull = text is null;
    if (isNull)
    {
        return "";
    }
    else
    {
        return text; // Warning
    }
}

Tanto M5 como M6 emiten advertencias. Entonces, no solo no obtenemos el efecto positivo del cambio de estado de "quizás nulo" a "no nulo" en M5 (como lo hicimos en M3) ... obtenemos lo contrario efecto en M6, de dónde va el estado " no nulo "a" tal vez nulo ". Eso realmente me sorprendió.

Parece que hemos aprendido que:

  • La lógica en torno a "cómo se calculó una variable local" no se usa para propagar información de estado. Más sobre eso más tarde.
  • Introducir una comparación nula puede advertir al compilador que algo que antes pensaba que no era nulo podría ser nulo después de todo.

Retorno incondicional después de una comparación ignorada

Veamos el segundo de esos puntos, introduciendo una comparación antes de un retorno incondicional. (Por lo tanto, estamos ignorando por completo el resultado de la comparación):

public static string M7(string? text)
{
    bool ignored = text is null;
    return text; // Warning
}

public static string M8(string text)
{
    bool ignored = text is null;
    return text; // Warning
}

Tenga en cuenta cómo se siente que M8 debería ser equivalente a M2, ambos tienen un parámetro no nulo que devuelven incondicionalmente, pero la introducción de una comparación con nulo cambia el estado de "no nulo" a "quizás nulo". Podemos obtener más evidencia de esto al intentar desreferenciar textantes de la condición:

public static string M9(string text)
{
    int length1 = text.Length;   // No warning
    bool ignored = text is null;
    int length2 = text.Length;   // Warning
    return text;                 // No warning
}

Observe cómo la returndeclaración no tiene una advertencia ahora: el estado después de la ejecución text.Lengthes "no nulo" (porque si ejecutamos esa expresión con éxito, no podría ser nula). Por lo tanto, el textparámetro comienza como "no nulo" debido a su tipo, se convierte en "quizás nulo" debido a la comparación nula, luego se vuelve "no nulo" nuevamente text2.Length.

¿Qué comparaciones afectan al estado?

Entonces, esa es una comparación de text is null... ¿qué efecto tienen comparaciones similares? Aquí hay cuatro métodos más, todos comenzando con un parámetro de cadena no anulable:

public static string M10(string text)
{
    bool ignored = text == null;
    return text; // Warning
}

public static string M11(string text)
{
    bool ignored = text is object;
    return text; // No warning
}

public static string M12(string text)
{
    bool ignored = text is { };
    return text; // No warning
}

public static string M13(string text)
{
    bool ignored = text != null;
    return text; // Warning
}

Entonces, aunque x is objectahora es una alternativa recomendada x != null, no tienen el mismo efecto: solo una comparación con nulo (con cualquiera de is, ==o !=) cambia el estado de "no nulo" a "quizás nulo".

¿Por qué alzar la condición tiene un efecto?

Volviendo a nuestro primer punto anterior, ¿por qué M5 y M6 no tienen en cuenta la condición que condujo a la variable local? Esto no me sorprende tanto como parece sorprender a los demás. Construir ese tipo de lógica en el compilador y la especificación es mucho trabajo y tiene relativamente poco beneficio. Aquí hay otro ejemplo que no tiene nada que ver con la nulabilidad en el que la alineación de algo tiene un efecto:

public static int X1()
{
    if (true)
    {
        return 1;
    }
}

public static int X2()
{
    bool alwaysTrue = true;
    if (alwaysTrue)
    {
        return 1;
    }
    // Error: not all code paths return a value
}

A pesar de que sabemos que alwaysTruesiempre habrá cierto, que no satisface los requisitos de la especificación que hacen que el código después de la ifdeclaración inalcanzable, que es lo que necesitamos.

Aquí hay otro ejemplo, alrededor de la asignación definida:

public static void X3()
{
    string x;
    bool condition = DateTime.UtcNow.Year == 2020;
    if (condition)
    {
        x = "It's 2020.";
    }
    if (!condition)
    {
        x = "It's not 2020.";
    }
    // Error: x is not definitely assigned
    Console.WriteLine(x);
}

A pesar de que sabemos que el código entrará exactamente uno de esos ifcuerpos de los estados, no hay nada en la especificación de trabajo que fuera. Las herramientas de análisis estático pueden ser capaces de hacerlo, pero tratar de poner eso en la especificación del lenguaje sería una mala idea, en mi opinión: está bien que las herramientas de análisis estático tengan todo tipo de heurísticas que pueden evolucionar con el tiempo, pero no tanto para una especificación de idioma.

Jon Skeet
fuente
77
Gran análisis Jon. Lo clave que aprendí al estudiar el verificador de Coverity es que el código es evidencia de las creencias de sus autores . Cuando vemos una verificación nula que debería informarnos que los autores del código creían que la verificación era necesaria. El verificador en realidad está buscando evidencia de que las creencias de los autores eran inconsistentes porque son los lugares donde vemos creencias inconsistentes sobre, digamos, la nulidad, que ocurren los errores.
Eric Lippert
66
Cuando vemos, por ejemplo if (x != null) x.foo(); x.bar();, tenemos dos pruebas; la ifdeclaración es evidencia de la proposición "el autor cree que x podría ser nulo antes del llamado a engañar" y la siguiente declaración es evidencia de "el autor cree que x no es nulo antes del llamado a prohibir", y esta contradicción lleva a la conclusión de que hay un error. El error es el error relativamente benigno de una comprobación nula innecesaria o el error potencialmente fallido. Qué error es el error real no está claro, pero está claro que hay uno.
Eric Lippert
1
El problema de que los verificadores relativamente poco sofisticados que no rastrean los significados de los lugareños y no podan los "caminos falsos", controlan los caminos de flujo que los humanos pueden decir que son imposibles, tienden a producir falsos positivos precisamente porque no han modelado con precisión el creencias de los autores. Esa es la parte difícil!
Eric Lippert
3
La inconsistencia entre "es objeto", "es {}" y "! = Nulo" es un elemento que hemos estado discutiendo internamente las últimas semanas. Vamos a mencionarlo en LDM en un futuro muy cercano para decidir si necesitamos considerarlos como verificaciones nulas puras o no (lo que hace que el comportamiento sea consistente).
JaredPar
1
@ArnonAxelrod Eso dice que no está destinado a ser nulo. Todavía podría ser nulo, porque los tipos de referencia anulables son solo una sugerencia del compilador. (Ejemplos: M8 (¡nulo!); O llamarlo desde el código C # 7, o ignorar las advertencias.) No es como el tipo de seguridad del resto de la plataforma.
Jon Skeet
29

Ha descubierto evidencia de que el algoritmo de flujo de programa que produce esta advertencia es relativamente poco sofisticado cuando se trata de rastrear los significados codificados en variables locales.

No tengo conocimiento específico de la implementación del verificador de flujo, pero después de haber trabajado en implementaciones de código similar en el pasado, puedo hacer algunas conjeturas. Es probable que el verificador de flujo deduzca dos cosas en el caso de falso positivo: (1) _testpodría ser nulo, porque si no pudiera, no tendría la comparación en primer lugar, y (2)isNull podría ser verdadero o falso, porque si no pudiera, no lo tendría en un if. Pero la conexión que return _test;solo se ejecuta si _testno es nula, esa conexión no se está haciendo.

Este es un problema sorprendentemente complicado, y debe esperar que al compilador le tome un tiempo lograr la sofisticación de las herramientas que han tenido varios años de trabajo por parte de expertos. El verificador de flujo Coverity, por ejemplo, no tendría ningún problema en deducir que ninguna de sus dos variaciones tuvo un rendimiento nulo, pero el verificador de flujo Coverity cuesta mucho dinero para los clientes corporativos.

Además, los verificadores de Coverity están diseñados para ejecutarse en grandes bases de código durante la noche ; El análisis del compilador de C # debe ejecutarse entre las pulsaciones de teclas en el editor , lo que cambia significativamente los tipos de análisis en profundidad que puede realizar razonablemente.

Eric Lippert
fuente
"No sofisticado" es correcto: considero que es perdonable si se topa con cosas como condicionales, ya que todos sabemos que el problema de detención es un poco difícil en tales asuntos, pero el hecho de que hay una diferencia entre bool b = x != nullvs bool b = x is { }(con ¡ninguna asignación realmente utilizada!) muestra que incluso los patrones reconocidos para las comprobaciones nulas son cuestionables. Para no menospreciar el trabajo indudablemente duro del equipo para hacer que esto funcione principalmente como debería para las bases de código reales en uso, parece que el análisis es pragmático capital P.
Jeroen Mostert
@JeroenMostert: Jared Par menciona en un comentario sobre la respuesta de Jon Skeet que Microsoft está discutiendo ese problema internamente.
Brian
8

Todas las otras respuestas son bastante correctas.

En caso de que alguien tenga curiosidad, traté de explicar la lógica del compilador de la manera más explícita posible en https://github.com/dotnet/roslyn/issues/36927#issuecomment-508595947

La única parte que no se menciona es cómo decidimos si un cheque nulo debe considerarse "puro", en el sentido de que si lo hace, deberíamos considerar seriamente si nulo es una posibilidad. Hay una gran cantidad de comprobaciones nulas "incidentales" en C #, donde se realiza una prueba de nulidad como parte de hacer otra cosa, por lo que decidimos que queríamos reducir el conjunto de comprobaciones a las que estábamos seguros de que la gente hacía deliberadamente. La heurística que se nos ocurrió fue "contiene la palabra nulo", así que es por eso x != nully x is objectproduce resultados diferentes.

Andy Gocke
fuente