Por que esse código fornece um aviso do compilador "Possível retorno de referência nula"?

70

Considere o seguinte 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 = "";
    }
}

Quando eu construir este, a linha marcada com !!!dá um aviso do compilador: warning CS8603: Possible null reference return..

Acho isso um pouco confuso, já que _testé somente leitura e inicializado como não nulo.

Se eu alterar o código para o seguinte, o aviso desaparecerá:

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

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

Alguém pode explicar esse comportamento?

Matthew Watson
fonte
11
O Debug.Assert é irrelevante porque é uma verificação de tempo de execução, enquanto o aviso do compilador é uma verificação de tempo de compilação. O compilador não tem acesso ao comportamento em tempo de execução.
Polyfun
5
The Debug.Assert is irrelevant because that is a runtime check- Ele é relevante porque se você comentar que fora de linha, o aviso vai embora.
Matthew Watson
11
@ Polyfun: O compilador pode saber (via atributos) que Debug.Assertlançará uma exceção se o teste falhar.
Jon Skeet
2
Venho adicionando muitos casos diferentes aqui, e há alguns resultados realmente interessantes. Irá escrever uma resposta mais tarde - trabalho a fazer por enquanto.
Jon Skeet
2
@ EricLippert: Debug.Assertagora tem uma anotação ( src ) DoesNotReturnIf(false)para o parâmetro de condição.
Jon Skeet

Respostas:

38

A análise de fluxo anulável rastreia o estado nulo das variáveis, mas não rastreia outro estado, como o valor de uma boolvariável (como isNullacima), e não rastreia o relacionamento entre o estado de variáveis ​​separadas (por exemplo, isNulle _test).

Um mecanismo de análise estática real provavelmente faria essas coisas, mas também seria "heurístico" ou "arbitrário" até certo ponto: você não podia necessariamente dizer as regras que ele estava seguindo, e essas regras podem até mudar ao longo do tempo.

Isso não é algo que podemos fazer diretamente no compilador C #. As regras para avisos anuláveis ​​são bastante sofisticadas (como mostra a análise de Jon!), Mas são regras e podem ser fundamentadas.

À medida que lançamos o recurso, parece que alcançamos o equilíbrio certo, mas há alguns lugares que parecem desajeitados e revisaremos os do C # 9.0.

Mads Torgersen - MSFT
fonte
3
Você sabe que deseja colocar a teoria da rede na especificação; a teoria da rede é impressionante e nada confusa! Faça! :)
Eric Lippert
7
Você sabe que sua pergunta é legítima quando o gerente do programa para C # responde!
Sam Rueby 13/12/19
11
@TanveerBadar: a teoria da treliça trata da análise de conjuntos de valores que têm uma ordem parcial; tipos são um bom exemplo; se um valor do tipo X é atribuível a uma variável do tipo Y, isso implica que Y é "grande o suficiente" para conter X e isso é suficiente para formar uma treliça, o que nos diz que a verificação da atribuibilidade no compilador pode ser redigida na especificação em termos da teoria da rede. Isso é relevante para a análise estática, porque muitos tópicos de interesse de um analisador que não sejam atribuíveis ao tipo também são expressáveis ​​em termos de redes.
Eric Lippert
11
@TanveerBadar: lara.epfl.ch/w/_media/sav08:schwartzbach.pdf tem alguns bons exemplos introdutórios de como os mecanismos de análise estática usam a teoria da rede.
Eric Lippert
11
@EricLippert Awesome não começa a descrevê-lo. Esse link está entrando na minha lista de leitura obrigatória imediatamente.
Tanveer Badar
56

Posso adivinhar razoavelmente o que está acontecendo aqui, mas é um pouco complicado :) Envolve o estado nulo e o rastreamento nulo descritos no rascunho da especificação . Fundamentalmente, no ponto em que queremos retornar, o compilador avisará se o estado da expressão for "talvez nulo" em vez de "não nulo".

Esta resposta é de certa forma narrativa, e não apenas "aqui estão as conclusões" ... Espero que seja mais útil dessa maneira.

Vou simplificar um pouco o exemplo, livrando-me dos campos e considere um método com uma dessas duas assinaturas:

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

Nas implementações abaixo, dei a cada método um número diferente para que eu possa consultar exemplos específicos sem ambiguidade. Ele também permite que todas as implementações estejam presentes no mesmo programa.

Em cada um dos casos descritos abaixo, faremos várias coisas, mas acabamos tentando retornar text- por isso é importante o estado nulo text.

Retorno incondicional

