Finalmente, um 2020 tão difícil está chegando ao fim, o que significa que é hora de fazer um balanço! Durante este ano, a equipe PVS-Studio escreveu muitos artigos que lidavam com vários erros encontrados pelo analisador em projetos de código aberto. Você pode ver o mais interessante deles aqui, nos erros TOP encontrados em projetos C # em 2020. Boa visualização!
Como o topo foi formado
Esta lista contém os gatilhos mais interessantes, na minha opinião, sobre os quais meus colegas e eu escrevemos em artigos de 2020. Um critério de seleção importante foi o grau de confiança de que um erro realmente havia sido cometido no fragmento de código relevante. E, claro, ao selecionar, além de colocar os locais, o 'interesse' do acionamento foi levado em consideração, mas esta é minha opinião subjetiva - você sempre pode contestá-la nos comentários.
Tentei tornar o topo o mais diversificado possível: tanto em termos de mensagens PVS-Studio, quanto em termos de projetos para os quais avisos de código foram emitidos. A lista inclui detecções nas fontes de 8 projetos verificados. Ao mesmo tempo, as regras de diagnóstico quase nunca são repetidas - você pode se encontrar duas vezes aqui apenas V3022 e V3106 (e não, eles não foram feitos por mim, mas aparentemente são meus favoritos). Assim, a variedade é garantida aqui :).
Bem, vamos começar! Top 10!
10º lugar - nova licença antiga
Nossa principal resposta começa com um artigo de uma pessoa muito competente sobre a verificação de projetos C # para Linux e macOS, onde o projeto RavenDB foi usado como exemplo:
private static void UpdateEnvironmentVariableLicenseString(....)
{
....
if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
return;
....
}
Aviso do analisador : V3066 Possível ordem incorreta de argumentos transmitidos ao método 'ValidateLicense': 'newLicense' e 'oldLicense'. LicenseHelper.cs (177) Raven.Server
Parece, o que há de errado aqui? O código compila muito bem. Por que o analisador decidiu que é necessário transferir primeiro a oldLicense e depois a newLicense ? Você adivinhou, certo? Vamos dar uma olhada na declaração ValidateLicense :
private static bool ValidateLicense(License oldLicense,
RSAParameters rsaParameters,
License newLicense)
Uau, é verdade - primeiro, o antigo está nos parâmetros, e só então - a nova licença. Diga-me, essa sua análise dinâmica pode captar isso? :)
Em qualquer caso, o acionamento é interessante. Talvez a ordem não seja muito importante aí, mas é melhor verificar novamente esses fragmentos, concorda?
9º lugar - 'FirstOrDefault' e inesperado 'nulo'
Em 9º lugar ficou a resposta do artigo " Brinque no" osu! ", Não se esqueça dos erros , escrito no início deste ano:
public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
var ruleset = rulesets.GetRuleset(OnlineRulesetID);
var mods = Mods != null ? ruleset.CreateInstance()
.GetAllMods().Where(....)
.ToArray() : Array.Empty<Mod>();
....
}
Viu o erro? E ela é! O que o analisador diz?
Aviso do analisador : V3146 [CWE-476] Possível desreferência nula do 'conjunto de regras'. O 'FirstOrDefault' pode retornar um valor nulo padrão. APILegacyScoreInfo.cs 24
Sim, sim, mais uma vez, não dei todas as informações necessárias de uma vez. Na verdade, você realmente não pode ver nada de suspeito neste código. Afinal, FirstOrDefault , sobre o qual o analisador nos fala, está na definição do método GetRuleset :
public RulesetInfo GetRuleset(int id) =>
AvailableRulesets.FirstOrDefault(....);
Negócio terrível! O método retornará RulesetInfo se encontrar um adequado. E se não? Retorne null com calma . E atirará já no local onde será utilizado o resultado da chamada. Nesse caso, ao chamar ruleset.CreateInstance () .
Pode surgir a pergunta: e se nunca houver nulo ? E se a coleção sempre contiver o elemento necessário? Bem, se o desenvolvedor tem certeza disso, por que não usar o First em vez de FirstOrDefault ?
8º lugar - Hello from Python
O último gatilho dos três primeiros foi emitido para o código do projeto RunUO. Um artigo sobre verificação foi escrito em fevereiro e está disponível aqui .
O fragmento encontrado é extremamente suspeito, embora seja difícil dizer com certeza se é um erro:
public override void OnCast()
{
if ( Core.AOS )
{
damage = m.Hits / 2;
if ( !m.Player )
damage = Math.Max( Math.Min( damage, 100 ), 15 );
damage += Utility.RandomMinMax( 0, 15 );
}
else { .... }
}
Aviso do analisador : V3043 A lógica operacional do código não corresponde à sua formatação. A instrução é recuada para a direita, mas é sempre executada. É possível que estejam faltando colchetes. Earthquake.cs 57
Isso mesmo - trata-se de indentação! Parece que o dano da linha + = Utility.RandomMinMax (0, 15) deveria ter sido executado apenas se m.Player for falso... Da mesma forma, o código Python funcionaria, onde o recuo não é apenas para beleza, mas também para definir a lógica do aplicativo. Mas o compilador C # tem uma opinião diferente sobre esse assunto! Eu me pergunto que opinião o desenvolvedor teve?
Em geral, nesta situação, existem 2 opções. Ou estão faltando chaves aqui e tudo funciona incorretamente, ou tudo funciona corretamente, mas com o tempo, definitivamente haverá uma pessoa que considerará isso um erro e "consertará".
Talvez eu esteja errado, e realmente há momentos em que é normal escrever algo assim. Se você estiver ciente disso, por favor, escreva um comentário - Eu realmente gostaria de saber sobre esses casos.
7º lugar - Perfeito ou Perfeito - eis a questão!
Está ficando cada vez mais difícil distribuir avisos em alguns lugares, e estamos passando para o segundo alarme nesta lista do artigo sobre osu! ...
Quanto tempo levará para você ver o erro?
protected override void CheckForResult(....)
{
....
ApplyResult(r =>
{
if ( holdNote.hasBroken
&& (result == HitResult.Perfect || result == HitResult.Perfect))
result = HitResult.Good;
....
});
}
Aviso do analisador : V3001 Existem subexpressões idênticas 'result == HitResult.Perfect' à esquerda e à direita de '||' operador. DrawableHoldNote.cs 266 Não
muito, eu acho, você só precisa ler o aviso PVS-Studio. Os desenvolvedores que usam análise estática geralmente fazem isso :). Pode-se argumentar sobre as posições anteriores no topo, mas aqui o erro é óbvio. É difícil dizer qual elemento específico da enumeração HitResult deveria ter sido usado aqui em vez do segundo Perfect (ou em vez do primeiro), mas no final algo estava claramente errado. Bem, está tudo bem - o erro foi encontrado, o que significa que pode ser facilmente corrigido.
6º lugar - nulo (não) aprovado!
O 6º lugar foi uma resposta muito legal ao código do projeto Open XML SDK. Você pode ler o artigo sobre como verificar aqui .
O desenvolvedor queria implementar uma propriedade que não retornasse nulo , mesmo se atribuída diretamente. E é muito bom quando você pode ter certeza de que null não será escrito em nenhuma circunstância. É uma pena que não seja esta a situação:
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}
Aviso do analisador : V3008 A variável '_rawOuterXml' recebe valores duas vezes sucessivamente. Talvez seja um engano. Verifique as linhas: 164, 161. OpenXmlElement.cs 164
Acontece que em _rawOuterXml será registrado o valor independente de ser nulo ou não. Se você olhar para esse código desatento, pode pensar que null nunca será gravado nessa propriedade - afinal, há uma verificação! Bem, se sim, então sob a árvore você não encontrará presentes, mas uma NullReferenceException inesperada :(
5º lugar - Filmado de uma matriz com uma matriz dentro
Os cinco principais acionamentos de 2020 foram emitidos pelo analisador do projeto TensorFlow.NET que testei pessoalmente. Ao clicar no link , você pode ler o artigo sobre como verificar este projeto (ah, e tenho visto muito todo mundo lá).
A propósito, se você gosta de ver exemplos de erros interessantes de projetos reais em C #, sugiro que se inscreva no meu twitter . Lá eu pretendo postar gatilhos curiosos e apenas fragmentos de código interessantes, muitos dos quais, infelizmente, não serão incluídos em artigos. Ficarei feliz em ver você! :)
Bem, agora vamos prosseguir para o acionamento:
public TensorShape(int[][] dims)
{
if(dims.Length == 1)
{
switch (dims[0].Length)
{
case 0: shape = new Shape(new int[0]); break;
case 1: shape = Shape.Vector((int)dims[0][0]); break;
case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
default: shape = new Shape(dims[0]); break;
}
}
else
{
throw new NotImplementedException("TensorShape int[][] dims");
}
}
Aviso do analisador : V3106 Possivelmente o índice está fora do limite. O índice '1' está apontando para além do limite de 'dimmer'. TensorShape.cs 107
Na verdade, foi muito difícil escolher onde colocar esse gatilho, pois é muito interessante, mas outros não estão muito atrás nesse quesito. Então, vamos descobrir o que está acontecendo aqui.
Se o número de matrizes em dims não for igual a 1, uma exceção do tipo NotImplementedException é lançada . E o que vai acontecer se escurecerexatamente uma matriz? O número de elementos neste 'array dentro de um array' é verificado. Observe o que acontece quando é 2. Como um dos argumentos para o construtor Shape.Matrix , inesperadamente, dims [1] [2] é passado . Agora, vamos lembrar - quantos elementos havia na matriz dims ?
Isso mesmo, exatamente um - acabamos de verificar! Uma tentativa de obter o segundo elemento de uma matriz, que contém apenas um elemento, lançará uma exceção IndexOutOfRangeException . Obviamente um erro. Mas existe uma maneira óbvia de consertar isso?
A primeira coisa que vem à mente é a mudança escurece [1] [2] para escurecer [0] [2] . O erro irá embora? Não importa como seja! Haverá a mesma exceção novamente. Mas desta vez ele estará conectado com o fato de que neste caso-branch o número de elementos em dims [0] é igual a 2. O desenvolvedor cometeu dois erros no índice em uma linha? Ou alguma outra variável deveria ser usada lá? Quem sabe ... O trabalho do analisador é encontrar um erro, e quem o cometeu ou seus colegas terão que corrigi-lo.
4º lugar - Propriedade de um objeto que não existe
Outro gatilho surgiu neste topo do artigo sobre a verificação OpenRA . Talvez mereça mais, mas por vontade do destino, este gatilho ficou em 4º lugar. Bem, isso também é muito bom! Vamos dar uma olhada em qual erro o PVS-Studio foi capaz de detectar desta vez:
public ConnectionSwitchModLogic(....)
{
....
var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
if (logo != null)
{
logo.GetSprite = () =>
{
....
};
}
if (logo != null && mod.Icon == null) // <=
{
// Hide the logo and center just the text
if (title != null)
title.Bounds.X = logo.Bounds.Left;
if (version != null)
version.Bounds.X = logo.Bounds.X;
width -= logo.Bounds.Width;
}
else
{
// Add an equal logo margin on the right of the text
width += logo.Bounds.Width; // <=
}
....
}
Aviso do analisador : V3125 O objeto 'logo' foi usado após ser verificado em relação ao nulo. Verifique as linhas: 236, 222. ConnectionLogic.cs 236 A
que você deve prestar atenção aqui? Bem, para começar, o logotipo provavelmente pode conter nulos . Isso é sugerido pelas verificações constantes e pelo nome do método GetOrNull , que retorna o valor escrito no logotipo . Nesse caso, vamos pensar no que acontece se GetOrNull realmente retornar nulo . A princípio tudo está em ordem, mas depois a condição é verificada logo! = null && mod.Icon == null . Como você pode imaginar, o resultado será uma transição para o outro -branch e ... Tentativa de acessar a propriedade Bounds da variável, que contém null , e então - bdysh! NullReferenceException está batendo na porta.
3º lugar - elemento de Schrödinger
Finalmente, chegamos aos três primeiros finalistas. Os 3 principais bugs para 2020 foram encontrados no projeto Nethermind, sobre a verificação do qual um artigo foi escrito com o título intrigante " Código em uma linha ou verificação de Nethermind usando PVS-Studio C # para Linux ". O erro é incrivelmente simples, mas invisível ao olho humano, especialmente quando você considera o tamanho do projeto. Você acha que este gatilho é digno de seu lugar?
public ReceiptsMessage Deserialize(byte[] bytes)
{
if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
return new ReceiptsMessage(null);
....
}
Aviso do analisador : V3106 Possivelmente o índice está fora do limite. O índice '0' está apontando além do limite de 'bytes'. Nethermind.Network ReceiptsMessageSerializer.cs 50
Provavelmente, ser capaz de pegar a primeira coisa em uma caixa vazia seria legal, mas aqui tal desejo só levará a uma IndexOutOfRangeException sendo lançada . Só uma bagatela - um pequeno erro no operador, e o aplicativo já está funcionando incorretamente, ou talvez até trava.
Obviamente, você deve usar '||' em vez de '&&'. Esses erros lógicos não são incomuns, especialmente em projetos complexos. Portanto, é bastante conveniente verificar esses momentos no modo automático.
2º lugar - menos de 2, mas mais de 3
Em segundo lugar, coloquei mais um gatilho no código do projeto RavenDB. Deixe-me lembrar que você pode ler sobre os resultados de sua verificação (e não apenas) no artigo correspondente .
Bem, agora conheçam - os 2 principais erros de 2020:
private OrderByField ExtractOrderByFromMethod(....)
{
....
if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
throw new InvalidQueryException(....);
....
}
Aviso do analisador : V3022 A expressão 'me.Arguments.Count <2 && me.Arguments.Count> 3' é sempre falsa. Provavelmente o '||' operador deve ser usado aqui. QueryMetadata.cs (861) Raven.Server
Anteriormente, vimos os momentos em que uma exceção inesperada foi lançada, mas agora, ao contrário, a exceção esperada nunca será lançada. Bem, ou ainda será descartado se alguém aparecer com um número menor que 2, mas maior que 3.
Eu não ficaria surpreso se você discordasse, mas eu realmente gosto desse gatilho mais do que todos os anteriores. Sim, o erro é incrivelmente simples e para corrigi-lo basta trocar a operadora. A propósito, isso também é sugerido pela mensagem passada ao construtor InvalidQueryException : chamada "ORDER BY 'spatial.distance (de, para, roundFactor)' inválida, esperados 2-3 argumentos, obteve" + me.Arguments.Count .
Eu concordo, este é um descuido elementar, mas ninguém percebeu ou corrigiu, pelo menos até que foi encontrado usando o PVS-Studio. Isso me lembra que os programadores, não importa o quão experientes sejam, ainda são (infelizmente?) Humanos. E as pessoas, independentemente das qualificações, podem perder até esses erros estúpidos por uma variedade de razões. Às vezes, o erro dispara imediatamente e às vezes você pode se perguntar por muito tempo por que o usuário não vê a mensagem sobre a chamada ORDER BY incorreta.
1º lugar - Cotações para + 100% de segurança do código
Viva, viva, viva! Finalmente chegamos ao gatilho, que achei o mais interessante, engraçado, legal e assim por diante. Foi emitido para o código do projeto ONLYOFFICE, cuja análise está associada a um dos artigos mais recentes deste ano - " ONLYOFFICE Community Server: Como Bugs Contribuem para Problemas de Segurança ".
Bem, eu apresento a vocês a história mais triste sobre ArgumentException , que nunca será lançada:
public void SetCredentials(string userName, string password, string domain)
{
if (string.IsNullOrEmpty(userName))
{
throw new ArgumentException("Empty user name.", "userName");
}
if (string.IsNullOrEmpty("password"))
{
throw new ArgumentException("Empty password.", "password");
}
CredentialsUserName = userName;
CredentialsUserPassword = password;
CredentialsDomain = domain;
}
Aviso do analisador : a expressão V3022 'string.IsNullOrEmpty ("senha")' é sempre falsa. SmtpSettings.cs 104
Foi muito difícil escolher qual erro colocar em qual lugar, mas esse gatilho para mim desde o início foi o líder entre todos. O erro de digitação menor mais simples - e o código já funciona incorretamente. Nem o destaque no IDE, nem a revisão, nem o bom e velho bom senso ajudaram. É uma função pequena, simples e bem escrita. E mesmo aqui o PVS-Studio foi capaz de encontrar o que os profissionais faziam falta.
Como sempre, o diabo está nos detalhes. Não seria ótimo se todos esses detalhes fossem pesquisados automaticamente? Claro que sim! E deixe o desenvolvedor fazer o que um analisador estático não pode fazer - ele cria novos aplicativos bonitos e seguros. Cria sem pensar se ele colocou aspas extras ao verificar uma variável ou não.
Conclusão
Encontrar 10 erros interessantes em artigos de 2020 foi fácil. Mas distribuí-los em lugares acabou sendo outra tarefa. Por um lado, alguns gatilhos refletem melhor o trabalho de mecanismos de análise avançados. Por outro lado, alguns dos erros parecem engraçados até certo ponto. Muitas das posições apresentadas podem ser invertidas - por exemplo, top-2 e top-3.
Ou talvez você ache que deveria haver alguns outros aspectos positivos nesta lista? Na verdade, você pode até criar seu próprio top seguindo o linkà lista de artigos e encontre os gatilhos mais deliciosos na sua opinião. Neste caso, certifique-se de jogar fora seus tops de 2020 nos comentários, vou ler com grande prazer. Você pode fazer uma lista que seja mais interessante do que a minha?
Claro, a questão do 'interesse' dos avisos é subjetiva de qualquer maneira. Na minha opinião, o principal critério para avaliar a resposta é se um programador que vê um aviso do PVS-Studio vai mudar algo no código correspondente? Esta lista foi compilada apenas a partir de gatilhos para fragmentos, que, na minha opinião, ficariam melhores se os desenvolvedores usassem análise estática. Além disso, não há problemas em experimentar o PVS-Studio verificando o seu próprio ou alguns outros projetos. Você só precisa seguir o link, baixe a versão necessária do analisador e solicite uma chave de teste preenchendo um pequeno formulário.
Bom, isso é tudo para mim, muito obrigado pela atenção e até breve!
Se você deseja compartilhar este artigo com um público de língua inglesa, por favor, use o link de tradução: Nikita Lipilin. Os 10 principais bugs encontrados em projetos C # em 2020 .