👩🏿‍💼 📙 🙎🏼 Estudo de qualidade de código Microsoft Open XML SDK 🤝 🔄 🎁

Meu conhecimento do Open XML SDK começou quando eu precisava de uma biblioteca para criar documentos do Word com alguns relatórios. Depois de trabalhar com a API do Word por mais de 7 anos, eu queria tentar algo novo e mais conveniente. Foi assim que descobri que a Microsoft tem uma solução alternativa. Tradicionalmente, nós pré-verificamos programas e bibliotecas usados na empresa usando o analisador PVS-Studio.

Introdução

Office Open XML, também conhecido como OpenXML ou OOXML, é um formato baseado em XML para documentos de escritório, incluindo documentos de processamento de texto, planilhas, apresentações e diagramas, formas e outros gráficos. A especificação foi desenvolvida pela Microsoft e adotada pela ECMA International em 2006. Em junho de 2014, a Microsoft lançou o Open XML SDK em código aberto. A fonte agora está disponível no GitHub sob a licença do MIT.

Para encontrar erros no código-fonte da biblioteca, usamos PVS-Studio . É uma ferramenta para detectar bugs e vulnerabilidades potenciais no código-fonte de programas escritos em C, C ++, C # e Java. Funciona em sistemas de 64 bits no Windows, Linux e macOS.

O projeto é pequeno o suficiente e houve poucos avisos. Mas a escolha da imagem do título foi baseada justamente nos resultados. Existem muitos operadores condicionais inúteis no código. Parece-me que se você refatorar todos esses lugares no código, o volume será visivelmente reduzido. Como resultado, a legibilidade do código também aumentará.

Por que a API do Word e não o Open XML SDK

Como você deve ter adivinhado pelo título, continuei a usar a API do Word. Este método tem muitas desvantagens:

API desajeitada antiga;
O Microsoft Office deve estar instalado;
A necessidade de distribuir um kit de distribuição com bibliotecas Office;
Dependência da API do Word nas configurações de localidade do sistema;
Baixa velocidade de trabalho.

Com o local em geral, aconteceu um incidente divertido. O Windows tem uma dúzia de configurações regionais. Em um dos computadores servidores, havia uma confusão com as localidades dos EUA e do Reino Unido. Por causa disso, foram criados documentos do Word, onde em vez do símbolo do dólar havia um rublo e as libras não eram exibidas. Melhorar as configurações do sistema operacional corrigiu o problema.

Listando tudo isso, me perguntei de novo por que ainda a uso ...

Mas não, ainda gosto mais da API do Word e aqui está o porquê.

OOXML se parece com isto:

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<w:document ....>
  <w:body>
    <w:p w:rsidR="00E22EB6"
         w:rsidRDefault="00E22EB6">
      <w:r>
        <w:t>This is a paragraph.</w:t>
      </w:r>
    </w:p>
    <w:p w:rsidR="00E22EB6"
         w:rsidRDefault="00E22EB6">
      <w:r>
        <w:t>This is another paragraph.</w:t>
      </w:r>
    </w:p>
  </w:body>
</w:document>

Onde <w: r> (Word Run) não é uma frase, nem mesmo uma palavra, mas qualquer pedaço de texto que tenha atributos diferentes dos fragmentos de texto adjacentes.

É programado com algo assim:

Paragraph para = body.AppendChild(new Paragraph());
Run run = para.AppendChild(new Run());
run.AppendChild(new Text(txt));

O documento possui uma estrutura interna específica, e no código você precisa criar os mesmos elementos. O Open XML SDK, em minha opinião, não tem camada de acesso a dados abstratos suficiente. A criação de um documento usando a API do Word será mais clara e curta. Especialmente quando se trata de tabelas e outras estruturas de dados complexas.

Por sua vez, o Open XML SDK resolve uma ampla gama de tarefas. Com ele, você pode criar documentos não só para Word, mas também para Excel e PowerPoint. Esta biblioteca provavelmente é mais adequada para algumas tarefas, mas decidi ficar na API do Word por enquanto. Em qualquer caso, não será possível abandoná-lo completamente, porque para necessidades internas, estamos desenvolvendo um plugin para o Word, onde é possível utilizar apenas a API do Word.

Dois valores para string

V3008 A variável '_rawOuterXml' recebe valores duas vezes sucessivamente. Talvez seja um engano. Verifique as linhas: 164, 161. OpenXmlElement.cs 164

