👩🏼‍💻 🙀 🕔 Microsoft Open XML SDK-Codequalitätsstudie 🦄 ✌🏽 🍪

Meine Bekanntschaft mit dem Open XML SDK begann, als ich eine Bibliothek zum Erstellen von Word-Dokumenten mit einigen Berichten benötigte. Nachdem ich über 7 Jahre mit der Word-API gearbeitet hatte, wollte ich etwas Neues und Bequemeres ausprobieren. So fand ich heraus, dass Microsoft eine alternative Lösung hat. Traditionell überprüfen wir Programme und Bibliotheken, die im Unternehmen verwendet werden, mit dem PVS-Studio-Analysegerät.

Einführung

Office Open XML, auch als OpenXML oder OOXML bezeichnet, ist ein XML-basiertes Format für Office-Dokumente, einschließlich Textverarbeitungsdokumenten, Tabellenkalkulationen, Präsentationen und Diagrammen, Formen und anderen Grafiken. Die Spezifikation wurde von Microsoft entwickelt und 2006 von ECMA International übernommen. Im Juni 2014 veröffentlichte Microsoft das Open XML SDK in Open Source. Die Quelle ist jetzt auf GitHub unter der MIT-Lizenz verfügbar .

Um Fehler im Quellcode der Bibliothek zu finden, haben wir PVS-Studio verwendet . Es ist ein Tool zum Erkennen von Fehlern und potenziellen Schwachstellen im Quellcode von Programmen, die in C, C ++, C # und Java geschrieben wurden. Funktioniert in 64-Bit-Systemen unter Windows, Linux und MacOS.

Das Projekt ist klein genug und es gab nur wenige Warnungen. Die Wahl des Titelbildes basierte jedoch genau auf den Ergebnissen. Der Code enthält viele nutzlose bedingte Operatoren. Es scheint mir, dass, wenn Sie alle diese Stellen im Code umgestalten, die Lautstärke merklich reduziert wird. Infolgedessen erhöht sich auch die Lesbarkeit des Codes.

Warum Word API und nicht Open XML SDK

Wie Sie vielleicht aus dem Titel erraten haben, habe ich weiterhin die Word-API verwendet. Diese Methode hat viele Nachteile:

Alte umständliche API;
Microsoft Office muss installiert sein.
Die Notwendigkeit, ein Distributionskit mit Office-Bibliotheken zu verteilen;
Abhängigkeit der Word-API von den Einstellungen des Systemgebietsschemas;
Niedrige Arbeitsgeschwindigkeit.

Mit dem Gebietsschema im Allgemeinen ereignete sich ein amüsanter Vorfall. Windows hat ein Dutzend regionale Einstellungen. Auf einem der Servercomputer gab es ein Durcheinander aus den USA und Großbritannien. Aus diesem Grund wurden Word-Dokumente erstellt, in denen anstelle des Dollarsymbols ein Rubel vorhanden war und Pfund überhaupt nicht angezeigt wurden. Durch das Verbessern der Betriebssystemeinstellungen wurde das Problem behoben.

Als ich das alles auflistete, fragte ich mich noch einmal, warum ich es immer noch benutze ...

Aber nein, ich mag die Word-API immer noch besser und hier ist der Grund dafür.

OOXML sieht folgendermaßen aus:

<?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>

Wobei <w: r> (Wortlauf) kein Satz und nicht einmal ein Wort ist, sondern ein Textstück mit Attributen, die sich von den benachbarten Textfragmenten unterscheiden.

Es ist mit so etwas programmiert:

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

Das Dokument hat eine bestimmte interne Struktur, und im Code müssen Sie dieselben Elemente erstellen. Das Open XML SDK verfügt meiner Meinung nach nicht über genügend abstrakte Datenzugriffsschicht. Das Erstellen eines Dokuments mit der Word-API wird klarer und kürzer. Besonders wenn es um Tabellen und andere komplexe Datenstrukturen geht.

Das Open XML SDK löst wiederum eine Vielzahl von Aufgaben. Damit können Sie Dokumente nicht nur für Word, sondern auch für Excel und PowerPoint erstellen. Diese Bibliothek ist wahrscheinlich für einige Aufgaben besser geeignet, aber ich habe mich entschlossen, vorerst auf der Word-API zu bleiben. In jedem Fall wird es nicht möglich sein, es vollständig aufzugeben, weil Für interne Anforderungen entwickeln wir ein Plugin für Word. Dort kann nur die Word-API verwendet werden.

Zwei Werte für Zeichenfolge

