Mi conocimiento del SDK de Open XML comenzó cuando necesitaba una biblioteca para crear documentos de Word con algunos informes. Después de trabajar con la API de Word durante más de 7 años, quería probar algo nuevo y más conveniente. Así es como descubrí que Microsoft tiene una solución alternativa. Tradicionalmente, verificamos previamente los programas y bibliotecas utilizados en la empresa mediante el analizador PVS-Studio.
Introducción
Office Open XML, también conocido como OpenXML u OOXML, es un formato basado en XML para documentos de Office, incluidos documentos de procesamiento de texto, hojas de cálculo, presentaciones y diagramas, formas y otros gráficos. La especificación fue desarrollada por Microsoft y adoptada por ECMA International en 2006. En junio de 2014, Microsoft lanzó Open XML SDK en código abierto. La fuente ahora está disponible en GitHub bajo la licencia MIT.
Para encontrar errores en el código fuente de la biblioteca, usamos PVS-Studio . Es una herramienta para identificar errores y posibles vulnerabilidades en el código fuente de programas escritos en C, C ++, C # y Java. Funciona en sistemas de 64 bits en Windows, Linux y macOS.
El proyecto es lo suficientemente pequeño y hubo pocas advertencias. Pero la elección de la imagen del título se basó precisamente en los resultados. Hay muchos operadores condicionales inútiles en el código. Me parece que si refactoriza todos esos lugares en el código, entonces el volumen se reducirá notablemente. Como resultado, también aumentará la legibilidad del código.
Por qué Word API y no Open XML SDK
Como habrás adivinado por el título, seguí usando la API de Word. Este método tiene muchas desventajas:
- API antigua e incómoda;
- Se debe instalar Microsoft Office;
- La necesidad de distribuir un kit de distribución con las bibliotecas de Office;
- Dependencia de la API de Word en la configuración regional del sistema;
- Baja velocidad de trabajo.
Con el lugar en general, ocurrió un incidente divertido. Windows tiene una docena de configuraciones regionales. En una de las computadoras servidor había un desorden de las configuraciones regionales de EE. UU. Y Reino Unido. Debido a esto, se crearon documentos de Word, donde en lugar del símbolo del dólar había un rublo y las libras no se mostraban en absoluto. La mejora de la configuración del sistema operativo solucionó el problema.
Al enumerar todo esto, me pregunté nuevamente por qué todavía lo uso ...
Pero no, todavía me gusta más la API de Word, y he aquí por qué.
OOXML se ve así:
<?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>Donde <w: r> (Word Run) no es una oración, ni siquiera una palabra, sino cualquier fragmento de texto que tenga atributos que difieran de los fragmentos de texto adyacentes.
Está programado con algo como esto:
Paragraph para = body.AppendChild(new Paragraph());
Run run = para.AppendChild(new Run());
run.AppendChild(new Text(txt));El documento tiene una estructura interna específica y en el código es necesario crear los mismos elementos. El SDK de Open XML, en mi opinión, no tiene suficiente capa de acceso a datos abstractos. La creación de un documento con la API de Word será más clara y breve. Especialmente cuando se trata de tablas y otras estructuras de datos complejas.
A su vez, el SDK de Open XML resuelve una amplia gama de tareas. Con él, puede crear documentos no solo para Word, sino también para Excel y PowerPoint. Esta biblioteca probablemente sea más adecuada para algunas tareas, pero decidí quedarme en la API de Word por ahora. En cualquier caso, no será posible abandonarlo por completo, porque para necesidades internas, estamos desarrollando un complemento para Word, y allí es posible usar solo la API de Word.
Dos valores para cadena
V3008 A la variable '_rawOuterXml' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verifique las líneas: 164, 161. OpenXmlElement.cs 164
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}El tipo de cadena puede tener 2 tipos de valores: nulo y valor de texto. Definitivamente es más seguro usar el significado textual, pero ambos enfoques son válidos. En este proyecto , es inaceptable usar el valor nulo y se reescribe a string.Empty ... al menos así es como se pretendía. Pero debido a un error en RawOuterXml , aún puede escribir nulo y luego hacer referencia a este campo, recibiendo una NullReferenceException .
La expresión V3022 'namespaceUri! = Null' siempre es verdadera. 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 fragmento utiliza el mismo enfoque, el autor del código no cometió ningún error importante, pero aún huele como una refactorización fallida. Lo más probable es que aquí se pueda eliminar una marca, lo que reducirá el ancho del código y, por lo tanto, aumentará su legibilidad.
Sobre la compacidad del código
V3009 Es extraño que este método siempre devuelva el mismo 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";
}
}No sé si hay algún error tipográfico aquí, o si el autor del código escribió un código "agradable" en su opinión. Estoy seguro de que no tiene sentido devolver tantos valores del mismo tipo desde una función, y el código se puede reducir considerablemente.
Este no es el único lugar de este tipo. Aquí hay un par más de estas advertencias:
- V3009 Es extraño que este método siempre devuelva el mismo valor de '".xml"'. CustomPropertyPartTypeInfo.cs 25
- V3009 Es extraño que este método siempre devuelva el mismo valor de '".bin"'. EmbeddedControlPersistenceBinaryDataPartTypeInfo.cs 22
Sería interesante saber por qué escribir así.
V3139 Dos o más ramas de casos realizan las mismas acciones. 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;
....
}
....
}Hay menos preguntas sobre este código. Lo más probable es que se puedan combinar casos idénticos y el código será más corto y más obvio.
Algunos lugares más:
- V3139 Dos o más ramas de casos realizan las mismas acciones. OpenXmlMiscNode.cs 312
- V3139 Dos o más ramas de casos realizan las mismas acciones. CustomPropertyPartTypeInfo.cs 30
- V3139 Dos o más ramas de casos realizan las mismas acciones. CustomXmlPartTypeInfo.cs 15
- V3139 Dos o más ramas de casos realizan las mismas acciones. OpenXmlElement.cs 1803
Aquellos siempre verdaderos / falsos
Ahora es el momento de proporcionar los ejemplos de código que determinaron mi elección de imagen de título.
Advertencia 1
V3022 La expresión 'Complete ()' siempre es 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;
}La propiedad IsComplete se usa 2 veces y es fácil de entender por el código que su valor no cambiará. Por lo tanto, al final de la función, simplemente puede devolver el segundo valor del operador ternario: verdadero .
Advertencia 2
La expresión V3022 '_elementStack.Count> 0' siempre es verdadera. 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, si no hay 0 elementos en _elementStack , entonces hay más. El código se puede acortar al menos en 8 líneas.
Advertencia 3
V3022 La expresión 'rootElement == null' siempre es 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(....);
}
....
}La función CreateElement no puede devolver un valor nulo . Si la empresa estableció una regla para escribir métodos para crear nodos xml que devuelvan un objeto válido o arrojen una excepción, los usuarios de dichas funciones no pueden abusar de las comprobaciones adicionales.
Advertencia 4
V3022 La expresión 'nameProvider' no siempre es nula. El operador '?.' es excesivo. 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();
}El operador is tiene el siguiente patrón:
expr is type varnameSi expr se evalúa como verdadero , entonces el objeto varname será válido y no es necesario compararlo con null nuevamente , como se hace en este fragmento de código.
Advertencia 5
V3022 Expression 'extensión == ".xlsx" || extension == ".xlsm" 'siempre es 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;
}
....
}
....
}Resultó un código interesante. Primer autor eliminar a todos los documentos con las siguientes extensiones no son .pptx , .pptm ,. potx y. potm , y luego decidió verificar que no hay .xlsx y .xlsm entre ellos . La función PresentationDocument es definitivamente una víctima de la refactorización.
Advertencia 7
V3022 La expresión 'OpenSettings.MarkupCompatibilityProcessSettings == null' siempre es 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;
}
}
}La propiedad MarkupCompatibilityProcessSettings nunca devuelve nulo . Si en el captador resulta que el campo de la clase es nulo , entonces el objeto se sobrescribe con uno nuevo. Además, tenga en cuenta que esta no es una llamada recursiva a una propiedad, sino propiedades del mismo nombre de diferentes clases. Quizás alguna confusión ha llevado a la emisión de cheques innecesarios.
Otras advertencias
Advertencia 1
V3080 Posible desreferencia nula. Considere la posibilidad de inspeccionar "hermano anterior". 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; // <=
....
}Y aquí hay un ejemplo en el que la verificación adicional simplemente no es suficiente. El método PreviousSibling puede devolver un valor nulo y el resultado de esta función se usa inmediatamente sin verificación.
2 lugares más peligrosos:
- V3080 Posible desreferencia nula. Considere inspeccionar 'prevNode'. OpenXmlCompositeElement.cs 489
- V3080 Posible desreferencia nula. Considere inspeccionar 'prevNode'. OpenXmlCompositeElement.cs 497
Advertencia 2
V3093 El operador '&' evalúa ambos operandos. Quizás debería utilizarse un operador de cortocircuito '&&' en su 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;
}
}
}
....
}A algunas personas les gusta usar el operador '&' en expresiones lógicas donde no deberían. En el caso de este operador, el segundo operando se evalúa primero, independientemente del resultado del primero. Este no es un error muy grave aquí, pero un código tan descuidado después de la refactorización puede dar lugar a posibles excepciones NullReferenceException .
Advertencia 3
V3097 Posible excepción: el tipo marcado por [Serializable] contiene miembros no serializables no 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; } // <=
}La serialización de la clase OpenXmlPackageValidationEventArgs puede fallar porque una de las propiedades se olvidó de marcar como no serializable. O es necesario modificar el tipo de retorno de esta propiedad para que sea serializable, de lo contrario puede ocurrir una excepción en tiempo de ejecución.
Conclusión
En la empresa, amamos los proyectos y las tecnologías de Microsoft. En la sección donde enumeramos los proyectos de código abierto probados con PVS-Studio, incluso asignamos una sección separada para Microsoft. Ya se han acumulado 21 proyectos, sobre los cuales se han escrito 26 artículos. Este es el 27.
Estoy seguro de que se está preguntando si Microsoft está entre nuestros clientes. ¡La respuesta es sí! Pero no olvidemos que esta es una gran corporación que lidera el desarrollo en todo el mundo. Definitivamente hay divisiones que ya usan PVS-Studio en sus proyectos, ¡pero hay aún más de las que no usan! Y nuestra experiencia con proyectos de código abierto muestra que claramente carecen de una buena herramienta para encontrar errores;).
Más noticias de las noticias para aquellos interesados en analizar código en C ++, C # y Java: recientemente hemos agregado soporte para el estándar OWASP y estamos aumentando activamente su cobertura.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Svyatoslav Razmyslov. Analizando la calidad del código del SDK Open XML de Microsoft .