Primeiro, vamos tentar devolvê-lo diretamente:

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

Até agora, tão simples. O estado anulável do parâmetro no início do método é "talvez nulo" se for do tipo string?e "não nulo" se for do tipo string.

Retorno condicional simples

Agora vamos verificar se há nulo na ifprópria condição da instrução. (Eu usaria o operador condicional, que acredito ter o mesmo efeito, mas queria permanecer mais fiel à questão.)

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
    }
}

Ótimo, portanto, dentro de uma ifinstrução em que a própria condição verifica a nulidade, o estado da variável em cada ramo da ifinstrução pode ser diferente: dentro do elsebloco, o estado "não é nulo" em ambas as partes do código. Assim, em particular, no M3 o estado muda de "talvez nulo" para "não nulo".

Retorno condicional com uma variável local

Agora vamos tentar elevar essa condição a uma variável 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
    }
}

Ambos M5 e M6 avisos. Portanto, não apenas não obtemos o efeito positivo da mudança de estado de "talvez nulo" para "não nulo" no M5 (como fizemos no M3) ... obtemos o efeito oposto no M6, de onde o estado passa " não nulo "a" talvez nulo ". Isso realmente me surpreendeu.

Parece que aprendemos isso:

  • A lógica em torno de "como uma variável local foi calculada" não é usada para propagar informações de estado. Mais sobre isso mais tarde.
  • A introdução de uma comparação nula pode avisar o compilador de que algo que antes pensava não ser nulo poderia ser nulo, afinal.

Retorno incondicional após uma comparação ignorada

Vejamos o segundo desses pontos, introduzindo uma comparação antes de um retorno incondicional. (Portanto, estamos ignorando completamente o resultado da comparação.):

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
}

Observe como M8 parece que deve ser equivalente a M2 - ambos têm um parâmetro não nulo que retornam incondicionalmente - mas a introdução de uma comparação com nulo altera o estado de "não nulo" para "talvez nulo". Podemos obter mais evidências disso tentando remover a referência textantes da condição:

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 como a returninstrução não possui um aviso agora: o estado após a execução text.Lengthé "não nulo" (porque se executarmos essa expressão com sucesso, ela não poderá ser nula). Portanto, o textparâmetro começa como "não nulo" devido ao seu tipo, torna-se "talvez nulo" devido à comparação nula e depois torna-se "não nulo" novamente depois text2.Length.

Quais comparações afetam o estado?

Então isso é uma comparação de text is null... que efeito as comparações semelhantes têm? Aqui estão mais quatro métodos, todos começando com um parâmetro de sequência não anulável:

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
}

Portanto, embora x is objectagora seja uma alternativa recomendada x != null, eles não têm o mesmo efeito: somente uma comparação com null (com qualquer um de is, ==ou !=) altera o estado de "not null" para "maybe null".

Por que içar a condição tem efeito?

Voltando ao nosso primeiro marcador, por que M5 e M6 não levam em consideração a condição que levou à variável local? Isso não me surpreende tanto quanto parece surpreender os outros. Construir esse tipo de lógica no compilador e na especificação é muito trabalhoso e com relativamente pouco benefício. Aqui está outro exemplo que não tem nada a ver com a nulidade, em que algo embutido tem efeito:

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
}

Mesmo sabendo que isso alwaysTruesempre será verdade, ele não satisfaz os requisitos da especificação que tornam o código após a ifinstrução inacessível, e é disso que precisamos.

Aqui está outro exemplo, em torno de atribuição 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);
}

Mesmo que nós sabemos que o código vai entrar exatamente um daqueles ifcorpos declaração, não há nada na especificação de trabalho que fora. As ferramentas de análise estática podem muito bem fazê-lo, mas tentar colocar isso na especificação da linguagem seria uma má idéia, IMO - é bom que as ferramentas de análise estática tenham todos os tipos de heurísticas que podem evoluir ao longo do tempo, mas não tanto. para uma especificação de idioma.

