Este artigo é o resultado da verificação do projeto da UI da Avalonia usando o analisador estático PVS-Studio. O Avalonia UI é uma plataforma de interface de usuário baseada em XAML de código-fonte aberto. Este é um dos projetos tecnologicamente significativos na história do .NET, pois permite criar interfaces de plataforma cruzada com base no sistema WPF. Espero que este artigo ajude os autores a corrigir alguns erros e convencê-los a usar analisadores estáticos no futuro.
Sobre a Avalonia UI
O projeto Avalonia UI (anteriormente chamado Perspex) fornece a capacidade de criar interfaces de usuário executadas no Windows, Linux e MacOS. Também há suporte experimental para Android e iOS no momento. A interface do usuário do Avalonia não é um invólucro sobre invólucros, mas refere-se à API nativa. Diferentemente do Xamarin Forms, que envolve os invólucros do Xamarin. Em um dos vídeos de demonstração, fiquei impressionado com a capacidade de trazer o controle para o console Debian. Além disso, o projeto oferece mais recursos de layout e design do que os designers de interface convencionais, graças ao uso da marcação XAML.
Os projetos que já usam a interface do usuário do Avalonia incluem o
AvalonStudio (um IDE de plataforma cruzada para desenvolvimento em C # e C / C ++) e o
Core2D (editor de diagramas e diagramas 2D). Como projeto comercial, você pode trazer a
Wasabi Wallet (carteira Bitcoin).
A luta contra a necessidade de ter várias bibliotecas diferentes ao criar um aplicativo de plataforma cruzada é de grande importância. Queríamos ajudar o projeto, baixei o projeto e o verifiquei com nosso analisador. Espero que os autores prestem atenção a este artigo e façam as alterações necessárias no código, ou talvez introduzam análises estáticas regulares no processo de desenvolvimento. Para fazer isso, eles podem aproveitar a opção de licenciamento gratuito do PVS-Studio para projetos de código aberto. O uso regular de um analisador estático ajuda a evitar muitos problemas e reduz o custo de detectar e corrigir muitos erros.
Resultados da validação
PVS-Studio Warning: 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; .... }
Começarei simbolicamente com nossos primeiros diagnósticos em C #. O analisador detectou um uso estranho do operador OR bit a bit. Deixe-me explicar sobre os números o que está acontecendo aqui:
expressão
1100 0011 | 1111 0000 ^ 1111 0000
semelhante a este:
1100 0011 | 0000 0000
Um OR exclusivo ("^") tem uma prioridade mais alta que um OR bit a bit ("|"). Muito provavelmente, uma ordem diferente de operações foi implícita aqui. Nesse caso, você deve pegar a primeira expressão entre parênteses:
private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }
Antes dos próximos dois avisos, devo admitir: os falsos positivos. Isso ocorre devido ao uso da API pública, o método
TransformToVisual . No nosso caso, o
VisualRoot é sempre o pai do
visual . Não entendi isso ao analisar a resposta, o autor do projeto me disse depois de escrever o artigo. Portanto, as edições propostas no artigo não são para proteção contra uma queda real, mas contra uma revisão em potencial que quebra essa lógica.
PVS-Studio Warning: 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; }
Um pequeno método. O analisador considera que a exclusão do resultado de uma chamada para o TranslatePoint não é segura. Dê 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, há um retorno
nulo .
Este método tem 6 chamadas. Em três casos, o valor é verificado e, no restante, o PVS-Studio detecta desreferenciamento em potencial e emite avisos. Eu citei o primeiro acima, e os outros dois avisos estão aqui:
- V3080 Possível desreferência nula. Considere inspecionar 'p'. VisualExtensions.cs 35
- V3080 Possível desreferência nula. Considere inspecionar 'controlPoint'. Scene.cs 176
Proponho
corrigi- lo por analogia, adicionando a 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 ....; }
PVS-Studio Warning: V3080 Possível desreferência nula do valor de retorno do método. Considere inspecionar: TransformToVisual (...). ViewportManager.cs 381
Um caso muito semelhante ao exemplo anterior:
private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }
O método
TransformToVisual é semelhante a este:
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 é usado em nove locais; há verificações em sete. O primeiro aviso a ser usado sem verificação é mais alto e o último está aqui:
V3080 Possível desreferência nula. Considere inspecionar 'transformar'. MouseDevice.cs 80
Aviso 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; }
Cheque estranho. Na enumeração
NavigationDirection , existem 9 tipos e o
PageDown é o último deles. Talvez nem sempre tenha sido esse o caso, ou é um seguro contra o surgimento repentino de novas opções de indicação. Parece-me que o primeiro cheque é suficiente aqui. Deixarei a decisão para os autores do projeto.
Aviso PVS-Studio: V3066 Possível ordem incorreta de argumentos transmitida ao construtor 'SelectionChangedEventArgs': 'removedSelectedItems' e 'addedSelectedItems'. DataGridSelectedItemsCollection.cs 338
internal SelectionChangedEventArgs GetSelectionChangedEventArgs() { .... return new SelectionChangedEventArgs (DataGrid.SelectionChangedEvent, removedSelectedItems, addedSelectedItems) { Source = OwningGrid }; }
Nesse caso, o analisador sugeriu que o segundo e o terceiro argumentos do construtor estão confusos. Vamos dar uma olhada no construtor chamado:
public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }
São aceitos dois contêineres do tipo
IList , fáceis de misturar. A julgar pelo comentário no início da classe, este é um erro no código de controle copiado da Microsoft e modificado em Avalonia. Mas parece-me que vale a pena corrigir a ordem dos argumentos do método, pelo menos para não procurar um possível erro em si mesmo quando chegar um relatório de bug.
O analisador encontrou mais três erros semelhantes:
Aviso 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));
Mesmo construtor
SelectionChangedEventArgs.PVS-Studio V3066 Avisos :
- 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
Duas operações em um método de uma chamada de 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,
tanto em
ItemsRepeaterElementIndexChangedEventArgs quanto no método
Update , os argumentos
oldIndex e
newIndex têm uma ordem diferente:
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 o código tenha sido escrito por diferentes programadores, e para um é mais importante o que aconteceu e para o outro - o que acontecerá :)
Como no caso anterior, você não deve editá-lo imediatamente, é necessário verificar se há realmente um erro.
PVS-Studio Warning: 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); } }
Uma implementação extremamente interessante do método
ThenBy . A interface
IEnumerable , da qual o argumento
seq é herdado
, possui um método
ThenBy ; Suponho que seu uso esteja implícito. Assim:
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); } }
Aviso 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 acredita que o
índice pode ter o valor -1. A variável é obtida no método
FindClosestBeforeKeyFrame , veja:
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 podemos ver, a condição é verificada no loop e o valor anterior do iterador é retornado. A condição é bastante difícil de verificar e não sei dizer exatamente o que
é CueValue , mas, de acordo com a descrição, o valor é de 0,0 a 1,0. Podemos dizer algo sobre o
tempo , este é o
animationTime do método de chamada, é definitivamente mais que zero e menos que um. Caso contrário, a execução do programa continuaria em uma ramificação diferente. Se esses são os métodos chamados para renderizar a animação, a situação parece um bom bug flutuante. Eu adicionaria uma verificação do resultado de
FindClosestBeforeKeyFrame se um tratamento especial for necessário nesse caso. Ou - se o primeiro elemento não atender a outras condições - remova-o do loop. Sem saber como tudo isso deve funcionar, vou escolher a segunda opção como exemplo de correção:
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."); }
PVS-Studio Warning: O parâmetro
V3117 Constructor '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; }
Um bom exemplo da diferença entre a operação do analisador e a revisão manual do código. Treze argumentos construtores, um não utilizado. De fato, o Visual Studio também observa um argumento não utilizado, mas no terceiro nível de avisos (eles geralmente são desativados). Nesse caso, este é um erro claro, porque a classe também possui treze propriedades para cada argumento e nenhum valor é atribuído em nenhum lugar em
Telefones . A edição é óbvia, não vou trazê-la.
PVS-Studio Warning: 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 considerou perigoso desreferenciar o resultado da chamada de
CreateContainer .
Dê 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 analisador pode ver a atribuição de
null a uma variável, mesmo ao passar um valor por uma cadeia de cinquenta métodos. Mas ele não pode dizer se a execução continuará nesse segmento pelo menos uma vez. Sim, e eu também não poderia, em geral ... Chamadas de método são perdidas entre métodos substituídos e virtuais. Então, sugiro apenas estar seguro com uma verificação adicional:
protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return null; tabItem.ParentTabControl = Owner; .... }
Aviso do PVS-Studio: V3142 Código inacessível detectado. É possível que haja um erro. DevTools.xaml.cs 91
Não vou escrever muito código aqui para criar intrigas, direi imediatamente: o aviso é falso. O analisador viu uma chamada para um método que lança uma exceção incondicional. Aqui está:
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"); }
Era impossível não prestar atenção aos trinta e cinco avisos (!) Sobre o código inacessível localizado após as chamadas para esse método. Perguntei a um dos desenvolvedores do projeto: como ele funciona? E eles me falaram sobre uma maneira de substituir chamadas de um método por chamadas de outros usando a biblioteca
Mono.Cecil . Permite substituir chamadas diretamente no código IL.
O analisador não suporta esta biblioteca; portanto, temos muitos falsos positivos; portanto, é melhor desativar esse diagnóstico neste projeto. É um pouco embaraçoso admitir que fui eu quem fez esse diagnóstico ... mas, como qualquer ferramenta, a análise estática precisa ser configurada.
Por exemplo, atualmente estamos desenvolvendo diagnósticos sobre conversão de tipo não segura. E oferece pouco menos de mil operações no projeto do jogo, nas quais o controle de digitação é realizado no lado do motor.
PVS-Studio Warning: 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 sempre retorna
verdadeiro . Talvez o objetivo do método tenha mudado desde que a assinatura foi gravada, mas provavelmente isso é um erro. Essa também é a classe de controle copiada da Microsoft, a julgar pelo comentário no início da classe. Na minha opinião, o
DataGrid é geralmente um dos controles mais instáveis e parece-me que vale a pena considerar: é necessário confirmar a rolagem se não atender às condições?
Conclusão
Alguns desses erros foram introduzidos no projeto não pelos próprios desenvolvedores da UI da Avalonia, mas pelo código copiado dos controles do WPF. No entanto, para o usuário da interface, a fonte do erro geralmente não desempenha um papel. Uma interface com falha ou com defeito estraga a opinião de todo o programa.
No artigo que mencionei a necessidade de configurar o analisador, há falsos positivos que são inevitáveis devido aos princípios de operação dos algoritmos de análise estática. Qualquer pessoa familiarizada com o
problema de parar está ciente das limitações matemáticas ao trabalhar com outro código. Mas, neste caso, estamos falando em desativar um diagnóstico em quase cento e meio. Portanto, não estamos falando sobre a perda de significado na análise estática (ou a questão não vale a pena). Além disso, esse diagnóstico pode ter tido boas respostas, é apenas mais difícil encontrar a massa de falsos positivos.
Observe a alta qualidade do código do projeto! Espero que os desenvolvedores mantenham o ritmo e o nível de qualidade do código. Infelizmente, quanto maior o projeto, mais bugs nele. Uma das maneiras possíveis de reduzir erros é configurar o CI \ CD corretamente, com a conexão de análises estáticas e dinâmicas. E se você deseja simplificar o trabalho com grandes projetos e reduzir a quantidade de tempo necessária para a depuração, faça o
download e experimente o PVS-Studio!
Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Alexander Senichkin.
Nossa pequena contribuição para a luta por menos plataformas da Avalonia UI .