Microsoft Open XML SDK代码质量研究

当我需要一个用于创建带有一些报告的Word文档的库时，我就开始接触Open XML SDK。在使用Word API超过7年之后，我想尝试一些新颖且更方便的方法。这就是我发现Microsoft有替代解决方案的方式。按照传统，我们使用PVS-Studio分析仪预先检查公司中使用的程序和库。

介绍

Office Open XML，也称为OpenXML或OOXML，是用于办公室文档的基于XML的格式，包括文字处理文档，电子表格，演示文稿以及图表，形状和其他图形。该规范由Microsoft开发，并于2006年被ECMA International采用。 2014年6月，Microsoft以开源形式发布了Open XML SDK。现在，该源代码已在MIT许可下在GitHub上提供。



为了找到库源代码中的错误，我们使用了PVS-Studio。它是用于识别用C，C ++，C＃和Java编写的程序的源代码中的错误和潜在漏洞的工具。在Windows，Linux和macOS上的64位系统上运行。



该项目足够小，几乎没有警告。但是标题图片的选择正是基于结果。代码中有很多无用的条件运算符。在我看来，如果您重构代码中的所有此类位置，那么体积将明显减少。结果，代码的可读性也将提高。

为什么使用Word API而不使用Open XML SDK

正如您可能从标题中猜到的那样，我继续使用Word API。此方法有很多缺点：

• 旧的笨拙的API；
• 必须安装Microsoft Office；
• 需要分发带有Office库的分发工具包；
• Word API对系统区域设置的依赖性；
• 工作速度低。

总体而言，发生了一件有趣的事件。Windows有十几个区域设置。在其中一台服务器计算机上，有来自美国和英国语言环境的乱七八糟的东西。因此，创建了Word文档，在其中用卢布代替美元符号，而根本不显示英镑。改进操作系统设置可以解决该问题。



列出所有这些，我再次想知道为什么我仍然使用它...



但是，不，我仍然更喜欢Word API，这就是原因。



OOXML看起来像这样：

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

其中<w：r>（单词运行）不是句子，甚至不是单词，而是具有与相邻文本片段不同的属性的任何文本。



它的编程如下：

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

该文档具有特定的内部结构，并且需要在代码中创建相同的元素。在我看来，Open XML SDK没有足够的抽象数据访问层。使用Word API创建文档将更加清晰和简短。特别是涉及表格和其他复杂数据结构时。



反过来，Open XML SDK解决了许多任务。使用它，您不仅可以为Word创建文档，还可以为Excel和PowerPoint创建文档。该库可能更适合于某些任务，但我决定暂时保留Word API。在任何情况下，都不可能完全放弃它，因为 为了满足内部需求，我们正在开发Word插件，并且可以仅使用Word API。

字符串的两个值

V3008'_rawOuterXml '变量被连续分配两次值。也许这是一个错误。检查行：164，161。OpenXmlElement.cs 164

internal string RawOuterXml
{
    get => _rawOuterXml;

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

        _rawOuterXml = value;
    }
}

字符串 类型可以有两种类型的值：null和文本值。使用文本含义绝对更安全，但是两种方法都是有效的。在此项目中，使用null值是不可接受的，并且将其重写为string.Empty ...至少是这样做的目的。但是由于RawOuterXml中的错误，您仍然可以编写null，然后引用此字段，从而收到NullReferenceException。



V3022表达式'namespaceUri！= Null'始终为true。 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)  // <=
        {
            ....
        }
        ....
    }
    ....
}

此代码段使用相同的方法，代码的编写者没有犯任何重大错误，但是仍然闻到不良的重构。最有可能的是，可以在此处删除一项检查，这将减少代码的宽度，并因此增加其可读性。

关于代码的紧凑性

V3009奇怪的是，此方法总是返回一个相同的值“ .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";
    }
}

我不知道这里是否有错字，或者代码的作者是否认为自己写的是“好”的代码。我确信从一个函数中返回这么多相同类型的值是没有意义的，并且代码可以大大减少。



这不是唯一的地方。以下是其中一些警告：

• V3009奇怪的是，此方法始终返回一个相同的值“ .xml”。CustomPropertyPartTypeInfo.cs 25
• V3009奇怪的是，此方法始终返回一个相同的值“ .bin”。EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22

听到为什么这样写会很有趣。



V3139两个或更多案例分支执行相同的操作。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;
        ....
    }
    ....
}

关于此代码的问题较少。很有可能，相同的情况可以合并，并且代码将变得更短，更明显。