Jon Skeet
fonte
7
Ótima análise Jon. A principal coisa que aprendi ao estudar o verificador Coverity é que o código é uma evidência das crenças de seus autores . Quando vemos uma verificação nula, isso deve nos informar que os autores do código acreditavam que a verificação era necessária. O verificador está realmente procurando evidências de que as crenças dos autores eram inconsistentes, porque são os lugares em que vemos crenças inconsistentes sobre, digamos, nulidade, que os erros acontecem.
Eric Lippert
6
Quando vemos, por exemplo if (x != null) x.foo(); x.bar();, temos duas evidências; a ifafirmação é evidência da proposição "o autor acredita que x pode ser nulo antes da chamada para foo" e a seguinte afirmação é evidência de "o autor acredita que x não é nula antes da chamada para barrar", e essa contradição leva ao conclusão de que existe um erro. O bug é o bug relativamente benigno de uma verificação nula desnecessária ou o bug potencialmente travando. Qual bug é o bug real não está claro, mas é claro que existe um.
Eric Lippert
11
O problema de verificadores relativamente pouco sofisticados que não rastreiam os significados dos habitantes locais e não eliminam "caminhos falsos" - controlam caminhos de fluxo que os humanos podem dizer que são impossíveis - tendem a produzir falsos positivos precisamente porque não modelaram com precisão o crenças dos autores. Essa é a parte complicada!
Eric Lippert
3
A inconsistência entre "é objeto", "é {}" e "! = Null" é um item que discutimos internamente nas últimas semanas. Vamos abordar o assunto no LDM em um futuro próximo para decidir se precisamos considerá-las como verificações nulas puras ou não (o que torna o comportamento consistente).
JaredPar
11
@ArnonAxelrod Isso diz que não é para ser nulo. Ainda pode ser nulo, porque os tipos de referência anuláveis ​​são apenas uma dica do compilador. (Exemplos: M8 (nulo!); Ou chamá-lo do código C # 7, ou ignorando avisos.) Não é como a segurança de tipo do restante da plataforma.
Jon Skeet
29

Você descobriu evidências de que o algoritmo de fluxo de programa que produz esse aviso é relativamente pouco sofisticado quando se trata de rastrear os significados codificados nas variáveis ​​locais.

Não tenho conhecimento específico da implementação do verificador de fluxo, mas, tendo trabalhado em implementações de código semelhante no passado, posso fazer algumas suposições. O verificador de fluxo provavelmente deduz duas coisas no caso de falso positivo: (1) _testpoderia ser nulo, porque, se não pudesse, você não teria a comparação em primeiro lugar e (2) isNullpoderia ser verdadeiro ou falso - porque se não pudesse, você não o teria em um if. Mas a conexão que o return _test;único executa se _testnão for nula, essa conexão não está sendo feita.

Esse é um problema surpreendentemente complicado, e você deve esperar que o compilador leve um tempo para obter a sofisticação das ferramentas que tiveram vários anos de trabalho por especialistas. O verificador de fluxo Coverity, por exemplo, não teria nenhum problema em deduzir que nenhuma das duas variações teve um retorno nulo, mas o verificador de fluxo Coverity custa muito dinheiro para clientes corporativos.

Além disso, os verificadores de cobertura são projetados para rodar em grandes bases de código durante a noite ; a análise do compilador C # deve ser executada entre as teclas digitadas no editor , o que altera significativamente os tipos de análises detalhadas que você pode executar razoavelmente.

Eric Lippert
fonte
"Não sofisticado" está certo - considero perdoável se tropeçar em coisas como condicionais, pois todos sabemos que o problema da interrupção é um pouco difícil em tais assuntos, mas o fato de haver uma diferença entre bool b = x != nullvs bool b = x is { }(com nenhuma atribuição realmente usada!) mostra que mesmo os padrões reconhecidos para verificações nulas são questionáveis. Para não menosprezar o trabalho indiscutivelmente difícil da equipe de fazer com que esse trabalho funcione principalmente como deveria para bases de códigos reais e em uso - parece que a análise é pragmática com capital P.
Jeroen Mostert
@JeroenMostert: Jared Par menciona em um comentário a resposta de Jon Skeet de que a Microsoft está discutindo essa questão internamente.
Brian
8

Todas as outras respostas estão praticamente exatamente corretas.

Caso alguém esteja curioso, tentei explicar a lógica do compilador da maneira mais explícita possível em https://github.com/dotnet/roslyn/issues/36927#issuecomment-508595947

A única parte que não é mencionada é como decidimos se uma verificação nula deve ser considerada "pura", no sentido de que, se você fizer isso, devemos considerar seriamente se nula é uma possibilidade. Há muitas verificações nulas "incidentais" no C #, nas quais você testa a nulidade como parte de outra ação, por isso decidimos que queríamos restringir o conjunto de verificações àquelas que tínhamos certeza de que as pessoas estavam fazendo deliberadamente. A heurística que surgiu foi "contém a palavra nula", então é por isso x != nulle x is objectproduzir resultados diferentes.

Andy Gocke
fonte