De vez em quando, voltamos aos projetos que verificamos anteriormente usando o PVS-Studio, o que resulta em suas descrições em vários artigos. Duas razões tornam essas respostas emocionantes para nós. Em primeiro lugar, a oportunidade de avaliar o progresso do nosso analisador. Em segundo lugar, monitorando o feedback dos autores do projeto em relação ao nosso artigo e o relatório de erros, que geralmente fornecemos a eles. Obviamente, os erros podem ser corrigidos sem a nossa participação. No entanto, é sempre bom quando nossos esforços ajudam a melhorar um projeto. Roslyn não foi exceção. O artigo anterior sobre essa verificação do projeto data de 23 de dezembro de 2015. Ainda há muito tempo, considerando o progresso que nosso analisador fez desde então. Como o núcleo C # do analisador PVS-Studio é baseado em Roslyn, isso nos dá interesse adicional nesse projeto. Como resultado, estamos muito interessados na qualidade do código deste projeto. Agora vamos testá-lo mais uma vez e descobrir alguns problemas novos e interessantes (mas espero que nada significativo) que o PVS-Studio possa encontrar.
Muitos de nossos leitores provavelmente conhecem bem o Roslyn (ou .NET Compiler Platform). Em resumo, é um conjunto de compiladores de código aberto e uma API para análise de código das linguagens C # e Visual Basic .NET da Microsoft. O código fonte do projeto está disponível no
GitHub .
Não darei uma descrição detalhada dessa plataforma e vou recomendar o artigo de meu colega Sergey Vasiliev "
Introdução ao Roslyn e seu uso no desenvolvimento de programas " a todos os leitores interessados. Neste artigo, você pode descobrir não apenas sobre os recursos da arquitetura do Roslyn, mas como exatamente usamos essa plataforma.
Como mencionei anteriormente, faz mais de três anos que meu colega Andrey Karpov escreveu o último artigo sobre a verificação de Roslyn "
Ano Novo PVS-Studio 6.00 Release: Scanning Roslyn ". Desde então, o analisador C # PVS-Studio recebeu muitos novos recursos. Na verdade, o artigo de Andrey era um caso de teste, pois naquele momento o analisador C # era adicionado ao PVS-Studio. Apesar disso, conseguimos detectar erros no projeto Roslyn, que certamente era de alta qualidade. Então, o que mudou no analisador para código C # neste momento que nos permitirá realizar uma análise mais aprofundada?
Desde então, o núcleo e a infraestrutura vêm se desenvolvendo. Adicionamos suporte ao Visual Studio 2017 e Roslyn 2.0 e uma profunda integração com o MSBuild. O artigo do meu colega Paul Eremeev "
Suporte do Visual Studio 2017 e Roslyn 2.0 no PVS-Studio: às vezes não é tão fácil usar soluções prontas quanto parece " descreve nossa abordagem para a integração com o MSBuild e os motivos dessa decisão.
No momento, estamos trabalhando ativamente para mudar para o Roslyn 3.0 da mesma maneira que inicialmente apoiamos o Visual Studio 2017. Ele requer o uso de nosso próprio conjunto de ferramentas, incluído no distribuidor PVS-Studio como um "stub", que é um MSBuild vazio arquivo .exe. Apesar de parecer uma "muleta" (a API do MSBuild não é muito amigável para reutilizar em projetos de terceiros devido à baixa portabilidade das bibliotecas), essa abordagem já nos ajudou a superar de maneira relativamente uniforme várias atualizações do Roslyn em termos de Visual Studio 2017. Até agora, estava ajudando (mesmo com alguns desafios) a passar pela atualização do Visual Studio 2019 e manter a compatibilidade e desempenho com versões anteriores para sistemas com versões mais antigas do MSBuild.
O núcleo do analisador também passou por várias melhorias. Uma das principais características é uma análise interprocedural completa, considerando os valores dos métodos de entrada e saída, avaliando (dependendo desses parâmetros) a acessibilidade dos ramos de execução e pontos de retorno.
Estamos no caminho de concluir a tarefa de monitorar parâmetros dentro dos métodos (por exemplo, desreferências potencialmente perigosas), além de salvar suas anotações automáticas. Para um diagnóstico que utiliza o mecanismo de fluxo de dados, isso permitirá levar em consideração situações perigosas que ocorrem ao passar um parâmetro em um método. Antes disso, ao analisar lugares tão perigosos, um aviso não era gerado, pois não podíamos saber sobre todos os possíveis valores de entrada nesse método. Agora podemos detectar o perigo, pois em todos os locais de chamada desse método, esses parâmetros de entrada serão levados em consideração.
Nota: você pode ler sobre mecanismos básicos do analisador, como fluxo de dados e outros no artigo "
Tecnologias usadas no analisador de código PVS-Studio para encontrar erros e possíveis vulnerabilidades ".
A análise interprocedural no PVS-Studio C # não é limitada nem pelos parâmetros de entrada nem pela profundidade. A única limitação são os métodos virtuais nas classes, abertos à herança e também à recursão (a análise para quando se depara com uma chamada repetida do método já avaliado). Ao fazer isso, o próprio método recursivo será avaliado eventualmente, assumindo que o valor de retorno de sua recursão seja desconhecido.
Outro grande recurso novo no analisador C # tornou-se levar em conta a possível desreferência de um ponteiro potencialmente nulo. Antes disso, o analisador reclamou de uma possível exceção de referência nula, garantindo que em todas as ramificações de execução o valor da variável seja nulo. Obviamente, estava errado em alguns casos, por isso o diagnóstico do
V3080 havia anteriormente chamado referência nula em potencial.
Agora, o analisador está ciente do fato de que a variável pode ser nula em uma das ramificações de execução (por exemplo, sob uma condição
if ). Se perceber o acesso a essa variável sem uma verificação, emitirá o aviso do V3080, mas com um nível mais baixo de certeza, do que se vê nulo em todas as ramificações. Juntamente com a análise interprocedural aprimorada, esse mecanismo permite encontrar erros que são muito difíceis de detectar. Aqui está um exemplo - imagine uma longa cadeia de chamadas de método, a última das quais não lhe é familiar. Sob certas circunstâncias, ele retorna nulo no bloco
catch , mas você não se protegeu disso, pois simplesmente não sabia. Nesse caso, o analisador apenas reclama quando vê exatamente a atribuição nula. Em nossa visão, ele distingue qualitativamente nossa abordagem de tal recurso do C # 8.0 como referência de tipo anulável, que, de fato, se limita a definir verificações para nulo para cada método. No entanto, sugerimos a alternativa - executar verificações apenas em locais onde o nulo pode realmente ocorrer, e nosso analisador agora pode procurar esses casos.
Portanto, não vamos atrasar o ponto principal por muito tempo e passar à culpa - analisando os resultados da verificação de Roslyn. Primeiro, vamos considerar os erros encontrados devido aos recursos descritos acima. Em suma, havia muitos avisos para o código de Roslyn dessa vez. Acho que está relacionado ao fato de a plataforma estar evoluindo muito ativamente (neste momento, a base de código é de cerca de 2.770.000 linhas, exceto as vazias), e não analisamos esse projeto por muito tempo. No entanto, não existem muitos erros críticos, embora sejam de maior interesse para o artigo. Como de costume, excluí testes da verificação, existem muitos deles em Roslyn.
Iniciarei com os erros V3080 do nível Médio de certeza, nos quais o analisador detectou um possível acesso por referência nula, mas não em todos os casos possíveis (ramificações de código).
Possível desreferência nula - MédiaV3080 Possível desreferência nula. Considere inspecionar 'atual'. CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70
private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....))
Vamos considerar o método
GetNode . O analisador sugere que o acesso por referência nula seja possível na condição do bloco
while . A variável recebe um valor no corpo do bloco
while , que é o resultado do método
AsNode . Em alguns casos, esse valor será
nulo . Um bom exemplo de análise interprocedural em ação.
Agora vamos considerar um caso semelhante, no qual a análise interprocedural foi realizada por meio de duas chamadas de método.
V3080 Possível desreferência nula. Considere inspecionar 'diretório'. CommonCommandLineParser.cs 911
private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ?
A variável de
diretório no corpo do método
ExpandFileNamePattern obtém o valor do método
GetDirectoryName (string) . Isso, por sua vez, retorna o resultado do método sobrecarregado
GetDirectoryName (string, bool) cujo valor pode ser
nulo . Como o
diretório de variáveis é usado sem uma verificação preliminar de null no corpo do método
ExpandFileNamePattern - podemos proclamar o analisador correto sobre a emissão do aviso. Esta é uma construção potencialmente insegura.
Outro fragmento de código com o erro V3080, mais precisamente, com dois erros, emitido para uma única linha de código. A análise interprocedural não foi necessária aqui.
V3080 Possível desreferência nula. Considere inspecionar 'spanStartLocation'. TestWorkspace.cs 574
V3080 Possível desreferência nula. Considere inspecionar 'spanEndLocationExclusive'. TestWorkspace.cs 574
private void MapMarkupSpans(....) { .... foreach (....) { .... foreach (....) { .... int? spanStartLocation = null; int? spanEndLocationExclusive = null; foreach (....) { if (....) { if (spanStartLocation == null && positionInMarkup <= markupSpanStart && ....) { .... spanStartLocation = ....; } if (spanEndLocationExclusive == null && positionInMarkup <= markupSpanEndExclusive && ....) { .... spanEndLocationExclusive = ....; break; } .... } .... } tempMappedMarkupSpans[key]. Add(new TextSpan( spanStartLocation.Value,
As variáveis
spanStartLocation e
spanEndLocationExclusive são do tipo
int anulável e são inicializadas por
null . Mais adiante, no código, eles podem receber valores, mas somente sob certas condições. Em alguns casos, seu valor permanece
nulo . Depois disso, essas variáveis são acessadas por referência sem verificação preliminar de nulo, o que o analisador indica.
O código Roslyn contém muitos desses erros, mais de 100. Geralmente, o padrão desses erros é o mesmo. Existe algum tipo de método geral, que potencialmente retorna
nulo . O resultado desse método é usado em muitos lugares, às vezes através de dezenas de chamadas de método intermediárias ou verificações adicionais. É importante entender que esses erros não são fatais, mas podem levar ao acesso por referência nula. Embora a detecção de tais erros seja bastante desafiadora. Por isso, em alguns casos, deve-se considerar a refatoração de código; nesse caso, se retornar
nulo , o método lançará uma exceção. Caso contrário, você poderá proteger seu código apenas com verificações gerais, o que é bastante cansativo e, às vezes, pouco confiável. De qualquer forma, é claro que cada caso específico requer uma solução com base nas especificações do projeto.
Nota Acontece que, em um determinado momento, não existem casos (dados de entrada), quando o método retorna
nulo e não há erro real. No entanto, esse código ainda não é confiável, porque tudo pode mudar ao introduzir algumas alterações no código.
Para descartar o assunto do
V3080 , vejamos erros óbvios do nível de alta segurança, quando o acesso por referência nula é o mais provável ou até inevitável.
Possível desreferência nula - AltaV3080 Possível desreferência nula. Considere inspecionar 'collectionType.Type'. AbstractConvertForToForEachCodeRefactoringProvider.cs 137
public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { .... var collectionType = semanticModel.GetTypeInfo(....); if (collectionType.Type == null && collectionType.Type.TypeKind == TypeKind.Error) { return; } .... }
Devido ao erro de digitação na condição (
&& é usado em vez do operador
|| ), o código funciona de maneira diferente da pretendida e o acesso à variável
collectionType.Type será executado quando for
nulo . A condição deve ser corrigida da seguinte maneira:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
A propósito, as coisas podem se desdobrar de outra maneira: na primeira parte da condição, os operadores
== e
! = Estão desarrumados
. Então o código correto ficaria assim:
if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....
Esta versão do código é menos lógica, mas também corrige o erro. A solução final cabe aos autores do projeto decidirem.
Outro erro semelhante.
V3080 Possível desreferência nula. Considere inspecionar 'ação'. TextViewWindow_InProc.cs 372
private Func<IWpfTextView, Task> GetLightBulbApplicationAction(....) { .... if (action == null) { throw new InvalidOperationException( $"Unable to find FixAll in {fixAllScope.ToString()} code fix for suggested action '{action.DisplayText}'."); } .... }
O erro é cometido ao gerar a mensagem para a exceção. É seguida pela tentativa de acessar a propriedade
action.DisplayText por meio da variável de
ação , que é conhecida por ser
nula .
Aí vem o último erro V3080 do nível Alto.
V3080 Possível desreferência nula. Considere inspecionar 'tipo'. ObjectFormatterHelpers.cs 91
private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }
O método é bastante pequeno, por isso cito-o inteiramente. A condição no bloco de
devolução está incorreta. Em alguns casos, ao acessar
type.FullName , uma exceção pode ocorrer. Vou usar parênteses para deixar claro (eles não vão mudar o comportamento):
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
De acordo com a precedência das operações, o código funcionará exatamente assim. Caso a variável
type seja
nula , cairemos na outra opção, onde usaremos a referência null do
tipo , verificando a variável
targetTypeName como
null . O código pode ser corrigido, por exemplo, da seguinte maneira:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Eu acho que é o suficiente para revisar os erros do V3080. Agora é hora de ver outras coisas interessantes que o analisador PVS-Studio conseguiu encontrar.
TypoV3005 A variável 'SourceCodeKind' é atribuída a si mesma. DynamicFileInfo.cs 17
internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind;
Devido à falha na nomeação de variáveis, um erro de digitação foi feito no construtor da classe
DynamicFileInfo . O campo
SourceCodeKind recebe seu próprio valor em vez de usar o parâmetro
sourceCodeKind . Para minimizar a probabilidade de tais erros, recomendamos que você use o prefixo de sublinhado nos nomes dos parâmetros nesses casos. Aqui está um exemplo de uma versão corrigida do código:
public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }
InadvertênciaV3006 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw new InvalidOperationException (FOO). ProjectBuildManager.cs 61
~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
Sob uma determinada condição, o destruidor deve lançar uma exceção, mas isso não está acontecendo enquanto o objeto de exceção é simplesmente criado. A palavra-chave
throw foi perdida. Aqui está a versão corrigida do código:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
O problema com destruidores em C # e lançando exceções a eles é um tópico para outra discussão, que está além do escopo deste artigo.
Quando o resultado não é importanteOs métodos, que receberam o mesmo valor em todos os casos, acionaram um certo número de avisos do
V3009 . Em alguns casos, isso pode não ser crítico ou o valor de retorno simplesmente não é verificado no código de chamada. Eu pulei esses avisos. Mas alguns trechos de código pareciam suspeitos. Aqui está um deles:
V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. GoToDefinitionCommandHandler.cs 62
internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }
O método
TryExecuteCommand retorna apenas
true . Ao fazer isso, no código de chamada, o valor retornado está envolvido em algumas verificações.
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
É difícil dizer exatamente até que ponto esse comportamento é perigoso. Mas se o resultado não for necessário, talvez o tipo do valor de retorno deva ser alterado para nulo e a pessoa faça pequenas edições no método de chamada. Isso tornará o código mais legível e seguro.
Avisos semelhantes do analisador:
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. CommentUncommentSelectionCommandHandler.cs 86
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. RenomearTrackingTaggerProvider.RenameTrackingCommitter.cs 99
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. JsonRpcClient.cs 138
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. AbstractFormatEngine.OperationApplier.cs 164
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'false'. TriviaDataFactory.CodeShapeAnalyzer.cs 254
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. ObjectList.cs 173
- V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. ObjectList.cs 249
Verificou a coisa erradaV3019 Possivelmente, uma variável incorreta é comparada com nulo após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis 'valor', 'valueToSerialize'. RoamingVisualStudioProfileOptionPersister.cs 277
public bool TryPersist(OptionKey optionKey, object value) { .... var valueToSerialize = value as NamingStylePreferences; if (value != null) { value = valueToSerialize.CreateXElement().ToString(); } .... }
A variável
value é convertida para o tipo
NamingStylePreferences . O problema está na verificação que se segue. Mesmo que a variável
value não seja nula, não garante que a conversão do tipo tenha sido bem-sucedida e
valueToSerialize não contenha
nulo . Possível lançamento da exceção
NullReferenceException . O código precisa ser corrigido da seguinte maneira:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Outro bug semelhante:
V3019 Possivelmente, uma variável incorreta é comparada com nulo após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181
private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name ==
A variável
columnState é convertida no tipo
ColumnState2 . No entanto, o resultado da operação, que é a variável
columnState2, não recebe mais
nulos . Em vez disso, a variável
columnState é verificada usando o operador
nulo condicional. Por que esse código é perigoso? Assim como no exemplo anterior, a conversão com o operador as pode falhar e a variável será
nula , resultando em uma exceção. A propósito, um erro de digitação pode ser o culpado aqui. Dê uma olhada na condição no bloco
if .
Talvez, em vez de
columnState? .Name, o autor queira escrever
columnState2? .Name . É muito provável, considerando nomes de variáveis bastante defeituosos
columnState e
columnState2.Verificações redundantesUm grande número de avisos (mais de 100) foi emitido em construções não críticas, mas potencialmente inseguras relacionadas a verificações redundantes. Por exemplo, este é um deles.
A expressão
V3022 'navInfo == null' sempre é falsa. AbstractSyncClassViewCommandHandler.cs 101
public bool ExecuteCommand(....) { .... IVsNavInfo navInfo = null; if (symbol != null) { navInfo = libraryService.NavInfoFactory.CreateForSymbol(....); } if (navInfo == null) { navInfo = libraryService.NavInfoFactory.CreateForProject(....); } if (navInfo == null)
Pode ser que não haja nenhum bug real aqui. É apenas uma boa razão para demonstrar "análise interprocedural + análise de fluxo de dados" trabalhando em um reboque. O analisador sugere que a segunda verificação
navInfo == null seja redundante. De fato, antes dele, o valor atribuído a
navInfo será obtido no método
libraryService.NavInfoFactory.CreateForProject , que construirá e retornará um novo objeto da classe
NavInfo . De maneira alguma ele retornará
nulo . Aqui surge a pergunta: por que o analisador não emitiu um aviso para a primeira verificação
navInfo == null ? Existem algumas razões. Primeiramente, se a variável do
símbolo for
nula , o valor
navInfo também permanecerá uma referência nula. Em segundo lugar, mesmo se
navInfo obtiver o valor do método
ibraryService.NavInfoFactory.CreateForSymbol , esse valor também poderá ser
nulo . Portanto, a primeira verificação
navInfo == null é realmente necessária.
Verificações insuficientesAgora, a situação inversa do discutido acima. Vários avisos da
V3042 foram acionados para o código, no qual o acesso por referência nula é possível. Mesmo uma ou duas pequenas verificações poderiam ter consertado tudo.
Vamos considerar outro fragmento de código interessante, que possui dois desses erros.
V3042 Possível NullReferenceException. O '?' e '.' operadores são usados para acessar membros do objeto 'receptor' Binder_Expressions.cs 7770
V3042 Possível NullReferenceException. O '?' e '.' operadores são usados para acessar membros do objeto 'receptor' Binder_Expressions.cs 7776
private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax !=
A variável do
receptor pode ser nula. O autor do código sabe disso, pois ele usa o operador
nulo condicional na condição do bloco
if para acessar o
receptor ? .
Sintaxe . Além disso, a variável do
receptor é usada sem nenhuma verificação para acessar o
receiver.Type ,
receiver.Syntax e
receiver.HasErrors . Esses erros devem ser corrigidos:
private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver?.Type; if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver?.Syntax, 0, receiverType ?? CreateErrorType(), hasErrors: receiver?.HasErrors) { WasCompilerGenerated = true }; return receiver; }
Também precisamos ter certeza de que o construtor suporta a obtenção de valores
nulos para seus parâmetros ou precisamos executar refatoração adicional.
Outros erros semelhantes:
- V3042 Possível NullReferenceException. O '?' e '.' operadores são usados para acessar membros do objeto 'containsType' SyntaxGeneratorExtensions_Negate.cs 240
- V3042 Possível NullReferenceException. O '?' e '.' operadores são usados para acessar membros do objeto 'expressão' ExpressionSyntaxExtensions.cs 349
- V3042 Possível NullReferenceException. O '?' e '.' operadores são usados para acessar membros do objeto 'expressão' ExpressionSyntaxExtensions.cs 349
Erro na condiçãoV3057 A função '
Substring ' pode receber o valor '-1' enquanto se espera um valor não negativo. Inspecione o segundo argumento. CommonCommandLineParser.cs 109
internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }
Caso a variável de
dois pontos seja 0, o que é bom de acordo com a condição no código, o método
Substring emitirá uma exceção. Isso precisa ser corrigido:
if (colon > 0)
Possível erro de digitaçãoV3065 O parâmetro 't2' não é utilizado dentro do corpo do método. CSharpCodeGenerationHelpers.cs 84
private static TypeDeclarationSyntax ReplaceUnterminatedConstructs(....) { .... var updatedToken = lastToken.ReplaceTrivia(lastToken.TrailingTrivia, (t1, t2) => { if (t1.Kind() == SyntaxKind.MultiLineCommentTrivia) { var text = t1.ToString(); .... } else if (t1.Kind() == SyntaxKind.SkippedTokensTrivia) { return ReplaceUnterminatedConstructs(t1); } return t1; }); .... }
A expressão lambda aceita dois parâmetros: t1 e t2. No entanto, apenas t1 é usado. Parece suspeito, levando em consideração o quão fácil é cometer um erro ao usar variáveis com esses nomes.
InadvertênciaV3083 Chamada não segura do evento 'TagsChanged', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. PreviewUpdater.Tagger.cs 37
public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span));
O evento
TagsChanged é invocado de maneira insegura. Entre a verificação de
nulo e a invocação do evento, alguém pode cancelar a inscrição, e uma exceção será lançada. Além disso, outras operações são executadas no corpo do bloco
if antes de chamar o evento. Chamei esse erro de "inadvertência", porque esse evento é tratado com mais cuidado em outros lugares, da seguinte maneira:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
O uso de uma variável de
manipulador adicional evita o problema. No método
OnTextBufferChanged, é necessário fazer edições para lidar com segurança com o evento.
Interseção de intervalosV3092 As interseções de faixa são possíveis dentro de expressões condicionais. Exemplo: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. ILBuilderEmit.cs 677
internal void EmitLongConstant(long value) { if (value >= int.MinValue && value <= int.MaxValue) { .... } else if (value >= uint.MinValue && value <= uint.MaxValue) { .... } else { .... } }
Para uma melhor compreensão, deixe-me reescrever esse código, alterando os nomes das constantes com seus valores reais:
internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }
Provavelmente, não há erro real, mas a condição parece estranha. Sua segunda parte (
senão se ) será executada apenas para o intervalo de 2147483648 + 1 a 4294967295.
Outro par de avisos semelhantes:
- V3092 As interseções de faixa são possíveis dentro de expressões condicionais. Exemplo: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 109
- V3092 As interseções de faixa são possíveis dentro de expressões condicionais. Exemplo: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 66
Mais sobre verificações de nulo (ou falta delas)Alguns erros do
V3095 na verificação de uma variável para nulo logo após seu uso. O primeiro é ambíguo, vamos considerar o código.
V3095 O objeto 'displayName' foi usado antes de ser verificado contra nulo. Verifique as linhas: 498, 503. FusionAssemblyIdentity.cs 498
internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }
Supõe-se que a referência
displayName possa ser nula. Para isso, a verificação
Debug.Assert foi realizada. Não está claro por que isso ocorre após o uso de uma string. Também deve ser levado em consideração que, para configurações diferentes de Debug, o compilador removerá o
Debug.Assert . Isso significa que obter uma referência nula é possível apenas para Depuração? Caso contrário, por que o autor fez a verificação de
string.IsNullOrEmpty (string) , por exemplo. É a pergunta para os autores do código.
O seguinte erro é mais evidente.
V3095 O objeto 'scriptArgsOpt' foi usado antes de ser verificado em relação a nulo. Verifique as linhas: 321, 325. CommonCommandLineParser.cs 321
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg);
Eu acho que esse código não precisa de explicações. Deixe-me dar a versão fixa:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
No código Roslyn, havia mais 15 erros semelhantes:
- V3095 O objeto 'LocalFunctions' foi usado antes de ser verificado contra nulo. Verifique as linhas: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289
- V3095 O objeto 'resolution.OverloadResolutionResult' foi usado antes de ser verificado como nulo. Verifique as linhas: 579, 588. Binder_Invocation.cs 579
- V3095 O objeto 'resolution.MethodGroup' foi usado antes de ser verificado contra nulo. Verifique as linhas: 592, 621. Binder_Invocation.cs 592
- V3095 O objeto 'touchedFilesLogger' foi usado antes de ser verificado como nulo. Verifique as linhas: 111, 126. CSharpCompiler.cs 111
- V3095 O objeto 'newExceptionRegionsOpt' foi usado antes de ser verificado como nulo. Verifique as linhas: 736, 743. AbstractEditAndContinueAnalyzer.cs 736
- V3095 O objeto 'symbol' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 422, 427. AbstractGenerateConstructorService.Editor.cs 422
- V3095 O objeto '_state.BaseTypeOrInterfaceOpt' foi usado antes de ser verificado como nulo. Verifique as linhas: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132
- V3095 O objeto 'elemento' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 232, 233. ProjectUtil.cs 232
- V3095 O objeto 'languages' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 22, 28. ExportCodeCleanupProvider.cs 22
- V3095 O objeto 'memberType' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
- V3095 O objeto 'validTypeDeclarations' foi usado antes de ser verificado em relação a nulo. Verifique as linhas: 223, 228. SyntaxTreeExtensions.cs 223
- V3095 O objeto 'texto' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 376, 385. MSBuildWorkspace.cs 376
- V3095 O objeto 'nameOrMemberAccessExpression' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 206, 223. CSharpGenerateTypeService.cs 206
- V3095 O objeto 'simpleName' foi usado antes de ser verificado com relação a nulo. Verifique as linhas: 83, 85. CSharpGenerateMethodService.cs 83
- V3095 O objeto 'opção' foi usado antes de ser verificado em relação a nulo. Verifique as linhas: 23, 28. OptionKey.cs 23
Vamos considerar os erros da
V3105 . Aqui, o operador
nulo condicional é usado ao inicializar a variável, mas ainda mais a variável é usada sem verificações de
nulo .
Dois avisos indicam o seguinte erro:
V3105 A variável 'documentId' foi usada após ser atribuída por meio do operador condicional nulo. NullReferenceException é possível. CodeLensReferencesService.cs 138
V3105 A variável 'documentId' foi usada após ser atribuída por meio do operador condicional nulo. NullReferenceException é possível. CodeLensReferencesService.cs 139
private static async Task<ReferenceLocationDescriptor> GetDescriptorOfEnclosingSymbolAsync(....) { .... var documentId = solution.GetDocument(location.SourceTree)?.Id; return new ReferenceLocationDescriptor( .... documentId.ProjectId.Id, documentId.Id, ....); }
A variável
documentId pode ser inicializada por
nulo . Como resultado, a criação de um objeto
ReferenceLocationDescriptor resultará no lançamento de uma exceção. O código deve ser corrigido:
return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
Os desenvolvedores também devem cobrir a possibilidade de variáveis, passadas para um construtor, serem
nulas.Outros erros semelhantes no código:
- V3105 A variável 'symbol' foi usada após ser atribuída através do operador condicional nulo. NullReferenceException é possível. SymbolFinder_Hierarchy.cs 44
- V3105 A variável 'symbol' foi usada após ser atribuída através do operador condicional nulo. NullReferenceException é possível. SymbolFinder_Hierarchy.cs 51
Prioridades e parêntesesV3123 Talvez o operador '?:'
Funcione de maneira diferente do esperado. Sua prioridade é menor que a prioridade de outros operadores em sua condição. Edit.cs 70
public bool Equals(Edit<TNode> other) { return _kind == other._kind && (_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode) && (_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode); }
A condição no bloco de retorno não é avaliada como o desenvolvedor pretendia. Assumiu-se que a primeira condição será
_tipo == outro._kin d, (é por isso que, após essa condição, há uma quebra de linha) e, depois disso, os blocos de condições com o operador "
? " Serão avaliados em sequência. De fato, a primeira condição é
_kind == other._kind && (_oldNode == null) . Isso se deve ao fato de o operador
&& ter uma prioridade mais alta que o operador "
? ". Para corrigir isso, um desenvolvedor deve usar todas as expressões do operador "
? " Entre parênteses:
return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Isso conclui minha descrição dos erros encontrados.
ConclusãoApesar do grande número de erros que eu consegui encontrar, em termos do tamanho do código do projeto Roslyn (2.770.000 linhas), não é demais. Como Andrey escreveu em um artigo anterior, também estou pronto para reconhecer a alta qualidade deste projeto.
Gostaria de observar que essas verificações ocasionais de código não têm nada a ver com a metodologia da análise estática e são quase inúteis. A análise estática deve ser aplicada regularmente, e não caso a caso. Dessa forma, muitos erros serão corrigidos nos estágios iniciais e, portanto, o custo para corrigi-los será dez vezes menor. Essa idéia é apresentada com mais detalhes nesta pequena
nota , por favor, confira.
Você pode verificar alguns erros neste projeto e em outro. Para fazer isso, basta fazer o
download e experimentar o nosso analisador.