internal string RawOuterXml
{
    get => _rawOuterXml;

    set
    {
        if (string.IsNullOrEmpty(value))
        {
            _rawOuterXml = string.Empty;
        }

        _rawOuterXml = value;
    }
}

O tipo de string pode ter 2 tipos de valores: valor nulo e valor de texto. Usar o significado textual é definitivamente mais seguro, mas ambas as abordagens são válidas. Neste projeto , é inaceitável usar o valor nulo e é reescrito para string.Empty ... pelo menos é assim que se pretendia. Porém, devido a um erro em RawOuterXml , você ainda pode escrever null e, em seguida, referir-se a este campo, recebendo uma NullReferenceException .

A expressão V3022 'namespaceUri! = Null' é sempre verdadeira. OpenXmlElement.cs 497

public OpenXmlAttribute GetAttribute(string localName, string namespaceUri)
{
    ....
    if (namespaceUri == null)
    {
        // treat null string as empty.
        namespaceUri = string.Empty;
    }
    ....
    if (HasAttributes)
    {
        if (namespaceUri != null)  // <=
        {
            ....
        }
        ....
    }
    ....
}

Este trecho usa a mesma abordagem, o autor do código não cometeu nenhum erro grave, mas ainda cheira a uma refatoração falhada. Provavelmente, uma verificação pode ser removida aqui, o que reduzirá a largura do código e, portanto, aumentará sua legibilidade.

Sobre a compactação do código

V3009 É estranho que este método sempre retorne um e o mesmo valor de '".xml"'. CustomXmlPartTypeInfo.cs 31

internal static string GetTargetExtension(CustomXmlPartType partType)
{
    switch (partType)
    {
        case CustomXmlPartType.AdditionalCharacteristics:
            return ".xml";

        case CustomXmlPartType.Bibliography:
            return ".xml";

        case CustomXmlPartType.CustomXml:
            return ".xml";

        case CustomXmlPartType.InkContent:
            return ".xml";

        default:
            return ".xml";
    }
}

Não sei se há algum erro de digitação aqui, ou se o autor do código escreveu um código "legal" em sua opinião. Tenho certeza de que não faz sentido retornar tantos valores do mesmo tipo de uma função, e o código pode ser bastante reduzido.

Este não é o único lugar. Aqui estão mais alguns desses avisos:

V3009 É estranho que este método sempre retorne um e o mesmo valor de '".xml"'. CustomPropertyPartTypeInfo.cs 25
V3009 É estranho que este método sempre retorne um e o mesmo valor de '".bin"'. EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22

Seria interessante ouvir por que escrever assim.

V3139 Duas ou mais ramificações de caso executam as mesmas ações. OpenXmlPartReader.cs 560

private void InnerSkip()
{
    Debug.Assert(_xmlReader != null);

    switch (_elementState)
    {
        case ElementState.Null:
            ThrowIfNull();
            break;

        case ElementState.EOF:
            return;

        case ElementState.Start:
            _xmlReader.Skip();
            _elementStack.Pop();
            GetElementInformation();
            return;

        case ElementState.End:
        case ElementState.MiscNode:
            // cursor is end element, pop stack
            _xmlReader.Skip();
            _elementStack.Pop();
            GetElementInformation();
            return;
        ....
    }
    ....
}

Existem menos perguntas sobre este código. Muito provavelmente, casos idênticos podem ser combinados e o código se tornará mais curto e mais óbvio.

Mais alguns desses lugares:

V3139 Duas ou mais ramificações de caso executam as mesmas ações. OpenXmlMiscNode.cs 312
V3139 Duas ou mais ramificações de caso executam as mesmas ações. CustomPropertyPartTypeInfo.cs 30
V3139 Duas ou mais ramificações de caso executam as mesmas ações. CustomXmlPartTypeInfo.cs 15
V3139 Duas ou mais ramificações de caso executam as mesmas ações. OpenXmlElement.cs 1803

Aqueles sempre verdadeiros / falsos

Agora é a hora de fornecer alguns exemplos de código que determinaram minha escolha da imagem do título.

Aviso 1

A expressão V3022 'Complete ()' é sempre falsa. ParticleCollection.cs 243

private bool IsComplete => Current is null ||
                           Current == _collection._element.FirstChild;

public bool MoveNext()
{
    ....
    if (IsComplete)
    {
        return Complete();
    }

    if (....)
    {
        return Complete();
    }

    return IsComplete ? Complete() : true;
}

