Já se tornou uma tradição que os programadores que reabastecem a equipe PVS-Studio iniciem seu trabalho escrevendo um artigo sobre a análise de um projeto de código aberto. Desta vez, um projeto comprovado será o Telerik UI for UWP.
Analisador de código PVS-Studio
O PVS-Studio é uma ferramenta para detectar erros e possíveis vulnerabilidades no código fonte de programas escritos em C, C ++, C # e Java. É executado no Windows, Linux e macOS.
O analisador fornece vários cenários de uso:
- pode ser usado localmente em máquinas de desenvolvimento, integrando como um plug-in ao Visual Studio ou IntelliJ IDEA;
- pode integrar-se à plataforma de garantia de qualidade contínua SonarQube;
- pode ser usado independentemente, integrando-se ao sistema de montagem;
- é possível usar o utilitário de monitoramento de compilação;
- é possível a integração com Azure DevOps, Jenkins, TeamCity, Travis CI e sistemas similares;
- e assim por diante.
Projeto Auditado
A interface do usuário da Telerik para UWP é um conjunto de componentes da interface do usuário para a Plataforma Universal do Windows (UWP). O código fonte do projeto pode ser
encontrado no Github . O conjunto inclui mais de 20 componentes que permitem visualizar dados na forma de gráficos, criar listas e tabelas, usar um mapa para demonstrar os dados associados a um local.
Fragmentos que atraíram a atenção ao estudar o relatório do analisador
Aviso do PVS-Studio :
V3013 É estranho que o corpo da função 'OnMinValuePropertyChanged' seja totalmente equivalente ao corpo da função 'OnMaxValuePropertyChanged'. RadGauge.cs 446
private static void OnMinValuePropertyChanged( DependencyObject sender, DependencyPropertyChangedEventArgs args) { double newVal = (double)args.NewValue; ValidateValue(newVal); RadGauge gauge = sender as RadGauge; if (gauge.panel != null) { gauge.panel.UpdateOnMinMaxValueChange(); } if(AutomationPeer.ListenerExists(AutomationEvents.PropertyChanged)) { var peer = FrameworkElementAutomationPeer.FromElement(gauge) as RadGaugeAutomationPeer; if (peer != null) { peer.RaiseMinimumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue); } } } private static void OnMaxValuePropertyChanged( DependencyObject sender, DependencyPropertyChangedEventArgs args) { double newVal = (double)args.NewValue; ValidateValue(newVal); RadGauge gauge = sender as RadGauge; if (gauge.panel != null) { gauge.panel.UpdateOnMinMaxValueChange(); } if (AutomationPeer.ListenerExists(AutomationEvents.PropertyChanged)) { var peer = FrameworkElementAutomationPeer.FromElement(gauge) as RadGaugeAutomationPeer; if (peer != null) { peer.RaiseMinimumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue); } } }
O analisador detectou dois métodos,
OnMinValuePropertyChanged e
OnMaxValuePropertyChanged , que executam as mesmas ações. Eu tenho fortes suspeitas de que um erro tenha entrado nesse código. Observe que o método
OnMinValuePropertyChanged e o
OnMaxValuePropertyChanged usam
RaiseMinimumPropertyChangedEvent . Ao mesmo tempo, na classe
RadGaugeAutomationPeer, você pode encontrar um método para "Mínimo" e "Máximo":
internal void RaiseMaximumPropertyChangedEvent(double oldValue, double newValue) { this.RaisePropertyChangedEvent( RangeValuePatternIdentifiers.MaximumProperty, oldValue, newValue); } internal void RaiseMinimumPropertyChangedEvent(double oldValue, double newValue) { this.RaisePropertyChangedEvent( RangeValuePatternIdentifiers.MinimumProperty, oldValue, newValue); }
O método
RaiseMaximumPropertyChangedEvent nunca é usado no código, mas
RaiseMinimumPropertyChangedEvent é usado duas vezes. E, você sabe, o desempenho do método
OnMaxValuePropertyChanged levanta perguntas ... Eu acho que deveria escrever o seguinte:
private static void OnMaxValuePropertyChanged( DependencyObject sender, DependencyPropertyChangedEventArgs args) { .... peer.RaiseMaximumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue); .... }
Mas, mesmo assim, o código não parece muito elegante devido ao grande número de elementos repetidos. É difícil de entender, e linhas repetidas atenuam a atenção do programador, e fica mais difícil realizar a revisão do código nessas condições. Mas as ferramentas de análise estática fazem um excelente trabalho para verificar esse código (mas isso não significa que você pode fazer isso sem refatorar e, principalmente, sem reduzir o número de linhas repetidas).
Com o snippet de código acima e o seguinte, podemos assumir que os autores não são avessos a copiar e colar. No entanto, como todos nós ... :)
PVS-Studio Warning :
V3001 Existem
subexpressões idênticas 'element.RenderSize == emptySize' à esquerda e à direita da '||' operador. TiltInteractionEffect.cs 181
private static bool IsPointInElementBounds(FrameworkElement element, Point position) { Size emptySize = new Size(0, 0); if (element.RenderSize == emptySize || element.RenderSize == emptySize) { return false; } return new Rect(....).Contains(position); }
O analisador detectou um fragmento de código de erro no qual, à direita e à esquerda do operador '||' a
instrução if usa as mesmas subexpressões. A segunda subexpressão claramente deveria ter parecido diferente. Talvez no lugar do segundo
RenderSize seja
DesiredSize . Ou não deve haver uma segunda subexpressão. De qualquer forma, esse trecho de código precisa ser corrigido.
PVS-Studio Warning :
V3001 Existem sub-expressões idênticas 'text [0] ==' - '' à esquerda e à direita do '||' operador. RadNumericBox.cs 1057
private void ValidateText() { string text = this.textBox.Text; .... if (text.Length == 1 && (text[0] == '-' || text[0] == '-')) { if (this.isNegative) { this.isNegative = false; } else { this.SetText(string.Empty); } return; } .... }
Aqui, o desenvolvedor grava o texto digitado no campo da caixa de texto em uma variável. Em seguida, o primeiro caractere da string é comparado duas vezes com o mesmo caractere '-', que é uma decisão suspeita. Obviamente, a validação de texto nesta função não funciona como originalmente previsto.
PVS-Studio Warning :
V3001 Existem
subexpressões idênticas 'newValue.HasValue' à esquerda e à direita do operador '&&'. DateTimePicker.cs 576
private static void OnValueChanged(object sender, DependencyPropertyChangedEventArgs args) { DateTimePicker picker = sender as DateTimePicker; var newValue = (DateTime?)args.NewValue; if (newValue.HasValue && newValue != null)
A expressão
newValue.HasValue retornará
true se
newValue contiver algum valor e a expressão
newValue! = Null fará o mesmo. O analisador presta atenção a isso, e o que fazer é remover uma das subexpressões ou substituí-la por outra (se algo mais deveria ter sido verificado), cabe aos desenvolvedores decidir.
PVS-Studio Warning :
V3125 O objeto '
CurrentAttachedMenu ' foi usado depois que foi verificado em relação a nulo. Verifique as linhas: 98, 96. PopupService.cs 98
internal static class PopupService { .... private static void Overlay_PointerPressed(....) { if (CurrentAttachedMenu == null || !CurrentAttachedMenu.hitTestService. HitTest(e.GetCurrentPoint(CurrentAttachedMenu).Position).Any()) { CurrentAttachedMenu.IsOpen = false; HideOverlay(); } } }
Se a variável
CurrentAttachedMenu for
nula , a avaliação da expressão
CurrentAttachedMenu.IsOpen gerará uma exceção. À primeira vista, parece que este é um erro de digitação simples e não significou uma comparação com
nulo , mas a operação inversa - '! ='. Mas uma exceção ocorrerá na condição da
instrução if se a variável
CurrentAttachedMenu for
nula .
Além disso, no código, havia outros
37 dos mesmos avisos, alguns dos quais parecem indicar erros. Mas descrevê-los na estrutura de um artigo ainda é demais, por isso vou deixá-los desacompanhados.
PVS-Studio Warning :
V3019 Possivelmente, uma variável incorreta é comparada com null após a conversão do tipo usando a palavra-chave 'as'. Verifique as variáveis 'dragDropElement', 'uiDragDropElement'. DragDrop.cs 91
internal static void StartDrag(....) { var dragDropElement = sender as IDragDropElement; .... UIElement uiDragDropElement = dragDropElement as UIElement; .... if (dragDropElement == null || !dragDropElement.CanStartDrag(trigger, initializeContext)) { return; } .... }
É muito provável que o autor tenha misturado as variáveis. A desigualdade
nula é verificada não pelo link obtido como resultado da
conversão , mas pelo original (
dragDropElement ). Provavelmente, o link
uiDragDropElement deveria ter sido
verificado . A conjectura também é confirmada pelo fato de que o programador utilizou ainda o
uiDragDropElement sem verificar se é
nulo .
PVS-Studio Warning :
V3030 Verificação recorrente. A condição '! ShowIndicatorWhenNoData' já foi verificada na linha 139. RadDataBoundListBox.PullToRefresh.cs 141
internal void HandlePullToRefreshItemStateChanged(object item, ItemState state) { .... bool showIndicatorWhenNoData = this.ShowPullToRefreshWhenNoData; if (this.realizedItems.Count == 0 && !showIndicatorWhenNoData) { if (state == ItemState.Recycled && !showIndicatorWhenNoData) { this.StopPullToRefreshLoading(false); this.HidePullToRefreshIndicator(); } return; } .... }
O analisador encontrou um pedaço de código no qual, sob duas condições, a mesma variável
showIndicatorWhenNoData é verificada novamente. Talvez a verificação seja simplesmente redundante, mas também existe a possibilidade de que uma das subexpressões duplicadas seja diferente.
Aviso do PVS-Studio :
V3031 Uma verificação excessiva pode ser simplificada. O '||' O operador é cercado por expressões opostas. SelectedItemCollection.cs 77
internal class SelectedItemCollection : ObservableCollection<object> { .... private bool CanInsertItem(object item) { return this.suspendLevel == 0 && this.AllowSelect && ((!this.AllowMultipleSelect && this.Count == 0) || this.AllowMultipleSelect); } }
Este pedaço de código não é formalmente errado. O analisador sugere alguma redundância de código na condição. No entanto, vale lembrar que às vezes o código extra é o resultado de um erro, por exemplo, quando, em vez de uma variável, outra é verificada várias vezes.
Você pode reduzir um pouco essa condição e remover o código extra. Por exemplo, assim:
internal class SelectedItemCollection : ObservableCollection<object> { .... private bool CanInsertItem(object item) { return this.suspendLevel == 0 && this.AllowSelect && (this.AllowMultipleSelect || this.Count == 0); } }
Outras mensagens semelhantes:
- V3031 Uma verificação excessiva pode ser simplificada. O '||' O operador é cercado por expressões opostas. SelectedItemCollection.cs 93
- V3031 Uma verificação excessiva pode ser simplificada. O '||' O operador é cercado por expressões opostas. StackVirtualizationStrategy.cs 49
- V3031 Uma verificação excessiva pode ser simplificada. O '||' O operador está cercado pelas expressões opostas 'state == null' e 'state! = null'. LocalFieldDescriptionsProviderBase.cs 24
Considere outro trecho de código em que o analisador retornou o seguinte:
Avisos do PVS-Studio :
- V3137 A variável 'leftMargin' é designada, mas não é usada no final da função. DragDrop.cs 87
- V3137 A variável 'topMargin' é designada, mas não é usada no final da função. DragDrop.cs 88
internal static class DragDrop { .... double leftMargin = 0d; double topMargin = 0d; if (frameworkElementSource != null) { leftMargin = frameworkElementSource.Margin.Left;
O analisador relata que as
variáveis leftMargin e
topMargin receberam valores, mas depois disso essas variáveis não são usadas até o final do método. Provavelmente não há erro aqui, mas esse código parece suspeito. Isso pode ser devido a um erro de digitação ou refatoração malsucedida.
O mesmo problema foi encontrado em outro lugar: V3137 A variável 'currentColumnLength' é atribuída, mas não é usada no final da função. WrapLayout.cs 824
PVS-Studio Warning :
V3061 O parâmetro 'index' é sempre reescrito no corpo do método antes de ser usado. DataEngine.cs 1443
private static Tuple<Group, int> FindGroupAndItemIndex(.... int index, ....) { if (exhaustiveSearch) { .... } else { var aggregateRowGroup = rowRootGroup; var rowGroupNames = valueProvider.GetRowGroupNames(item); foreach (var groupName in rowGroupNames) { Group group; if (aggregateRowGroup.TryGetGroup(groupName, out group)) { aggregateRowGroup = group; } } index = aggregateRowGroup.IndexOf(item,
O parâmetro
index do método
FindGroupAndItemIndex é substituído antes de ser usado. Provavelmente, isso indica um erro do programador.
PVS-Studio Warning :
V3083 Chamada insegura do evento 'Concluído', NullReferenceException é possível. Considere atribuir um evento a uma variável local antes de invocá-lo. ActionBase.cs 32
internal abstract class ActionBase { .... protected virtual void OnCompleted() { this.IsCompleted = true; if (this.Completed != null) { this.Completed(this, EventArgs.Empty); } } }
O programador permitiu uma chamada potencialmente insegura ao manipulador de eventos nesse método, o que pode levar a uma exceção do tipo
NullReferenceException . Uma exceção será lançada, desde que entre a verificação
nula e a chamada dos manipuladores de eventos, esse evento não permaneça.
Existem outros
49 problemas semelhantes no código. Não será interessante assistir a todos eles neste artigo, e os autores podem encontrá-los facilmente por conta própria usando o PVS-Studio; portanto, passaremos a outros erros.
PVS-Studio Warning :
V3145 Desreferência não segura de um destino WeakReference, considere inspecionar info.Target. O objeto pode ter sido coletado como lixo entre a verificação de 'IsAlive' e o acesso à propriedade 'Target'. FadeAnimation.cs 84
public class RadFadeAnimation : RadAnimation { .... protected internal override void ApplyAnimationValues(PlayAnimationInfo info) { .... if (info.Target.Opacity != opacity)
O analisador alerta sobre o perigo de uma exceção do tipo
NullReferenceException ao acessar a propriedade
info.Target.Opacity . Para entender melhor a essência do problema, é necessário examinar fragmentos da classe
PlayAnimationInfo , em particular a propriedade
Target .
public class PlayAnimationInfo { .... private WeakReference target; .... public PlayAnimationInfo(Storyboard storyboard, RadAnimation animation, UIElement target) { .... this.target = new WeakReference(target); .... } .... public UIElement Target { get { if (this.target.IsAlive) { return this.target.Target as UIElement; } return null; } } .... }
Em geral, quanto mais você digita esse código, mais problemas em potencial você pode encontrar nele. Vejamos talvez o mais interessante - aquele para o qual o analisador emitiu um aviso. O fato é que, mesmo que a execução siga a ramificação então da
instrução if , isso não garante que uma referência diferente de zero será retornada. Independentemente das conversas sobre o elenco, consideramos aqui tudo o que é permitido devido à inicialização do objeto no construtor.
Como isso é possível? O fato é que, entre a verificação
IsAlive e a chamada
Target, a coleta de lixo é realizada, sob a qual o objeto referenciado por
WeakReference cai ,
this.target.Target retornará
nulo . Ou seja, a verificação
IsAlive não garante que na próxima vez que o
Target for acessado, o objeto ainda esteja disponível.
A propósito, a situação é
retorno nulo; captura outra regra de diagnóstico: V3080 Possível desreferência nula. Considere inspecionar 'info.Target'. FadeAnimation.cs 84
Problemas semelhantes no código ocorreram várias vezes:
- V3145 Desreferenciação insegura de um alvo WeakReference, considere inspecionar o alvo. O objeto poderia ter sido coletado como lixo antes do acesso à propriedade 'Target'. MoveXAnimation.cs 80
- V3145 Desreferenciação insegura de um alvo WeakReference, considere inspecionar o alvo. O objeto poderia ter sido coletado como lixo antes do acesso à propriedade 'Target'. MoveYAnimation.cs 80
- V3145 Desreferenciação insegura de um destino WeakReference, considere inspecionar info.Target. O objeto poderia ter sido coletado como lixo antes do acesso à propriedade 'Target'. PlaneProjectionAnimation.cs 244
- V3145 Desreferenciação insegura de um destino WeakReference. O objeto pode ter sido coletado como lixo entre a verificação de 'IsAlive' e o acesso à propriedade 'Target'. WeakEventHandler.cs 109
Vamos para o próximo exemplo.
Aviso PVS-Studio :
V3066 Possível ordem incorreta de argumentos transmitida ao construtor 'NotifyCollectionChangedEventArgs': 'oldItem' e 'newItem'. CheckedItemsCollection.cs 470
public class CheckedItemsCollection<T> : IList<T>, INotifyCollectionChanged { .... private NotifyCollectionChangedEventArgs GenerateArgs(....) { switch (action) { case NotifyCollectionChangedAction.Add: .... case NotifyCollectionChangedAction.Remove: .... case NotifyCollectionChangedAction.Replace: return new NotifyCollectionChangedEventArgs( action, oldItem, newItem, changeIndex);
Para entender o significado desse aviso do analisador, vale a pena examinar os
parâmetros do construtor
NotifyCollectionChangedEventArgs :
public NotifyCollectionChangedEventArgs( NotifyCollectionChangedAction action, object newItem, object oldItem, int index);
O analisador alerta que, na expressão
return new NotifyCollectionChangedEventArgs( action, oldItem, newItem, changeIndex);
trocou as variáveis
oldItem e
newItem . Na definição do construtor, eles são listados em uma ordem diferente. Se isso foi feito conscientemente ou não, só podemos adivinhar.
PVS-Studio Warning :
V3102 Acesso suspeito ao elemento do objeto 'x' por um índice constante dentro de um loop. DataEngine.cs 1718
private class ObjectArrayComparer : IEqualityComparer<object[]> { public bool Equals(object[] x, object[] y) { .... for (int i = 0; i < x.Length; i++) { if (!object.Equals(x[0], y[0]))
A cada iteração no loop, o programador compara
x [0] e
y [0]. No entanto, o loop não faz sentido nesse código, pois apenas os primeiros elementos são comparados. Muito provavelmente, isso significou uma comparação dos elementos correspondentes das matrizes. Então o código correto será assim:
for (int i = 0; i < x.Length; i++) { if (!object.Equals(x[i], y[i])) { return false; } }
Aviso PVS-Studio :
V3123 Talvez o operador '?:'
Funcione de maneira diferente da esperada. Sua prioridade é menor que a prioridade de outros operadores em sua condição. EditRowHostPanel.cs 35
protected override Size MeasureOverride(Size availableSize) { .... bool shouldUpdateRowHeight = editorLine == 0 || displayedElement == null ? false : displayedElement.ContainerType != typeof(DataGridGroupHeader); .... }
Um aviso está associado ao uso do operador '?:'. Tem uma prioridade mais baixa que
! =, ||, ==. Portanto, uma expressão não pode ser avaliada conforme planejado pelo programador. Aparentemente, nesse caso, esse é um falso positivo e o código funciona corretamente. Mas ler esse código é muito difícil e nunca há certeza de que ele seja entendido corretamente. Parece que o desenvolvedor escreveu de tal maneira que ninguém entendeu nada :) A melhor maneira de fazer isso é mais legível - use colchetes ou a
instrução if .
PVS-Studio Warning :
V3078 A ordem de classificação original será perdida após a chamada repetida ao método 'OrderBy'. Use o método 'ThenBy' para preservar a classificação original. GridModel.Selection.cs 107
internal partial class GridModel { private void BuildCellSelectionRegions(....) { .... this.MergeCellSelectionRegions(selectedItemsInView .OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line) .OrderBy(c => c.RowItemInfo.LayoutInfo.Line)); } }
O erro está relacionado à chamar
OrderBy novamente para uma coleção do tipo
IOrderedEnumerable . Aqui, a coleção é classificada primeiro por colunas e depois por linhas. Além disso, no momento da classificação por linhas, a classificação anterior por colunas não é levada em consideração em nenhum lugar. Para não perder a classificação por colunas e classificar a coleção por vários critérios ao mesmo tempo, use
ThenBy :
this.MergeCellSelectionRegions(selectedItemsInView .OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line) .ThenBy(c => c.RowItemInfo.LayoutInfo.Line));
PVS-Studio Warning :
V3008 A variável 'currentColumnLength' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 791, 785. WrapLayout.cs 791
private void OnAvailableLengthChanged(double oldValue, double newValue) { .... if (....) { if (currentColumnLength > 0) { var paddingValue = Math.Max(0, newValue - currentColumnLength); this.paddingRenderInfo.Add(paddingValue); currentColumnLength = 0;
Parecia suspeito para o analisador que a variável
currentColumnLength recebesse um valor duas vezes. A variável não é usada entre atribuições. Independentemente da condição, essa variável será zero. Este código está incorreto ou redundante.
PVS-Studio Warning :
V3127 Dois fragmentos de código semelhantes foram encontrados. Talvez esse seja um erro de digitação e a variável 'emptyIconContainer' deva ser usada em vez de 'filledIconContainer' RadRatingItem.cs 240
public class RadRatingItem : RadContentControl { .... protected override void OnApplyTemplate() { .... this.filledIconContainer = this.GetTemplateChild( "FilledIconContainer") as Border; if (this.filledIconContainer == null)
Por causa de um erro de digitação, duas condições idênticas apareceram no código. A julgar pela exceção gerada, a segunda condição deve ficar assim:
if (this.emptyIconContainer == null) { throw new MissingTemplatePartException( "EmptyIconContainer", typeof(Border)); }
PVS-Studio Warning :
V3020 Uma 'interrupção' incondicional dentro de um loop. NodePool.cs 189
public IEnumerable<KeyValuePair<int, List<T>>> GetUnfrozenDisplayedElements() { foreach (var item in this.generatedContainers) { foreach (var pair in item.Value) { if (!pair.IsFrozen) { yield return item; } break; } } }
O analisador descobriu que aqui a instrução
break não pertence à
instrução if .
A interrupção será executada independentemente do valor do
par. IsFrozen e, por isso, em cada
foreach , apenas uma iteração será executada.
Isso conclui minha consideração dos avisos. Para que os desenvolvedores da Telerik possam realizar uma análise de código mais completa e corrigir defeitos, estamos prontos para fornecer uma licença temporária. Além disso, eles podem tirar proveito do
uso gratuito da opção
PVS-Studio fornecida aos autores de projetos de código aberto.
Conclusão
Embora os desenvolvedores da Telerik UI for UWP tenham feito um ótimo trabalho, não houve erros de digitação, como geralmente acontece :). Todos esses erros podem ser facilmente encontrados por um analisador estático e corrigidos. O mais importante é usar o analisador
correta e regularmente .

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Ekaterina Nikiforova.
Verificando a interface do usuário da Telerik quanto à UWP como uma maneira de iniciar o PVS-Studio .