Finalmente, un 2020 tan difícil está llegando a su fin, ¡lo que significa que es hora de hacer balance! Durante este año, el equipo de PVS-Studio ha escrito muchos artículos que tratan de varios errores encontrados por el analizador en proyectos de código abierto. Puede ver los más interesantes aquí, en los errores TOP encontrados en proyectos de C # en 2020. ¡Feliz visualización!
Cómo se formó la cima
Esta lista contiene los desencadenantes más interesantes, en mi opinión, sobre los que mis colegas y yo escribimos en artículos para 2020. Un criterio de selección importante fue el grado de confianza en que efectivamente se había cometido un error en el fragmento de código correspondiente. Y, por supuesto, al seleccionar, además de colocar lugares, se tuvo en cuenta el 'interés' del disparador, pero esta ya es mi opinión subjetiva: siempre puedes desafiarlo en los comentarios.
Intenté que la parte superior fuera lo más diversa posible: tanto en términos de mensajes de PVS-Studio como en términos de proyectos para los que se emitieron advertencias de código. La lista incluye detecciones en las fuentes de 8 proyectos verificados. Al mismo tiempo, las reglas de diagnóstico casi nunca se repiten: puede reunirse dos veces aquí solo V3022 y V3106 (y no, no los hice yo, pero aparentemente estos son mis favoritos). Por lo tanto, la variedad está garantizada aquí :).
Bueno, ¡comencemos! Top 10!
10mo lugar - Nueva licencia antigua
Nuestra respuesta principal comienza con un artículo de una muy buena persona sobre la verificación de proyectos C # para Linux y macOS, donde se usó el proyecto RavenDB como ejemplo:
private static void UpdateEnvironmentVariableLicenseString(....)
{
....
if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
return;
....
}
Advertencia del analizador : V3066 Posible orden incorrecto de los argumentos pasados al método 'ValidateLicense': 'newLicense' y 'oldLicense'. LicenseHelper.cs (177) Raven.Server
Al parecer, ¿qué pasa aquí? El código se compila bastante bien. ¿Por qué decidió el analizador que es necesario transferir primero la licencia anterior y luego la nueva ? Lo has adivinado, ¿verdad? Echemos un vistazo a la declaración ValidateLicense :
private static bool ValidateLicense(License oldLicense,
RSAParameters rsaParameters,
License newLicense)
Vaya, es cierto, primero, el anterior está en los parámetros, y solo entonces, la nueva licencia. Dime, ¿puede captar esto este análisis dinámico tuyo? :)
En cualquier caso, la activación es interesante. Tal vez el orden no sea realmente importante allí, pero es mejor verificar esos fragmentos, ¿de acuerdo?
Noveno lugar: 'FirstOrDefault' e inesperado 'nulo'
El noveno lugar lo ocupó la respuesta del artículo "¡ Juega en" osu! ", No te olvides de los errores ", escrito a principios del año en curso:
public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
var ruleset = rulesets.GetRuleset(OnlineRulesetID);
var mods = Mods != null ? ruleset.CreateInstance()
.GetAllMods().Where(....)
.ToArray() : Array.Empty<Mod>();
....
}
¿Ves el error? ¡Y ella es! ¿Qué dice el analizador?
Advertencia del analizador : V3146 [CWE-476] Posible desreferencia nula de 'conjunto de reglas'. El 'FirstOrDefault' puede devolver un valor nulo predeterminado. APILegacyScoreInfo.cs 24
Sí, sí, de nuevo no di toda la información necesaria a la vez. De hecho, realmente no puede ver nada sospechoso en este código. Después de todo, FirstOrDefault , del que nos habla el analizador, está en la definición del método GetRuleset :
public RulesetInfo GetRuleset(int id) =>
AvailableRulesets.FirstOrDefault(....);
¡Terrible negocio! El método devolverá RulesetInfo si encuentra uno adecuado. ¿Y si no? Devuelve nulo con calma . Y disparará en el lugar donde se utilizará el resultado de la llamada. En este caso, al llamar a ruleset.CreateInstance () .
Puede surgir la pregunta: ¿y si nunca hay nulo ? ¿Qué pasa si la colección siempre contiene el elemento requerido? Bueno, si el desarrollador está seguro de esto, ¿por qué no usar First en lugar de FirstOrDefault ?
8vo lugar - Hola de Python
El último disparador de los tres primeros se emitió para el código del proyecto RunUO. En febrero se escribió un artículo sobre cómo verificarlo y está disponible aquí .
El fragmento encontrado es extremadamente sospechoso, aunque es difícil decir con certeza si es un error:
public override void OnCast()
{
if ( Core.AOS )
{
damage = m.Hits / 2;
if ( !m.Player )
damage = Math.Max( Math.Min( damage, 100 ), 15 );
damage += Utility.RandomMinMax( 0, 15 );
}
else { .... }
}
Advertencia del analizador : V3043 La lógica operativa del código no se corresponde con su formato. La declaración tiene sangría a la derecha, pero siempre se ejecuta. Es posible que falten corchetes. Earthquake.cs 57
Así es, ¡se trata de sangría! Parece que el daño de línea + = Utility.RandomMinMax (0, 15) debería haberse ejecutado solo si m.Player es falso... De manera similar, el código Python funcionaría, donde la sangría no es solo por belleza, sino también para definir la lógica de la aplicación. ¡Pero el compilador de C # tiene una opinión diferente sobre este asunto! Me pregunto qué opinión tuvo el desarrollador.
En general, en esta situación, hay 2 opciones. O bien faltan las llaves aquí y todo no funciona correctamente, o todo funciona correctamente, pero con el tiempo definitivamente habrá una persona que considerará esto como un error y lo "arreglará".
Tal vez me equivoque, y realmente hay momentos en los que está bien escribir algo así. Si tiene conocimiento de estos, por favor escriba un comentario. Me interesaría mucho conocer estos casos.
Séptimo lugar - Perfecto o Perfecto - ¡esa es la cuestión!
Se vuelve cada vez más difícil distribuir advertencias en algunos lugares, ¡y estamos pasando a la segunda alarma en esta lista del artículo sobre osu! ...
¿Cuánto tiempo tardarás en ver el error?
protected override void CheckForResult(....)
{
....
ApplyResult(r =>
{
if ( holdNote.hasBroken
&& (result == HitResult.Perfect || result == HitResult.Perfect))
result = HitResult.Good;
....
});
}
Advertencia del analizador : V3001 Hay subexpresiones idénticas 'resultado == HitResult.Perfect' a la izquierda y a la derecha de '||' operador. DrawableHoldNote.cs 266 No
mucho, creo, solo necesita leer la advertencia de PVS-Studio. Los desarrolladores que utilizan análisis estático suelen hacer esto :). Se podría discutir sobre los lugares anteriores en la parte superior, pero aquí el error es obvio. Es difícil decir qué elemento específico de la enumeración HitResult debería haberse utilizado aquí en lugar del segundo Perfecto (o en lugar del primero), pero al final algo estaba claramente escrito mal. Bueno, está bien, se encontró el error, lo que significa que se puede solucionar fácilmente.
6to lugar - ¡pase nulo (no)!
El sexto lugar fue una respuesta muy buena al código del proyecto Open XML SDK. Puedes leer el artículo sobre cómo comprobarlo aquí .
El desarrollador quería implementar una propiedad que no devolviera un valor nulo , incluso si se asignaba directamente. Y es realmente genial cuando puede estar seguro de que no se escribirá nulo bajo ninguna circunstancia. Es una pena que esta no sea la situación en absoluto:
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}
Advertencia del analizador : V3008 A la variable '_rawOuterXml' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 164, 161. OpenXmlElement.cs 164
Resulta que en _rawOuterXml se registrará el valor independientemente de si es nulo o no. Si observa este código sin atención, podría pensar que nunca se escribirá null en esta propiedad ; después de todo, ¡hay una verificación! Bueno, si es así, debajo del árbol no puede encontrar regalos, sino una NullReferenceException inesperada :(
5to lugar - Disparo de una matriz con una matriz dentro
El analizador del proyecto TensorFlow.NET que probé personalmente emitió los cinco primeros activadores de 2020. Al hacer clic en el enlace , puede leer el artículo sobre cómo verificar este proyecto (oh, y he visto a todos allí mucho).
Por cierto, si le gusta ver ejemplos de errores interesantes de proyectos reales de C #, le sugiero que se suscriba a mi twitter . Allí planeo publicar disparadores curiosos y fragmentos de código simplemente interesantes, muchos de los cuales, lamentablemente, no se incluirán en los artículos. ¡Me alegrará verte! :)
Bueno, ahora pasemos a la activación:
public TensorShape(int[][] dims)
{
if(dims.Length == 1)
{
switch (dims[0].Length)
{
case 0: shape = new Shape(new int[0]); break;
case 1: shape = Shape.Vector((int)dims[0][0]); break;
case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
default: shape = new Shape(dims[0]); break;
}
}
else
{
throw new NotImplementedException("TensorShape int[][] dims");
}
}
Advertencia del analizador : V3106 Posiblemente el índice esté fuera de límites. El índice '1' apunta más allá del límite de 'dims'. TensorShape.cs 107
De hecho, fue muy difícil elegir dónde poner este gatillo, ya que es realmente interesante, pero otros no se quedan atrás en este sentido. Entonces, averigüemos qué está pasando aquí.
Si el número de matrices en dims no es igual a 1, se lanza una excepción del tipo NotImplementedException . Y que pasará si en la oscuridadexactamente una matriz? Se comprueba el número de elementos de esta 'matriz dentro de una matriz'. Note lo que sucede cuando es 2. Como uno de los argumentos del constructor Shape.Matrix , inesperadamente, se pasa dims [1] [2] . Ahora recordemos: ¿cuántos elementos había en la matriz de atenuación ?
Así es, exactamente uno, ¡acabamos de comprobarlo! Un intento de obtener el segundo elemento de una matriz, que contiene solo un elemento, generará una excepción IndexOutOfRangeException . Obviamente un error. Pero, ¿hay alguna forma obvia de solucionarlo?
Lo primero que me viene a la mente es el cambio atenúa [1] [2] a atenúa [0] [2] . ¿Desaparecerá el error? ¡No importa cómo sea! Habrá la misma excepción nuevamente. Pero esta vez estará relacionado con el hecho de que en este caso-branch el número de elementos en dims [0] es igual a 2. ¿Ha cometido el desarrollador dos errores en el índice seguidos? ¿O debería usarse alguna otra variable allí? Quién sabe ... El trabajo del analizador es encontrar un error, y la persona que lo cometió o sus compañeros tendrán que corregirlo.
4to lugar - Propiedad de un objeto que no existe
Otro disparador entró en esta parte superior del artículo sobre el cheque OpenRA . Quizás merezca más, pero por la voluntad del destino, este gatillo estaba en el cuarto lugar. Bueno, ¡eso también es bastante bueno! Echemos un vistazo al error que PVS-Studio pudo detectar esta vez:
public ConnectionSwitchModLogic(....)
{
....
var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
if (logo != null)
{
logo.GetSprite = () =>
{
....
};
}
if (logo != null && mod.Icon == null) // <=
{
// Hide the logo and center just the text
if (title != null)
title.Bounds.X = logo.Bounds.Left;
if (version != null)
version.Bounds.X = logo.Bounds.X;
width -= logo.Bounds.Width;
}
else
{
// Add an equal logo margin on the right of the text
width += logo.Bounds.Width; // <=
}
....
}
Advertencia del analizador : V3125 El objeto 'logo' se utilizó después de que se verificara con un valor nulo. Verifique las líneas: 236, 222. ConnectionLogic.cs 236
¿A qué debe prestar atención aquí? Bueno, para empezar, el logo probablemente puede contener null . Esto se insinúa mediante comprobaciones constantes y el nombre del método GetOrNull , que devuelve el valor escrito en el logotipo . Si es así, pensemos en lo que sucede si GetOrNull realmente devuelve null . Al principio todo está en orden, pero luego se comprueba el estado logo! = null && mod.Icon == null . Como puede imaginar, el resultado será una transición a else -branch y ... Intente acceder a la propiedad Bounds de la variable, que contiene null , y luego - bdysh! NullReferenceException está llamando a la puerta.
3er lugar - elemento de Schrödinger
Finalmente, llegamos a los tres primeros finalistas. Los 3 errores principales para 2020 se encontraron en el proyecto Nethermind, sobre cuya verificación se escribió un artículo con el título intrigante " Código en una línea o comprobación de Nethermind usando PVS-Studio C # para Linux ". El error es increíblemente simple, pero invisible para el ojo humano, especialmente si se considera el tamaño del proyecto. ¿Crees que este disparador es digno de su lugar?
public ReceiptsMessage Deserialize(byte[] bytes)
{
if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
return new ReceiptsMessage(null);
....
}
Advertencia del analizador : V3106 Posiblemente el índice esté fuera de límites. El índice '0' apunta más allá del límite de los 'bytes'. Nethermind.Network ReceiptsMessageSerializer.cs 50
Probablemente ser capaz de tomar lo primero en una caja vacía sería genial, pero aquí tal deseo solo conducirá a que se lance una IndexOutOfRangeException . Solo una bagatela: un pequeño error en el operador, y la aplicación ya está funcionando incorrectamente, o tal vez incluso se bloquea.
Obviamente, debería usar '||' en lugar de '&&'. Tales errores lógicos no son infrecuentes, especialmente en diseños complejos. Por lo tanto, es muy conveniente verificar esos momentos en modo automático.
2do lugar - Menos de 2, pero más de 3
En segundo lugar, puse un disparador más en el código del proyecto RavenDB. Permítanme recordarles que pueden leer sobre los resultados de su verificación (y no solo) en el artículo correspondiente .
Bueno, ahora conocemos: los 2 errores principales de 2020:
private OrderByField ExtractOrderByFromMethod(....)
{
....
if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
throw new InvalidQueryException(....);
....
}
Advertencia del analizador : V3022 Expression 'me.Arguments.Count <2 && me.Arguments.Count> 3' es siempre falsa. Probablemente el '||' operador debe usarse aquí. QueryMetadata.cs (861) Raven.Server
Anteriormente vimos los momentos en los que se lanzó una excepción inesperada, pero ahora, por el contrario, la excepción esperada nunca se lanzará. Bueno, o aún se descartará si a alguien se le ocurre un número que es menor que 2, pero más de 3.
No me sorprendería si no está de acuerdo, pero realmente me gusta este disparador más que todos los anteriores. Sí, el error es increíblemente simple y para solucionarlo solo necesitas cambiar de operador. Por cierto, esto también se insinúa en el mensaje pasado al constructor. InvalidQueryException : "Invalid ORDER BY 'Spacial.distance (from, to, roundFactor)', esperados 2-3 argumentos, obtuvo" + me.Arguments.Count .
Estoy de acuerdo, este es un descuido elemental, pero nadie lo notó ni lo corrigió, al menos hasta que se encontró usando PVS-Studio. Me recuerda que los programadores, sin importar la experiencia que tengan, siguen siendo (¿desafortunadamente?) Humanos. Y las personas, independientemente de sus calificaciones, pueden pasar por alto incluso esos errores estúpidos por diversas razones. A veces, el error se activa de inmediato y, a veces, puede preguntarse durante mucho tiempo por qué el usuario no ve el mensaje sobre la llamada ORDER BY incorrecta.
1er lugar - Cotizaciones para + 100% de seguridad de código
¡Hurra, hurra, hurra! Finalmente llegamos al gatillo, que encontré más interesante, divertido, genial, etc. Se emitió para el código del proyecto ONLYOFFICE, cuyo análisis está asociado con uno de los artículos más recientes de este año: " Servidor comunitario de ONLYOFFICE: cómo los errores contribuyen a los problemas de seguridad ".
Bueno, les presento la historia más triste sobre ArgumentException , que nunca se lanzará:
public void SetCredentials(string userName, string password, string domain)
{
if (string.IsNullOrEmpty(userName))
{
throw new ArgumentException("Empty user name.", "userName");
}
if (string.IsNullOrEmpty("password"))
{
throw new ArgumentException("Empty password.", "password");
}
CredentialsUserName = userName;
CredentialsUserPassword = password;
CredentialsDomain = domain;
}
Advertencia del analizador : La expresión V3022 'string.IsNullOrEmpty ("contraseña")' siempre es falsa. SmtpSettings.cs 104
Fue muy difícil elegir qué error poner en qué lugar, pero este disparador para mí desde el principio fue el líder entre todos. El error tipográfico menor más simple, y el código ya funciona incorrectamente. Ni lo destacado en el IDE, ni la revisión, ni el buen sentido común ayudaron. Es una función pequeña, simple y bellamente escrita. E incluso aquí PVS-Studio pudo encontrar lo que los profesionales se perdieron.
Como de costumbre, el diablo está en los detalles. ¿No sería fantástico si todos esos detalles se buscaran automáticamente? ¡Por supuesto que sí! Y deje que el desarrollador haga lo que un analizador estático no puede hacer: crea nuevas aplicaciones hermosas y seguras. Crea sin pensar en si puso comillas adicionales al marcar una variable o no.
Conclusión
Encontrar 10 errores interesantes en los artículos de 2020 fue fácil. Pero distribuirlos en lugares resultó ser una gran tarea. Por un lado, algunos desencadenantes reflejan mejor el trabajo de los mecanismos analizadores avanzados. Por otro lado, algunos de los errores parecen divertidos hasta cierto punto. Muchas de las posiciones presentadas podrían intercambiarse, por ejemplo, top 2 y top 3.
¿O tal vez crees que debería haber otros aspectos positivos en esta lista? De hecho, incluso puedes crear tu propio top siguiendo el enlacea la lista de artículos y encuentre los desencadenantes más deliciosos en su opinión. En este caso, asegúrese de arrojar sus tops 2020 en los comentarios, lo leeré con mucho gusto. ¿Puedes hacer una lista que sea más interesante que la mía?
Por supuesto, la cuestión del "interés" de las advertencias es subjetiva de todos modos. En mi opinión, el criterio principal para evaluar la respuesta es si un programador que ve una advertencia de PVS-Studio cambiará algo en el código correspondiente. Esta lista acaba de compilarse a partir de desencadenantes de fragmentos, que, en mi opinión, se verían mejor si los desarrolladores usaran análisis estático. Además, no hay problemas para probar PVS-Studio comprobando sus propios proyectos o algunos otros. Solo necesitas seguir el enlace, descargue la versión requerida del analizador allí y solicite una clave de prueba completando un pequeño formulario.
Bueno, eso es todo para mí, muchas gracias por su atención y ¡hasta pronto!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Nikita Lipilin. Los 10 errores principales encontrados en proyectos de C # en 2020 .