A propriedade IsComplete é usada 2 vezes e é fácil entender pelo código que seu valor não mudará. Assim, no final da função, você pode simplesmente retornar o segundo valor do operador ternário - verdadeiro .

Aviso 2

A expressão V3022 '_elementStack.Count> 0' é sempre verdadeira. OpenXmlDomReader.cs 501

private readonly Stack<OpenXmlElement> _elementStack;

private bool MoveToNextSibling()
{
    ....
    if (_elementStack.Count == 0)
    {
        _elementState = ElementState.EOF;
        return false;
    }
    ....
    if (_elementStack.Count > 0) // <=
    {
        _elementState = ElementState.End;
    }
    else
    {
        // no more element, EOF
        _elementState = ElementState.EOF;
    }
    ....
}

Obviamente, se não houver 0 elementos em _elementStack , haverá mais deles. O código pode ser reduzido em pelo menos 8 linhas.

Aviso 3

A expressão V3022 'rootElement == null' é sempre falsa. OpenXmlPartReader.cs 746

private static OpenXmlElement CreateElement(string namespaceUri, string name)
{
    if (string.IsNullOrEmpty(name))
    {
        throw new ArgumentException(....);
    }

    if (NamespaceIdMap.TryGetNamespaceId(namespaceUri, out byte nsId)
        && ElementLookup.Parts.Create(nsId, name) is OpenXmlElement element)
    {
        return element;
    }

    return new OpenXmlUnknownElement();
}

private bool ReadRoot()
{
  ....
  var rootElement = CreateElement(....);

  if (rootElement == null) // <=
  {
      throw new InvalidDataException(....);
  }
  ....
}

A função CreateElement não pode retornar nulo . Se a empresa fez uma regra para escrever métodos para criar xml-nodes que retornam um objeto válido ou lançam uma exceção, os usuários de tais funções não precisam abusar de verificações adicionais.

Aviso 4

A expressão V3022 'nameProvider' sempre não é nula. O operador '?.' é excessivo. OpenXmlSimpleTypeExtensions.cs 50

public static XmlQualifiedName GetSimpleTypeQualifiedName(....)
{
    foreach (var validator in validators)
    {
        if (validator is INameProvider nameProvider &&
            nameProvider?.QName is XmlQualifiedName qname) // <=
        {
            return qname;
        }
    }

    return type.GetSimpleTypeQualifiedName();
}

O operador is tem o seguinte padrão:

expr is type varname

Se expr for avaliado como true , então o objeto varname será válido e não precisa ser comparado com null novamente , como é feito neste trecho de código.

Aviso 5

V3022 Expression 'extension == ".xlsx" || extension == ".xlsm" 'é sempre falso. PresentationDocument.cs 246

public static PresentationDocument CreateFromTemplate(string path)
{
    ....
    string extension = Path.GetExtension(path);
    if (extension != ".pptx" && extension != ".pptm" &&
        extension != ".potx" && extension != ".potm")
    {
        throw new ArgumentException("...." + path, nameof(path));
    }

    using (PresentationDocument template = PresentationDocument.Open(....)
    {
        PresentationDocument document = (PresentationDocument)template.Clone();

        if (extension == ".xlsx" || extension == ".xlsm")
        {
            return document;
        }
        ....
    }
    ....
}

Um código interessante foi descoberto. Primeiro autor eliminar todos os documentos com as seguintes extensões não são .pptx , .pptm ,. potx e. potm e, em seguida, decidiu verificar se não havia .xlsx e .xlsm entre eles . A função PresentationDocument é definitivamente uma vítima de refatoração.

Aviso 7

A expressão V3022 'OpenSettings.MarkupCompatibilityProcessSettings == null' é sempre falsa. OpenXmlPackage.cs 661

public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
    get
    {
        if (_mcSettings is null)
        {
            _mcSettings = new MarkupCompatibilityProcessSettings(....);
        }

        return _mcSettings;
    }

    set
    {
        _mcSettings = value;
    }
}

public MarkupCompatibilityProcessSettings MarkupCompatibilityProcessSettings
{
    get
    {
        if (OpenSettings.MarkupCompatibilityProcessSettings == null) // <=
        {
            return new MarkupCompatibilityProcessSettings(....);
        }
        else
        {
            return OpenSettings.MarkupCompatibilityProcessSettings;
        }
    }
}