还有一些这样的地方：

• V3139两个或更多分支机构执行相同的操作。OpenXmlMiscNode.cs 312
• V3139两个或更多案例分支执行相同的操作。CustomPropertyPartTypeInfo.cs 30
• V3139两个或更多分支机构执行相同的操作。CustomXmlPartTypeInfo.cs 15
• V3139两个或更多分支机构执行相同的操作。OpenXmlElement.cs 1803

那些总是对/错

现在该提供一些确定我选择标题图像的代码示例了。



警告1



V3022表达式'Complete（）'始终为false。粒子收集.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;
}

IsComplete 属性使用了2次，从代码中很容易理解它的值不会改变。因此，在函数的末尾，你可以简单地返回三元运算符的第二个值-真实。



警告2



V3022表达式'_elementStack.Count> 0'始终为true。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;
    }
    ....
}

显然，如果_elementStack中不存在0个元素，则它们将更多。该代码至少可以缩短8行。



警告3



V3022表达式'rootElement == null'始终为false。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(....);
  }
  ....
}

CreateElement 函数不能返回null。如果公司制定了规则来编写用于创建返回有效对象或引发异常的xml节点的方法，则此类函数的用户将无法滥用其他检查。



警告4



V3022表达式'nameProvider'始终不为null。运营商 '？。' 太过分了。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();
}

is运算符具有以下模式：

expr is type varname

如果expr评估为true，则varname对象将是有效的，不需要再次与null比较，如此代码段所示。



警告5



V3022表达式'extension ==“ .xlsx” || extension ==“ .xlsm”'始终为false。 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;
        }
        ....
    }
    ....
}

结果是一个有趣的代码。第一作者清除了具有以下扩展名的所有文档，而不是.pptx，.pptm。POTX和。potm，然后决定检查其中是否没有.xlsx和.xlsm。PresentationDocument函数绝对是重构的受害者。



警告7



V3022表达式'OpenSettings.MarkupCompatibilityProcessSettings == null'始终为false。 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;
        }
    }
}

MarkupCompatibilityProcessSettings 属性从不返回null。如果在getter方法中发现class字段为null，则该对象将被新对象覆盖。另外，请注意，这不是对一个属性的递归调用，而是来自不同类的相同名称的属性。也许有些混乱导致编写不必要的支票。

其他警告

警告1



V3080可能会取消引用。考虑检查“ 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;    // <=
    ....
}

这是一个示例，其中额外的验证仅仅是不够的。PreviousSibling方法可以返回null，并且无需检查即可立即使用此函数的结果。



2个更危险的地方：

• V3080可能为空的取消引用。考虑检查“ prevNode”。OpenXmlCompositeElement.cs 489
• V3080可能为空的取消引用。考虑检查“ prevNode”。OpenXmlCompositeElement.cs 497

警告2



V3093 '＆'运算符同时计算两个操作数。也许应该使用短路的“ &&”运算符。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;
            }
        }
    }
    ....
}

有些人喜欢将'＆'运算符应用于不应该使用的逻辑表达式。对于此运算符，无论第一个操作数的结果如何，都将首先计算第二个操作数。这不是一个很严重的错误，但是重构后的这种草率代码可能会导致潜在的NullReferenceException异常。



警告3



V3097可能的例外：由[可序列化]标记的类型包含未由[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; } // <=
}

由于忘记了其中一个属性被标记为不可序列 化，因此OpenXmlPackageValidationEventArgs类的序列化可能会失败。或者您需要将此属性的返回类型修改为可序列化，否则在运行时可能会发生异常。

结论

我们公司喜欢Microsoft的项目和技术。在列出使用PVS-Studio测试的开源项目的部分中，我们甚至为Microsoft分配了单独的部分。已经积累了21个项目，已撰写了26篇文章。这是27日。



我确定您想知道我们的客户中是否有Microsoft。答案是肯定的！但是，请不要忘记这是一家领导全球发展的大型公司。肯定有一些部门已在其项目中使用PVS-Studio，但还有更多部门未使用！而且我们在开源项目中的经验表明，它们显然缺乏查找错误的好工具；）。

对于那些有兴趣分析C ++，C＃和Java代码的人来说，更多的新闻来自于新闻：我们最近增加了对OWASP标准的支持，并正在积极扩大其覆盖范围。

如果您想与讲英语的读者分享本文，请使用翻译链接：Svyatoslav Razmyslov。分析Microsoft Open XML SDK的代码质量。