Este artigo é uma revisão dos erros encontrados no projeto Avalonia UI com o analisador estático PVS-Studio. A Avalonia UI é uma estrutura de interface do usuário de plataforma aberta baseada em XAML de código-fonte. Este é um dos projetos tecnologicamente mais significativos na história do .NET, pois permite que os desenvolvedores criem interfaces de plataforma cruzada com base no sistema WPF. Esperamos que os autores do projeto considerem este artigo útil na correção de alguns bugs e suficientemente convincentes para tornar a análise estática parte de seu processo de desenvolvimento.
Sobre a Avalonia UI
O Avalonia UI (anteriormente conhecido como Perspex) permite que os desenvolvedores criem interfaces de usuário que podem ser executadas no Windows, Linux e MacOS. Como um recurso experimental, também fornece suporte para Android e iOS. A interface do usuário do Avalonia não é um invólucro em torno de outros invólucros, como o Xamarin Forms, que envolve os invólucros do Xamarin, mas acessa diretamente a API nativa. Enquanto assistia a um dos vídeos de demonstração, fiquei surpreso ao saber que você pode gerar um controle para o console Debian. Além disso, graças ao uso da linguagem de marcação XAML, o Avalonia UI oferece mais recursos de design e layout em comparação com outros construtores de UI.
Para citar alguns exemplos, a Avalonia UI é usada no
AvalonStudio (um IDE de plataforma cruzada para desenvolvimento de software em C # e C / C ++) e no
Core2D (um editor de diagrama 2D).
O Wasabi Wallet (uma carteira de bitcoin) é um exemplo de software comercial que utiliza a Avalonia UI.
A luta contra a necessidade de manter várias bibliotecas ao criar um aplicativo de plataforma cruzada é extremamente importante. Queríamos ajudar os autores da Avalonia na interface do usuário com isso, então baixei o código-fonte do projeto e o verifiquei com nosso analisador. Espero que eles vejam este artigo e façam as correções sugeridas e até comecem a usar a análise estática regularmente como parte de seu processo de desenvolvimento. Isso pode ser feito facilmente, graças à opção de licenciamento gratuito do PVS-Studio, disponível para desenvolvedores de código aberto. O uso regular de análises estáticas ajuda a evitar muitos problemas e torna a detecção e a correção de bugs muito mais baratas.
Resultados da análise
Mensagem de diagnóstico do PVS-Studio: V3001 Existem sub-expressões idênticas 'controladaFlags' à esquerda e à direita do operador '^'. WindowImpl.cs 975TwitterClientMessageHandler.cs 52
private void UpdateWMStyles(Action change) { .... var style = (WindowStyles)GetWindowLong(....); .... style = style | controlledFlags ^ controlledFlags; .... }
Para adicionar algum simbolismo, vamos começar com o nosso primeiro diagnóstico de C #. O analisador detectou uma expressão estranha com o operador OR bit a bit. Deixe-me explicar isso usando números:
a expressão
1100 0011 | 1111 0000 ^ 1111 0000
é equivalente a
1100 0011 | 0000 0000
A precedência do OR exclusivo ("^") é maior que a do OR bit a bit ("|"). O programador provavelmente não pretendia esse pedido. O código pode ser corrigido colocando a primeira expressão entre parênteses:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Quanto aos próximos dois avisos, devo admitir: estes são falsos positivos. Veja bem, os desenvolvedores estão usando a API pública do método
TransformToVisual . Nesse caso, o
VisualRoot é sempre um elemento pai do
visual . Não entendi isso ao examinar o aviso; foi só depois que terminei o artigo que um dos autores do projeto me falou sobre isso. Portanto, as correções sugeridas abaixo visam realmente proteger o código contra possíveis modificações que quebram essa lógica, em vez de uma falha real.
Mensagem de diagnóstico do PVS-Studio: V3080 Possível desreferência nula do valor de retorno do método. Considere inspecionar: TranslatePoint (...). VisualExtensions.cs 23
public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; }
Este método é pequeno. O analisador acredita que a desreferência do valor retornado pela chamada do
TranslatePoint é insegura. Vamos dar uma olhada neste método:
public static Point? TranslatePoint(this IVisual visual, Point point, IVisual relativeTo) { var transform = visual.TransformToVisual(relativeTo); if (transform.HasValue) { return point.Transform(transform.Value); } return null; }
De fato, poderia retornar
nulo .
Esse método é chamado seis vezes: três vezes com uma verificação do valor retornado e as outras três sem verificação, acionando o aviso sobre desreferência potencial. O primeiro é o acima, e aqui estão os outros dois:
- V3080 Possível desreferência nula. Considere inspecionar 'p'. VisualExtensions.cs 35
- V3080 Possível desreferência nula. Considere inspecionar 'controlPoint'. Scene.cs 176
Sugiro corrigir esses bugs seguindo o padrão usado nas versões seguras, ou seja, adicionando uma verificação
Nullable <Struct> .HasValue dentro do método
PointToClient :
public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); if (rootPoint.HasValue) return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; else throw ....; }
Mensagem de diagnóstico do PVS-Studio: V3080 Possível desreferência nula do valor de retorno do método. Considere inspecionar: TransformToVisual (...). ViewportManager.cs 381
Este erro é muito semelhante ao anterior:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
Este é o código do método
TransformToVisual :
public static Matrix? TransformToVisual(this IVisual from, IVisual to) { var common = from.FindCommonVisualAncestor(to); if (common != null) { .... } return null; }
A propósito, o método
FindCommonVisualAncestor pode realmente retornar
nulo como o valor padrão para tipos de referência:
public static IVisual FindCommonVisualAncestor(this IVisual visual, IVisual target) { Contract.Requires<ArgumentNullException>(visual != null); return ....FirstOrDefault(); }
O método
TransformToVisual é chamado nove vezes, com apenas sete verificações. A primeira chamada com desreferenciação insegura é a acima, e aqui está a segunda:
V3080 Possível desreferência nula. Considere inspecionar 'transformar'. MouseDevice.cs 80
Mensagem de diagnóstico do PVS-Studio: A expressão
V3022 é sempre verdadeira. Provavelmente o operador '&&' deve ser usado aqui. NavigationDirection.cs 89
public static bool IsDirectional(this NavigationDirection direction) { return direction > NavigationDirection.Previous || direction <= NavigationDirection.PageDown; }
Essa verificação é estranha. A enumeração
NavigationDirection contém 9 tipos, com o tipo
PageDown sendo o último. Talvez nem sempre tenha sido assim, ou talvez essa seja uma proteção contra a adição SUDDEN de novas opções de direção. Na minha opinião, a primeira verificação deve ser suficiente. De qualquer forma, vamos deixar isso para os autores decidirem.
Mensagem de diagnóstico do PVS-Studio: V3066 Possível ordem incorreta de argumentos passada para o construtor 'SelectionChangedEventArgs': 'removedSelectedItems' e 'addedSelectedItems'. DataGridSelectedItemsCollection.cs 338
internal SelectionChangedEventArgs GetSelectionChangedEventArgs() { .... return new SelectionChangedEventArgs (DataGrid.SelectionChangedEvent, removedSelectedItems, addedSelectedItems) { Source = OwningGrid }; }
O analisador está alertando sobre a ordem incorreta do segundo e terceiro argumentos do construtor. Vamos dar uma olhada nesse construtor:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
Ele usa dois contêineres do tipo
IList como argumentos, o que facilita muito a gravação deles na ordem errada. Um comentário no início da classe sugere que este é um erro no código do controle emprestado da Microsoft e modificado para uso em Avalonia. Mas eu ainda insistiria em fixar a ordem dos argumentos, apenas para evitar um relatório de erro e perder tempo procurando um erro em seu próprio código.
Havia mais três erros desse tipo:
Mensagem de diagnóstico do PVS-Studio: V3066 Possível ordem incorreta de argumentos transmitida ao construtor 'SelectionChangedEventArgs': 'removed' e 'added'. AutoCompleteBox.cs 707
OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));
É o mesmo construtor
SelectionChangedEventArgs.Mensagens de diagnóstico do PVS-Studio V3066 :
- Possível ordem incorreta de argumentos transmitida ao construtor 'ItemsRepeaterElementIndexChangedEventArgs': 'oldIndex' e 'newIndex'. ItemsRepeater.cs 532
- Possível ordem incorreta de argumentos passada para o método 'Update': 'oldIndex' e 'newIndex'. ItemsRepeater.cs 536
Dois avisos no método de chamada de um evento.
internal void OnElementIndexChanged(IControl element, int oldIndex, int newIndex) { if (ElementIndexChanged != null) { if (_elementIndexChangedArgs == null) { _elementIndexChangedArgs = new ItemsRepeaterElementIndexChangedEventArgs(element, oldIndex, newIndex); } else { _elementIndexChangedArgs.Update(element, oldIndex, newIndex); } ..... } }
O analisador observou que os argumentos
oldIndex e
newIndex são gravados em uma ordem diferente nos dois métodos
ItemsRepeaterElementIndexChangedEventArgs e
Update :
internal ItemsRepeaterElementIndexChangedEventArgs( IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; } internal void Update(IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; }
Talvez esse código estivesse sendo escrito por diferentes programadores, um dos quais estava mais interessado no passado e o outro no futuro :)
Assim como a edição anterior, esta não exige conserto imediato; ainda não foi determinado se esse código está com defeito.
Mensagem de diagnóstico do PVS-Studio: V3004 A
instrução 'then' é equivalente à instrução 'else'. DataGridSortDescription.cs 235
public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } }
Esta é uma implementação bastante curiosa do método
ThenBy . A interface
IEnumerable , da qual o argumento
seq é herdado, contém o método
ThenBy , que aparentemente deveria ser usado da seguinte maneira:
public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenBy(o => GetValue(o), InternalComparer); } }
Mensagem de diagnóstico do PVS-Studio: V3106 Possível valor negativo do índice. O valor do índice 'index' pode chegar a -1. Animator.cs 68
protected T InterpolationHandler(double animationTime, T neutralValue) { .... if (kvCount > 2) { if (animationTime <= 0.0) { .... } else if (animationTime >= 1.0) { .... } else { int index = FindClosestBeforeKeyFrame(animationTime); firstKeyframe = _convertedKeyframes[index]; } .... } .... }
O analisador tem certeza de que a variável de
índice pode terminar com o valor -1. Essa variável recebe o valor retornado pelo método
FindClosestBeforeKeyFrame , então vamos dar uma olhada nela:
private int FindClosestBeforeKeyFrame(double time) { for (int i = 0; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }
Como você pode ver, o loop contém uma condição seguida por uma declaração de retorno retornando o valor anterior do iterador. É difícil verificar se essa condição é verdadeira e não sei ao certo qual valor o
CueValue terá, mas a descrição sugere que ele assume um valor de 0,0 a 1,0. Mas ainda podemos dizer algumas palavras sobre o
tempo : é a variável
animationTime passada para o método de chamada e é definitivamente maior que zero e menor que um. Se não fosse assim, a execução seguiria um ramo diferente. Se esses métodos forem usados para animação, essa situação se parecerá com um Heisenbug decente. Eu recomendo verificar o valor retornado por
FindClosestBeforeKeyFrame se este caso precisar de algum tratamento especial ou remover o primeiro elemento do loop se ele não atender a outras condições. Não sei exatamente como tudo isso deve funcionar, então eu usaria a segunda solução como exemplo:
private int FindClosestBeforeKeyFrame(double time) { for (int i = 1; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }
Mensagem de diagnóstico do PVS-Studio: O parâmetro do construtor
V3117 'phones' não é usado. Country.cs 25
public Country(string name, string region, int population, int area, double density, double coast, double? migration, double? infantMorality, int gdp, double? literacy, double? phones, double? birth, double? death) { Name = name; Region = region; Population = population; Area = area; PopulationDensity = density; CoastLine = coast; NetMigration = migration; InfantMortality = infantMorality; GDP = gdp; LiteracyPercent = literacy; BirthRate = birth; DeathRate = death; }
Este é um bom exemplo de como a análise estática é melhor que as revisões de código. O construtor é chamado com treze argumentos, um dos quais não é usado. Na verdade, o Visual Studio também pode detectá-lo, mas apenas com a ajuda de diagnósticos de terceiro nível (que geralmente são desativados). Definitivamente, estamos lidando com um bug aqui, porque a classe também contém treze propriedades - uma por argumento - mas não há atribuição à variável
Phones . Como a correção é óbvia, não vou explicá-la.
Mensagem de diagnóstico do PVS-Studio: V3080 Possível desreferência nula. Considere inspecionar 'tabItem'. TabItemContainerGenerator.cs 22
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); tabItem.ParentTabControl = Owner; .... }
O analisador considera a desreferência do valor retornado pelo método
CreateContainer como insegura. Vamos dar uma olhada neste método:
protected override IControl CreateContainer(object item) { var container = item as T; if (item == null) { return null; } else if (container != null) { return container; } else { .... return result; } }
O PVS-Studio pode rastrear uma atribuição de
null mesmo através de uma cadeia de cinquenta métodos, mas não é possível dizer se a execução seguirá esse ramo. Nem eu, por falar nisso ... As chamadas são perdidas entre os métodos substituídos e virtuais, então eu simplesmente sugiro que faça uma verificação adicional apenas no caso de:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return; tabItem.ParentTabControl = Owner; .... }
Mensagem de diagnóstico do PVS-Studio: V3142 Código inacessível detectado. É possível que haja um erro. DevTools.xaml.cs 91
Não adianta citar muito código tentando manter o suspense; Vou lhe dizer imediatamente: esse aviso é um falso positivo. O analisador detectou uma chamada do método que lança uma exceção incondicional:
public static void Load(object obj) { throw new XamlLoadException($"No precompiled XAML found for {obj.GetType()}, make sure to specify x:Class and include your XAML file as AvaloniaResource"); }
Trinta e cinco (!) Avisos sobre código inacessível após as chamadas para esse método eram demais para ignorar, então perguntei a um dos desenvolvedores o que estava acontecendo aqui. Ele me disse que eles usavam uma técnica, em que você substitui as chamadas para um método pelas chamadas para outros métodos usando a biblioteca
Mono.Cecil . Essa biblioteca permite substituir chamadas diretamente no código IL.
Nosso analisador não suporta esta biblioteca, daí a enorme quantidade de falsos positivos. Isso significa que esse diagnóstico deve ser desativado ao verificar a Avalonia UI. Parece um pouco estranho, mas tenho que confessar que sou eu quem fez esse diagnóstico ... Mas, como qualquer outra ferramenta, um analisador estático precisa de alguns ajustes.
Por exemplo, atualmente estamos trabalhando em um diagnóstico para detectar conversões de tipo não seguras. Produz cerca de mil falsos positivos em um projeto de jogo em que a verificação de tipo é feita no lado do mecanismo.
Mensagem de diagnóstico do PVS-Studio: V3009 É estranho que esse método sempre retorne um e o mesmo valor de 'true'. DataGridRows.cs 412
internal bool ScrollSlotIntoView(int slot, bool scrolledHorizontally) { if (....) { .... if (DisplayData.FirstScrollingSlot < slot && DisplayData.LastScrollingSlot > slot) { return true; } else if (DisplayData.FirstScrollingSlot == slot && slot != -1) { .... return true; } .... } .... return true; }
O método retorna
verdadeiro o tempo todo. Talvez seu objetivo tenha mudado desde que foi escrito, mas parece mais um bug. A julgar pelo comentário no início da classe, essa é outra classe de controle emprestada da Microsoft. Se você me perguntar, o
DataGrid é um dos controles menos estáveis, portanto, talvez não seja uma boa ideia confirmar a rolagem quando ela não atender às condições.
Conclusão
Alguns dos erros descritos acima foram emprestados juntamente com o código copiado dos controles WPF, e os autores da Avalonia UI não têm nada a ver com eles. Mas isso não faz diferença para o usuário: uma interface com falha ou falha deixa uma má impressão da qualidade geral do programa.
Mencionei a necessidade de ajustar o analisador: falsos positivos são inevitáveis devido aos princípios de trabalho por trás dos algoritmos de análise estática. Os familiarizados com o
problema da
parada sabem que existem restrições matemáticas no processamento de um pedaço de código com outro. Nesse caso, porém, estamos falando em desativar um diagnóstico em quase cento e meio. Portanto, não há problema de perda de significado no caso da análise estática. Além disso, esse diagnóstico também poderia produzir avisos apontando para erros genuínos, mas seria difícil perceber entre toneladas de falsos positivos.
Devo mencionar a notável qualidade do projeto Avalonia UI! Espero que os desenvolvedores continuem assim. Infelizmente, o número de erros cresce inevitavelmente junto com o tamanho do programa. O ajuste fino e inteligente dos sistemas CI \ CD, com backup com análise estática e dinâmica, é uma das maneiras de manter os bugs afastados. E se você deseja facilitar o desenvolvimento de grandes projetos e gastar menos tempo na depuração, faça o
download e experimente o PVS-Studio!