V3008 Der Variablen '_rawOuterXml' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 164, 161. OpenXmlElement.cs 164

internal string RawOuterXml
{
    get => _rawOuterXml;

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

        _rawOuterXml = value;
    }
}

Der Zeichenfolgentyp kann zwei Arten von Werten haben: Null und Textwert. Es ist definitiv sicherer, Textbedeutung zu verwenden, aber beide Ansätze sind gültig. In diesem Projekt ist es nicht akzeptabel, den Nullwert zu verwenden , und er wird in string umgeschrieben. Leer ... zumindest war das so beabsichtigt. Aufgrund eines Fehlers in RawOuterXml können Sie jedoch weiterhin null schreiben und dann auf dieses Feld verweisen und eine NullReferenceException erhalten .

V3022 Der Ausdruck 'namespaceUri! = Null' ist immer wahr. 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)  // <=
        {
            ....
        }
        ....
    }
    ....
}

Dieses Snippet verwendet den gleichen Ansatz, der Autor des Codes hat keinen größeren Fehler gemacht, aber es riecht immer noch nach einem fehlgeschlagenen Refactoring. Höchstwahrscheinlich kann hier eine Prüfung entfernt werden, wodurch die Breite des Codes verringert und daher seine Lesbarkeit erhöht wird.

Über die Kompaktheit des Codes

V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von ".xml" zurückgibt. 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";
    }
}

Ich weiß nicht, ob es hier einen Tippfehler gibt oder ob der Autor des Codes seiner Meinung nach "netten" Code geschrieben hat. Ich bin sicher, dass es keinen Sinn macht, so viele Werte desselben Typs von einer Funktion zurückzugeben, und der Code kann stark reduziert werden.

Dies ist nicht der einzige Ort dieser Art. Hier sind noch ein paar dieser Warnungen:

V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von ".xml" zurückgibt. CustomPropertyPartTypeInfo.cs 25
V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von '".bin"' zurückgibt. EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22

Es wäre interessant zu hören, warum man so schreibt.

V3139 Zwei oder mehr Fallzweige führen dieselben Aktionen aus. 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;
        ....
    }
    ....
}

Es gibt weniger Fragen zu diesem Code. Höchstwahrscheinlich können identische Fälle kombiniert werden, und der Code wird kürzer und offensichtlicher.

Noch ein paar solche Orte:

V3139 Zwei oder mehr Fallzweige führen dieselben Aktionen aus. OpenXmlMiscNode.cs 312
V3139 Zwei oder mehr Fallzweige führen dieselben Aktionen aus. CustomPropertyPartTypeInfo.cs 30
V3139 Zwei oder mehr Fallzweige führen dieselben Aktionen aus. CustomXmlPartTypeInfo.cs 15
V3139 Zwei oder mehr Fallzweige führen dieselben Aktionen aus. OpenXmlElement.cs 1803

Die immer wahr / falsch

Jetzt ist es an der Zeit, einige Codebeispiele bereitzustellen, die meine Wahl des Titelbilds bestimmt haben.

Warnung 1

V3022 Der Ausdruck 'Complete ()' ist immer falsch. 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;
}

Die IsComplete- Eigenschaft wird zweimal verwendet, und aus dem Code ist leicht ersichtlich , dass sich ihr Wert nicht ändert. Am Ende der Funktion können Sie also einfach den zweiten Wert des ternären Operators zurückgeben - true .

Warnung 2

V3022 Der Ausdruck '_elementStack.Count> 0' ist immer wahr. 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;
    }
    ....
}

Wenn der _elementStack keine 0 Elemente enthält , gibt es natürlich mehr davon. Der Code kann um mindestens 8 Zeilen gekürzt werden.

Warnung 3

V3022 Der Ausdruck 'rootElement == null' ist immer falsch. 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(....);
  }
  ....
}

Die CreateElement- Funktion kann nicht null zurückgeben . Wenn das Unternehmen eine Regel zum Schreiben von Methoden zum Erstellen von XML-Knoten festgelegt hat, die entweder ein gültiges Objekt zurückgeben oder eine Ausnahme auslösen, können Benutzer solcher Funktionen keine zusätzlichen Überprüfungen missbrauchen.

Warnung 4

V3022 Der Ausdruck 'nameProvider' ist immer nicht null. Der Operator '?' ist übertrieben. 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();
}

Der Operator is hat das folgende Muster:

expr is type varname

Wenn expr als true ausgewertet wird , ist das varname- Objekt gültig und muss nicht erneut mit null verglichen werden , wie in diesem Code-Snippet beschrieben.

