Geekly articles weekly

WinForms: erros, Holmes

Quadro 5

Gostamos de procurar erros em projetos da Microsoft. Porque É simples: seus projetos geralmente são fáceis de verificar (você pode trabalhar no ambiente do Visual Studio para o qual o PVS-Studio possui um plug-in conveniente) e eles contêm poucos erros. É por isso que o algoritmo de trabalho usual é o seguinte: encontre e baixe um projeto de código aberto do MS; verifique; escolha erros interessantes; verifique se há alguns deles; escreva um artigo sem esquecer de elogiar os desenvolvedores. Ótimo! Ganha-ganha-ganha: demorou um pouco, os chefes estão felizes em ver novos materiais no blog, e o karma é bom. Mas desta vez "algo deu errado". Vamos ver o que descobrimos no código fonte do Windows Forms e se devemos falar muito da Microsoft dessa vez.

1. Introdução

No início de dezembro de 2018, a Microsoft anunciou o lançamento do .NET Core 3 Preview 1. Um pouco antes (cerca de meados de outubro), o GitHub começou a divulgar ativamente as fontes do Windows Forms - a plataforma de interface do usuário do .NET Core para criar aplicativos de desktop do Windows . Você pode ver as estatísticas de confirmação aqui . Agora qualquer pessoa pode baixar o código fonte do WinForms para revisão.

Também baixei as fontes para procurar erros no PVS-Studio. A verificação não causou nenhuma dificuldade. Precisávamos de: Visual Studio 2019, Visualização do SDK do .NET Core 3.0, PVS-Studio. E aqui temos o log dos avisos do analisador.

Depois de receber o relatório do PVS-Studio, costumo classificá-lo por números de diagnóstico em ordem crescente (a janela com o log de mensagens do PVS-Studio no ambiente do Visual Studio tem várias opções de classificação e filtragem da lista). Ele permite que você trabalhe com grupos de erros semelhantes, o que simplifica bastante a análise do código fonte. Marquei erros interessantes na lista com uma "estrela" e só então, depois de analisar todo o log, escrevo fragmentos de código e os descrevo. Como geralmente existem poucos erros, eu os "estimulo" a tentar colocar os mais interessantes no início e no final do artigo. Mas desta vez, houve muitos erros (eh, a intriga não foi salva por muito tempo) e eu os citarei na ordem dos números de diagnósticos.

O que encontramos? 833 avisos de Alto e Médio (249 e 584, respectivamente) foram emitidos para 540.000 linhas de código (não incluindo as vazias) em arquivos de 1670 cs. E sim, tradicionalmente não verificava os testes e não considerava os avisos baixos (havia 215 deles). De acordo com minhas observações anteriores, os avisos são muitos para o projeto MS. Mas nem todos os avisos são erros.

Para este projeto, o número de alarmes falsos foi de cerca de 30%. Em cerca de 20% dos casos, eu simplesmente não conseguia concluir exatamente se foi um erro ou não, porque eu não estava familiarizado com o código o suficiente. E pelo menos 20% dos erros que perdi podem ser considerados "fatores humanos": pressa, cansaço etc. A propósito, o efeito oposto também é possível: alguns gatilhos do mesmo tipo, cujo número pode chegar a 70-80, eu parecia "próximo, mas um", que às vezes podia aumentar o número de erros que eu achava reais.

De qualquer forma, 30% dos avisos indicam erros reais, o que é uma porcentagem bastante grande se você levar em consideração que o analisador não estava pré-configurado.

Portanto, o número de erros que eu consegui encontrar foi de cerca de 240, o que está dentro do intervalo das estatísticas fornecidas. Novamente, na minha opinião, esse não é o resultado mais destacado para um projeto MS (embora cometa apenas 0,44 erros por 1000 linhas de código) e provavelmente há também mais erros reais no código WinForms. Sugiro considerar as razões no final do artigo e agora vamos ver os erros mais interessantes.

Erros

PVS-Studio: V3003 O uso do padrão 'if (A) {...} else if (A) {...}' foi detectado. Há uma probabilidade de presença de erro lógico. Verifique as linhas: 213, 224. ButtonStandardAdapter.cs 213

void PaintWorker(PaintEventArgs e, bool up, CheckState state) { up = up && state == CheckState.Unchecked; .... if (up & IsHighContrastHighlighted()) { .... } else if (up & IsHighContrastHighlighted()) { .... } else { .... } .... }