A propriedade MarkupCompatibilityProcessSettings nunca retorna nulo . Se no getter o campo da classe for nulo , o objeto será sobrescrito por um novo. Além disso, observe que esta não é uma chamada recursiva para uma propriedade, mas propriedades com o mesmo nome de classes diferentes. Talvez alguma confusão tenha levado à redação de cheques desnecessários.

Outros avisos

Aviso 1

V3080 Possível desreferência nula. Considere inspecionar 'previousSibling'. OpenXmlCompositeElement.cs 380

public OpenXmlElement PreviousSibling()
{
    if (!(Parent is OpenXmlCompositeElement parent))
    {
        return null;
    }
    ....
}

public override T InsertBefore<T>(T newChild, OpenXmlElement referenceChild)
{
    ....
    OpenXmlElement previousSibling = nextNode.PreviousSibling();
    prevNode.Next = nextNode;
    previousSibling.Next = prevNode;    // <=
    ....
}

E aqui está um exemplo em que a verificação adicional não é suficiente. O método PreviousSibling pode retornar nulo e o resultado desta função é usado imediatamente sem verificação.

Mais 2 lugares perigosos:

V3080 Possível desreferência nula. Considere inspecionar 'prevNode'. OpenXmlCompositeElement.cs 489
V3080 Possível desreferência nula. Considere inspecionar 'prevNode'. OpenXmlCompositeElement.cs 497

Aviso 2

V3093 O operador '&' avalia ambos os operandos. Talvez um operador de curto-circuito '&&' deva ser usado em seu lugar. UniqueAttributeValueConstraint.cs 60

public override ValidationErrorInfo ValidateCore(ValidationContext context)
{
    ....
    foreach (var e in root.Descendants(....))
    {
        if (e != element & e.GetType() == elementType) // <=
        {
            var eValue = e.ParsedState.Attributes[_attribute];

            if (eValue.HasValue && _comparer.Equals(....))
            {
                return true;
            }
        }
    }
    ....
}

Algumas pessoas gostam de usar o operador '&' em expressões lógicas onde não deveriam. No caso desse operador, o segundo operando é avaliado primeiro, independentemente do resultado do primeiro. Este não é um erro muito sério aqui, mas esse código desleixado após a refatoração pode levar a possíveis exceções NullReferenceException .

Aviso 3

V3097 Possível exceção: o tipo marcado por [Serializable] contém membros não serializáveis não marcados por [NonSerialized]. OpenXmlPackageValidationEventArgs.cs 15

[Serializable]
[Obsolete(ObsoleteAttributeMessages.ObsoleteV1ValidationFunctionality, false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public sealed class OpenXmlPackageValidationEventArgs : EventArgs
{
    private string _message;

    [NonSerialized]
    private readonly object _sender;

    [NonSerialized]
    private OpenXmlPart _subPart;

    [NonSerialized]
    private OpenXmlPart _part;

    ....

    internal DataPartReferenceRelationship
        DataPartReferenceRelationship { get; set; } // <=
}

A serialização da classe OpenXmlPackageValidationEventArgs pode falhar porque uma das propriedades foi esquecida de ser marcada como não serializável. Ou você precisa modificar o tipo de retorno desta propriedade para ser serializável, caso contrário, uma exceção pode ocorrer em tempo de execução.

Conclusão

Nós, na empresa, amamos os projetos e tecnologias da Microsoft. Na seção onde listamos os projetos Open Source testados com PVS-Studio, até alocamos uma seção separada para a Microsoft. Já são 21 projetos acumulados, sobre os quais já foram escritos 26 artigos. Este é o dia 27.

Tenho certeza de que você está se perguntando se existe a Microsoft entre nossos clientes. A resposta é sim! Mas não esqueçamos que esta é uma grande corporação líder em desenvolvimento em todo o mundo. Definitivamente existem divisões que já usam o PVS-Studio em seus projetos, mas há ainda mais daquelas que não usam! E nossa experiência com projetos de código aberto mostra que eles claramente carecem de uma boa ferramenta para encontrar bugs;).

Mais notícias da notícia para os interessados em analisar código em C ++, C # e Java: recentemente adicionamos suporte para o padrão OWASP e estamos aumentando ativamente sua cobertura.

Se você deseja compartilhar este artigo com um público que fala inglês, use o link de tradução: Svyatoslav Razmyslov. Analisando a qualidade do código do SDK de XML aberto da Microsoft .

Estudo de qualidade de código Microsoft Open XML SDK