Warnung 5

V3022 Ausdruck 'Erweiterung == ".xlsx" || extension == ".xlsm" 'ist immer falsch. 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;
        }
        ....
    }
    ....
}

Es stellte sich ein interessanter Code heraus. Der Erstautor hat alle Dokumente mit den folgenden Erweiterungen ausgesondert : Nicht .pptx , .pptm ,. Potx und. potm und beschloss dann zu überprüfen, ob sich keine .xlsx und .xlsm unter ihnen befanden . Die PresentationDocument- Funktion ist definitiv ein Opfer von Refactoring.

Warnung 7

V3022 Der Ausdruck 'OpenSettings.MarkupCompatibilityProcessSettings == null' ist immer falsch. 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;
        }
    }
}

Die MarkupCompatibilityProcessSettings- Eigenschaft gibt niemals null zurück . Wenn sich im Getter herausstellt, dass das Klassenfeld null ist , wird das Objekt mit einem neuen überschrieben. Beachten Sie außerdem, dass dies kein rekursiver Aufruf einer Eigenschaft ist, sondern gleichnamige Eigenschaften aus verschiedenen Klassen. Vielleicht hat einige Verwirrung dazu geführt, dass unnötige Schecks ausgestellt wurden.

Andere Warnungen

Warnung 1

V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Überprüfung des vorherigen Geschwisters. 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;    // <=
    ....
}

Und hier ist ein Beispiel, bei dem eine zusätzliche Überprüfung einfach nicht ausreicht. Die PreviousSibling- Methode kann null zurückgeben , und das Ergebnis dieser Funktion wird sofort ohne Überprüfung verwendet.

2 weitere gefährliche Orte:

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie 'prevNode'. OpenXmlCompositeElement.cs 489
V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie 'prevNode'. OpenXmlCompositeElement.cs 497

Warnung 2

V3093 Der Operator '&' wertet beide Operanden aus. Vielleicht sollte stattdessen ein Kurzschlussoperator '&&' verwendet werden. 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;
            }
        }
    }
    ....
}

Einige Leute wenden den Operator '&' gerne auf logische Ausdrücke an, wo sie nicht sollten. Bei diesem Operator wird der zweite Operand unabhängig vom Ergebnis des ersten zuerst ausgewertet. Dies ist hier kein sehr schwerwiegender Fehler, aber ein derart schlampiger Code nach dem Refactoring kann zu potenziellen NullReferenceException- Ausnahmen führen .

Warnung 3

V3097 Mögliche Ausnahme: Der mit [Serializable] gekennzeichnete Typ enthält nicht serialisierbare Elemente, die nicht mit [NonSerialized] gekennzeichnet sind. 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; } // <=
}

Die Serialisierung der OpenXmlPackageValidationEventArgs- Klasse schlägt möglicherweise fehl, da vergessen wurde, dass eine der Eigenschaften als nicht serialisierbar markiert wurde. Oder Sie müssen den Rückgabetyp dieser Eigenschaft so ändern, dass er serialisierbar ist. Andernfalls kann zur Laufzeit eine Ausnahme auftreten.

Fazit

Wir im Unternehmen lieben Microsoft-Projekte und -Technologien. In dem Abschnitt, in dem wir Open Source-Projekte auflisten, die mit PVS-Studio getestet wurden , haben wir sogar einen separaten Abschnitt für Microsoft zugewiesen. Es wurden bereits 21 Projekte akkumuliert, über die 26 Artikel geschrieben wurden. Dies ist der 27 ..

Ich bin sicher, Sie fragen sich, ob Microsoft zu unseren Kunden gehört. Die Antwort ist ja! Aber vergessen wir nicht, dass dies ein riesiges Unternehmen ist, das weltweit führend ist. Es gibt definitiv Abteilungen, die PVS-Studio bereits in ihren Projekten verwenden, aber es gibt noch mehr, die diese nicht verwenden! Und unsere Erfahrung mit Open Source-Projekten zeigt, dass ihnen eindeutig ein gutes Werkzeug zum Auffinden von Fehlern fehlt;).

Weitere Neuigkeiten aus den Nachrichten für diejenigen, die an der Analyse von Code in C ++, C # und Java interessiert sind: Wir haben kürzlich die Unterstützung für den OWASP- Standard hinzugefügt und erhöhen aktiv dessen Abdeckung.

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Svyatoslav Razmyslov. Analyse der Codequalität des Open XML SDK von Microsoft .

Microsoft Open XML SDK-Codequalitätsstudie