Os blocos If e else if verificam a mesma condição. Parece copiar e colar. Isso é um erro? Se você observar a declaração do método IsHighContrastHighlighted , poderá duvidar:

 protected bool IsHighContrastHighlighted() { return SystemInformation.HighContrast && Application.RenderWithVisualStyles && (Control.Focused || Control.MouseIsOver || (Control.IsDefault && Control.Enabled)); }

O método provavelmente pode retornar valores diferentes para chamadas seqüenciais. E o que está acontecendo no método de chamada, é claro, parece estranho, mas tem o direito de existir. No entanto, eu aconselharia os autores a dar uma olhada neste fragmento de código. Apenas no caso. Também é um bom exemplo de quão difícil é tirar conclusões ao analisar código desconhecido.

PVS-Studio: V3004 A instrução 'then' é equivalente à instrução 'else'. RichTextBox.cs 1018

 public int SelectionCharOffset { get { int selCharOffset = 0; .... NativeMethods.CHARFORMATA cf = GetCharFormat(true); // if the effects member contains valid info if ((cf.dwMask & RichTextBoxConstants.CFM_OFFSET) != 0) { selCharOffset = cf.yOffset; // <= } else { // The selection contains characters of different offsets, // so we just return the offset of the first character. selCharOffset = cf.yOffset; // <= } .... } .... }

E há definitivamente um erro de copiar e colar aqui. Independentemente da condição, a variável selCharOffset sempre obterá o mesmo valor.

Existem mais dois desses erros no código WinForms:
  • V3004 A instrução 'then' é equivalente à instrução 'else'. SplitContainer.cs 1700
  • V3004 A instrução 'then' é equivalente à instrução 'else'. ToolstripProfessionalRenderer.cs 371

PVS-Studio: V3008 A variável recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 681, 680. ProfessionalColorTable.cs 681

 internal void InitSystemColors(ref Dictionary<KnownColors, Color> rgbTable) { .... rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonFace; rgbTable[ProfessionalColorTable.KnownColors.msocbvcrCBBdrOuterDocked] = buttonShadow; .... }

O método preenche o dicionário rgbTable . O analisador apontou para um fragmento de código no qual valores diferentes são gravados duas vezes na mesma chave em sequência. As coisas ficariam bem, mas ainda existem 16 desses fragmentos nesse método. Não parece mais um erro único. Mas por que eles fazem isso continua sendo um mistério para mim. Não encontrei nenhum sinal de código gerado automaticamente. Parece com isso no editor:

Quadro 3

Darei a você os dez primeiros avisos da lista:

  1. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 785, 784. ProfessionalColorTable.cs 785
  2. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 787, 786. ProfessionalColorTable.cs 787
  3. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 789, 788. ProfessionalColorTable.cs 789
  4. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 791, 790. ProfessionalColorTable.cs 791
  5. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 797, 796. ProfessionalColorTable.cs 797
  6. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 799, 798. ProfessionalColorTable.cs 799
  7. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 807, 806. ProfessionalColorTable.cs 807
  8. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 815, 814. ProfessionalColorTable.cs 815
  9. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 817, 816. ProfessionalColorTable.cs 817
  10. V3008 A variável recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 823, 822. ProfessionalColorTable.cs 823

PVS-Studio: V3011 Foram encontradas duas condições opostas. A segunda condição é sempre falsa. Verifique as linhas: 5242, 5240. DataGrid.cs 5242

 private void CheckHierarchyState() { if (checkHierarchy && listManager != null && myGridTable != null) { if (myGridTable == null) // <= { // there was nothing to check return; } for (int j = 0; j < myGridTable.GridColumnStyles.Count; j++) { DataGridColumnStyle gridColumn = myGridTable.GridColumnStyles[j]; } checkHierarchy = false; } }

O operador de retorno nunca será executado. Provavelmente, a condição myGridTable! = Null no externo se o bloco foi adicionado posteriormente durante a refatoração. E agora a verificação de myGridTable == null não faz sentido. Para melhorar a qualidade do código, você deve remover esta verificação.

PVS-Studio: V3019 Possivelmente uma variável incorreta é comparada a nula após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis ​​'left', 'cscLeft'. TypeCodeDomSerializer.cs 611

PVS-Studio: V3019 Possivelmente uma variável incorreta é comparada a nula após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis ​​'right', 'cscRight'. TypeCodeDomSerializer.cs 615

 public int Compare(object left, object right) { OrderedCodeStatementCollection cscLeft = left as OrderedCodeStatementCollection; OrderedCodeStatementCollection cscRight = right as OrderedCodeStatementCollection; if (left == null) { return 1; } else if (right == null) { return -1; } else if (right == left) { return 0; } return cscLeft.Order - cscRight.Order; // <= }

O analisador gerou dois avisos para o método Compare de uma só vez. Qual é o problema? É que os valores cscLeft e cscRight não são verificados quanto a nulo . Eles podem obter esse valor após a conversão sem êxito no tipo OrderedCodeStatementCollection . Em seguida, uma exceção será lançada na última expressão de retorno . Essa situação é possível quando todas as verificações à esquerda e à direita passam e não levam a uma saída preliminar do método.

Para corrigir o código, você deve usar cscLeft / cscRight em vez de esquerda / direita em qualquer lugar.

PVS-Studio: V3020 Uma 'quebra' incondicional dentro de um loop. SelectionService.cs 421

 void ISelectionService.SetSelectedComponents( ICollection components, SelectionTypes selectionType) { .... // Handle the click case object requestedPrimary = null; int primaryIndex; if (fPrimary && 1 == components.Count) { foreach (object o in components) { requestedPrimary = o; if (o == null) { throw new ArgumentNullException(nameof(components)); } break; } } .... }

Este fragmento refere-se mais ao "cheiro do código". Não há erro aqui. Mas surgem dúvidas sobre a forma como o ciclo foreach é organizado. É claro por que é necessário aqui: devido à necessidade de extrair elementos da coleção, passada como ICollection . Mas por que o loop, inicialmente projetado para iteração única (a pré-condição é a presença de um único elemento nos componentes da coleção), requer suporte adicional, como quebra ? Provavelmente, a resposta pode ser considerada da seguinte maneira: "Historicamente, isso veio a ser". O código parece feio.

PVS-Studio: V3022 A expressão 'ocxState! = Null' sempre é verdadeira. AxHost.cs 2186

 public State OcxState { .... set { .... if (value == null) { return; } .... ocxState = value; if (ocxState != null) // <= { axState[manualUpdate] = ocxState._GetManualUpdate(); licenseKey = ocxState._GetLicenseKey(); } else { axState[manualUpdate] = false; licenseKey = null; } .... } }

Devido a um erro lógico, "código morto" ocorreu neste fragmento. Expressões no bloco else nunca serão executadas.

PVS-Studio: V3027 A variável 'e' foi utilizada na expressão lógica antes de ser verificada contra nula na mesma expressão lógica. ImageEditor.cs 99

 public override object EditValue(....) { .... ImageEditor e = ....; Type myClass = GetType(); if (!myClass.Equals(e.GetType()) && e != null && myClass.IsInstanceOfType(e)) { .... } .... }

A variável e na condição é usada primeiro e depois verificada como nula . Olá, NullReferenceException .

Mais um desses erros:

PVS-Studio: V3027 A variável 'dropDownItem' foi utilizada na expressão lógica antes de ser verificada contra nula na mesma expressão lógica. ToolStripMenuItemDesigner.cs 1351

 internal void EnterInSituEdit(ToolStripItem toolItem) { .... ToolStripDropDownItem dropDownItem = toolItem as ToolStripDropDownItem; if (!(dropDownItem.Owner is ToolStripDropDownMenu) && dropDownItem != null && dropDownItem.Bounds.Width < commitedEditorNode.Bounds.Width) { .... } .... }

A situação é semelhante à anterior, mas com a variável dropDownItem . Penso que esses erros aparecem como resultado de refatoração descuidada. Provavelmente, uma parte da condição ! (DropDownItem.Owner é ToolStripDropDownMenu) foi adicionada ao código posteriormente.

PVS-Studio: V3030 Verificação recorrente. A condição 'columnCount> 0' já foi verificada na linha 3900. ListView.cs 3903

 internal ColumnHeader InsertColumn( int index, ColumnHeader ch, bool refreshSubItems) { .... // Add the column to our internal array int columnCount = (columnHeaders == null ? 0 : columnHeaders.Length); if (columnCount > 0) { ColumnHeader[] newHeaders = new ColumnHeader[columnCount + 1]; if (columnCount > 0) { System.Array.Copy(columnHeaders, 0, newHeaders, 0, columnCount); } .... } .... }

Um erro que pode parecer inofensivo. De fato, é realizada uma verificação desnecessária que não afeta a lógica operacional. E às vezes isso é feito quando você precisa verificar o estado de algum componente visual novamente, por exemplo, obtendo o número de entradas na lista. Mas, neste caso, a variável local columnCount é verificada duas vezes. É muito suspeito. Eles queriam verificar outra variável ou usaram uma condição errada em uma das verificações.

PVS-Studio: V3061 O parâmetro 'lprcClipRect' sempre é reescrito no corpo do método antes de ser usado. WebBrowserSiteBase.cs 281

 int UnsafeNativeMethods.IOleInPlaceSite.GetWindowContext( out UnsafeNativeMethods.IOleInPlaceFrame ppFrame, out UnsafeNativeMethods.IOleInPlaceUIWindow ppDoc, NativeMethods.COMRECT lprcPosRect, NativeMethods.COMRECT lprcClipRect, NativeMethods.tagOIFI lpFrameInfo) { ppDoc = null; ppFrame = Host.GetParentContainer(); lprcPosRect.left = Host.Bounds.X; lprcPosRect.top = Host.Bounds.Y; .... lprcClipRect = WebBrowserHelper.GetClipRect(); // <= if (lpFrameInfo != null) { lpFrameInfo.cb = Marshal.SizeOf<NativeMethods.tagOIFI>(); lpFrameInfo.fMDIApp = false; .... } return NativeMethods.S_OK; }

Um erro imprevisto. Sim, o parâmetro lprcClipRect é realmente inicializado com um novo valor sem usá-lo de forma alguma. Mas o que isso leva no final? Eu acho que em algum lugar no código de chamada a referência passada por esse parâmetro permanecerá inalterada, embora não fosse a intenção. Realmente, aprecie o manuseio de outras variáveis ​​nesse método. Até o nome (prefixo "Get") sugere que alguma inicialização será realizada dentro do método através de parâmetros passados. E é assim. Os dois primeiros parâmetros ( ppFrame e ppDoc ) são passados ​​com o modificador out e obtêm novos valores. As referências lprcPosRect e lpFrameInfo são usadas para acessar e inicializar os campos da classe. Apenas lprcClipRect se destaca. Provavelmente, o modificador out ou ref é necessário para este parâmetro.

PVS-Studio: V3066 Possível ordem incorreta de argumentos passada para o método 'AdjustCellBorderStyle': 'isFirstDisplayedRow' e 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934

 protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { .... dgvabsEffective = AdjustCellBorderStyle( DataGridView.AdvancedCellBorderStyle, dgvabsPlaceholder, singleVerticalBorderAdded, singleHorizontalBorderAdded, isFirstDisplayedRow, // <= isFirstDisplayedColumn); // <= .... }

O analisador suspeitou que os dois últimos argumentos foram confusos. Vamos dar uma olhada na declaração do método AdjustCellBorderStyle :

 public virtual DataGridViewAdvancedBorderStyle AdjustCellBorderStyle( DataGridViewAdvancedBorderStyledataGridViewAdvancedBorderStyleInput, DataGridViewAdvancedBorderStyle dataGridViewAdvancedBorderStylePlaceholder, bool singleVerticalBorderAdded, bool singleHorizontalBorderAdded, bool isFirstDisplayedColumn, bool isFirstDisplayedRow) { .... }

Parece um erro. Sim, alguns argumentos são frequentemente transmitidos em ordem inversa, por exemplo, para trocar algumas variáveis. Mas não acho que seja esse o caso. Nada nos métodos de chamada ou chamada indica esse padrão de uso. Primeiro, as variáveis ​​do tipo bool são misturadas. Segundo, os nomes dos métodos também são regulares: sem "Swap" ou "Reverse". Além disso, não é tão difícil cometer um erro como esse. As pessoas geralmente percebem a ordem do par "linha / coluna" de maneira diferente. Para mim, por exemplo, é a "linha / coluna" que é familiar. Mas para o autor do método chamado AdjustCellBorderStyle , obviamente, a ordem mais comum é "coluna / linha".

PVS-Studio: V3070 A variável não inicializada 'LANG_USER_DEFAULT' é usada ao inicializar a variável 'LOCALE_USER_DEFAULT'. NativeMethods.cs 890

 internal static class NativeMethods { .... public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT); .... }

Erro raro. A ordem de inicialização dos campos da classe é misturada. Para calcular o valor do campo LOCALE_USER_DEFAULT, é utilizado o campo LANG_USER_DEFAULT , que ainda não foi inicializado e tem um valor de 0. A propósito, a variável LANG_USER_DEFAULT não é usada em nenhum outro lugar do código. Fiz uma milha extra e escrevi um pequeno programa de console que simula a situação. Substituí algumas constantes usadas no código WinForms pelos seus valores reais:

 internal static class NativeMethods { public static readonly int LOCALE_USER_DEFAULT = MAKELCID(LANG_USER_DEFAULT); public static readonly int LANG_USER_DEFAULT = MAKELANGID(0x00, 0x01); public static int MAKELANGID(int primary, int sub) { return ((((ushort)(sub)) << 10) | (ushort)(primary)); } public static int MAKELCID(int lgid) { return MAKELCID(lgid, 0x0); } public static int MAKELCID(int lgid, int sort) { return ((0xFFFF & lgid) | (((0x000f) & sort) << 16)); } } class Program { static void Main() { System.Console.WriteLine(NativeMethods.LOCALE_USER_DEFAULT); } }

Como resultado, o console exibirá: 0. Agora, vamos trocar as declarações dos campos LOCALE_USER_DEFAULT e LANG_USER_DEFAULT . O resultado da execução do programa é o seguinte: 1024. Acho que não há mais nada a comentar aqui.

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar 'ces'. CodeDomSerializerBase.cs 562

 protected void DeserializeStatement( IDesignerSerializationManager manager, CodeStatement statement) { .... CodeExpressionStatement ces = statement as CodeExpressionStatement; if (ces != null) { .... } else { .... DeserializeExpression(manager, null, ces.Expression); // <= .... } .... }

O código que deve "travar" regularmente, porque você pode entrar no ramo else apenas quando a referência ces é igual a nulo .

Outro exemplo semelhante:

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar 'comboBox'. ComboBox.cs 6610

 public void ValidateOwnerDrawRegions(ComboBox comboBox, ....) { .... if (comboBox != null) { return; } Rectangle topOwnerDrawArea = new Rectangle(0, 0, comboBox.Width, innerBorder.Top); .... }

O código paradoxal. Aparentemente, a verificação if (comboBox! = Null) foi confundida com if (comboBox == null) . E assim, obteremos outra NullReferenceException.

Consideramos dois erros bastante óbvios da V3080 , nos quais é possível rastrear visualmente um uso potencial de referência nula em um método. Mas o diagnóstico do V3080 é muito mais eficiente e pode encontrar esses erros nas cadeias de chamadas de método. Há pouco tempo, aprimoramos significativamente os mecanismos de fluxo de dados e análise interprocedural. Você pode ler sobre isso no artigo " Tipos de referência nula no C # 8.0 e análise estática ". Mas aqui está esse tipo de erro detectado no WinForms:

PVS-Studio: V3080 Possível desreferência nula dentro do método em 'reader.NameTable'. Considere inspecionar o primeiro argumento: contentReader. ResXResourceReader.cs 267

 private void EnsureResData() { .... XmlTextReader contentReader = null; try { if (fileContents != null) { contentReader = new XmlTextReader(....); } else if (reader != null) { contentReader = new XmlTextReader(....); } else if (fileName != null || stream != null) { .... contentReader = new XmlTextReader(....); } SetupNameTable(contentReader); // <= .... } finally { .... } .... }

Veja o que acontece com a variável contentReader no corpo do método. Após a inicialização com nulo, ele será inicializado novamente em uma das verificações. Mas a série de verificações não termina no bloco else . Isso significa que, em alguns casos raros (ou devido a refatoração no futuro), a referência ainda pode permanecer nula. Em seguida, ele será passado para o método SetupNameTable , onde é usado sem nenhuma verificação:

 private void SetupNameTable(XmlReader reader) { reader.NameTable.Add(ResXResourceWriter.TypeStr); reader.NameTable.Add(ResXResourceWriter.NameStr); .... }

Este é um código potencialmente inseguro.

E mais um erro em que o analisador precisou passar pela cadeia de chamadas para detectar o problema:

PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar o 'layout'. DockAndAnchorLayout.cs 156

 private static Rectangle GetAnchorDestination( IArrangedElement element, Rectangle displayRect, bool measureOnly) { .... AnchorInfo layout = GetAnchorInfo(element); int left = layout.Left + displayRect.X; .... }

O analisador afirma que é possível obter uma referência nula do método GetAnchorInfo , que causará uma exceção ao calcular o valor esquerdo . Vamos percorrer toda a cadeia de chamadas e verificar se é verdade:

 private static AnchorInfo GetAnchorInfo(IArrangedElement element) { return (AnchorInfo)element.Properties.GetObject(s_layoutInfoProperty); } public object GetObject(int key) => GetObject(key, out _); public object GetObject(int key, out bool found) { short keyIndex = SplitKey(key, out short element); if (!LocateObjectEntry(keyIndex, out int index)) { found = false; return null; } // We have found the relevant entry. See if // the bitmask indicates the value is used. if (((1 << element) & s_objEntries[index].Mask) == 0) { found = false; return null; } found = true; switch (element) { case 0: return s_objEntries[index].Value1; .... default: Debug.Fail("Invalid element obtained from LocateObjectEntry"); return null; } }

De fato, em alguns casos, o método GetObject que encerra a cadeia de chamadas retornará nulo , que será passado para o método chamador sem nenhuma verificação adicional. Provavelmente, é necessário cobrir essa situação no método GetAnchorDestination .

Existem muitos desses erros no código WinForms, mais de 70 . Todos são parecidos e não os descreverei no artigo.

PVS-Studio: V3091 Análise empírica. É possível que um erro de digitação esteja presente dentro da cadeia literal: "ShowCheckMargin". A palavra 'ShowCheckMargin' é suspeita. PropertyNames.cs 136

 internal class PropertyNames { .... public static readonly string ShowImageMargin = "ShowCheckMargin"; ... public static readonly string ShowCheckMargin = "ShowCheckMargin"; .... }

Um bom exemplo de erro que não é tão fácil de encontrar. Ao inicializar os campos da classe, o mesmo valor é usado, embora o autor do código obviamente não pretendesse (a culpa é de copiar e colar). O analisador chegou a essa conclusão comparando os nomes das variáveis ​​e os valores das seqüências atribuídas. Eu dei apenas linhas com erros, mas você deve verificar como fica no editor de código:

Quadro 2

A detecção de tais erros é o que demonstra todo o poder e atenção infinita das ferramentas de análise estática.

PVS-Studio: V3095 O objeto 'currentForm' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 3386, 3404. Application.cs 3386

 private void RunMessageLoopInner(int reason, ApplicationContext context) { .... hwndOwner = new HandleRef( null, UnsafeNativeMethods.GetWindowLong( new HandleRef(currentForm, currentForm.Handle), // <= NativeMethods.GWL_HWNDPARENT)); .... if (currentForm != null && ....) .... }

Isto é clássico. A variável currentForm é usada sem nenhuma verificação. Mas é verificado nulo no código. Nesse caso, posso aconselhá-lo a ficar mais atento ao trabalhar com tipos de referência e também usar analisadores estáticos :).

Mais um desses erros:

PVS-Studio: V3095 O objeto 'backgroundBrush' foi usado antes de ser verificado com valor nulo. Verifique as linhas: 2331, 2334. DataGrid.cs 2331

 public Color BackgroundColor { .... set { .... if (!value.Equals(backgroundBrush.Color)) // <= { if (backgroundBrush != null && BackgroundBrush != DefaultBackgroundBrush) .... } } }

No código WinForms, me deparei com mais de 60 desses erros. Na minha opinião, todos eles são bastante críticos e requerem atenção dos desenvolvedores. Mas não é tão interessante contar mais sobre eles no artigo, então vou me limitar aos dois mencionados acima.

PVS-Studio: V3125 O objeto '_propInfo' foi usado e verificado contra nulo em diferentes ramificações de execução. Verifique as linhas: 996, 982. Binding.cs 996

 private void SetPropValue(object value) { .... if (....) { if .... else if (_propInfo != null) .... } else { _propInfo.SetValue(_control, value); } .... }

Por uma questão de integridade - também é uma espécie de clássico, erro V3125 . A situação oposta. No início, o desenvolvedor usa uma referência potencialmente nula com segurança, depois de a ter verificado nula, mas para de fazer mais no código.

E mais um desses erros:

PVS-Studio: V3125 O objeto 'proprietário' foi usado depois que foi verificado contra nulo. Verifique as linhas: 64, 60. FlatButtonAppearance.cs 64

 public int BorderSize { .... set { .... if (owner != null && owner.ParentInternal != null) { LayoutTransaction.DoLayoutIf(....); } owner.Invalidate(); // <= .... } }

Adorável Mas esse é um ponto de vista de um pesquisador externo. Afinal, o analisador encontrou mais de 50 desses padrões no código WinForms além desses dois V3125 . Os desenvolvedores têm muito a trabalhar.

E, finalmente, há um erro interessante, na minha opinião.

PVS-Studio: V3137 A variável 'hCurrentFont' é atribuída, mas não é usada no final da função. DeviceContext2.cs 241

 sealed partial class DeviceContext : .... { WindowsFont selectedFont; .... internal void DisposeFont(bool disposing) { if (disposing) { DeviceContexts.RemoveDeviceContext(this); } if (selectedFont != null && selectedFont.Hfont != IntPtr.Zero) { IntPtr hCurrentFont = IntUnsafeNativeMethods.GetCurrentObject( new HandleRef(this, hDC), IntNativeMethods.OBJ_FONT); if (hCurrentFont == selectedFont.Hfont) { // select initial font back in IntUnsafeNativeMethods.SelectObject(new HandleRef(this, Hdc), new HandleRef(null, hInitialFont)); hCurrentFont = hInitialFont; // <= } selectedFont.Dispose(disposing); selectedFont = null; } } .... }

Vamos ver o que alertou o analisador e por que isso pode indicar um problema de que uma variável recebeu um valor, mas nunca foi usada no código.

O arquivo DeviceContext2.cs contém uma classe parcial. O método DisposeFont é usado para liberar recursos depois de trabalhar com gráficos: contexto do dispositivo e fontes. Para uma melhor compreensão, forneci todo o método DisposeFont . Preste atenção na variável local hCurrentFont . O problema é que a declaração dessa variável no método oculta o campo de classe com o mesmo nome. Encontrei dois métodos da classe DeviceContext em que o campo com o nome hCurrentFont é usado:

 public IntPtr SelectFont(WindowsFont font) { .... hCurrentFont = font.Hfont; .... } public void ResetFont() { .... hCurrentFont = hInitialFont; }

Veja o método ResetFont . A última linha é exatamente o que o método DisposeFont faz no sub-bloco se (é para isso que o analisador aponta). Este campo hCurrentFont com o mesmo nome é declarado em outra parte da classe parcial no arquivo DeviceContext.cs :

 sealed partial class DeviceContext : .... { .... IntPtr hInitialFont; .... IntPtr hCurrentFont; // <= .... }

Assim, um erro óbvio foi cometido. Outra questão está em sua importância. Agora, como resultado do trabalho do método DisposeFont na seção marcada com o comentário "selecione a fonte inicial de volta", o campo hCurrentFont não será inicializado. Eu acho que apenas os autores do código podem dar um veredicto exato.

Conclusões

Então, desta vez, vou ter que criticar a EM um pouco. No WinForms, existem muitos erros que exigem muita atenção dos desenvolvedores. Talvez seja culpa de alguma pressa com a qual a MS trabalha no .NET Core 3 e componentes, incluindo o WinForms. Na minha opinião, o código WinForms ainda é "bruto", mas espero que a situação mude para melhor em breve.

A segunda razão para o grande número de erros pode ser que nosso analisador simplesmente se tornou melhor na busca por eles :).

A propósito, em breve será publicado um artigo do meu colega Sergey Vasiliev, no qual ele pesquisa e encontra muitos problemas no código das bibliotecas do .NET Core. Espero que o trabalho dele também contribua para melhorar as características da plataforma .NET, pois sempre tentamos informar os desenvolvedores sobre os resultados da análise de seus projetos.

E para aqueles que desejam melhorar seus produtos por conta própria ou procurar erros nos projetos de outras pessoas, sugiro que você baixe e experimente o PVS-Studio .

Código limpo para todos!

Source: https://habr.com/ru/post/pt462809/

More articles:


All Articles