De vez en cuando volvemos a los proyectos que hemos verificado previamente utilizando PVS-Studio, lo que da como resultado sus descripciones en varios artículos. Dos razones hacen que estos regresos sean emocionantes para nosotros. En primer lugar, la oportunidad de evaluar el progreso de nuestro analizador. En segundo lugar, monitorear los comentarios de los autores del proyecto sobre nuestro artículo y el informe de errores, que generalmente les proporcionamos. Por supuesto, los errores pueden corregirse sin nuestra participación. Sin embargo, siempre es bueno cuando nuestros esfuerzos ayudan a mejorar un proyecto. Roslyn no fue la excepción. El artículo anterior sobre la verificación de este proyecto data del 23 de diciembre de 2015. En vista del progreso que nuestro analizador ha hecho desde entonces, es bastante largo. Dado que el núcleo C # del analizador PVS-Studio se basa en Roslyn, nos da un interés adicional en este proyecto. Como resultado, estamos tan interesados como la mostaza sobre la calidad del código de este proyecto. Ahora probémoslo una vez más y descubramos algunos problemas nuevos e interesantes (pero esperemos que nada significativo) que PVS-Studio pueda encontrar.
Es probable que muchos de nuestros lectores conozcan bien a Roslyn (o .NET Compiler Platform). En resumen, es un conjunto de compiladores de código abierto y una API para el análisis de código de los lenguajes C # y Visual Basic .NET de Microsoft. El código fuente del proyecto está disponible en
GitHub .
No daré una descripción detallada de esta plataforma y recomendaré revisar el artículo de mi colega Sergey Vasiliev "
Introducción a Roslyn y su uso en el desarrollo de programas " a todos los lectores interesados. A partir de este artículo, puede descubrir no solo las características de la arquitectura de Roslyn, sino también cómo usamos exactamente esta plataforma.
Como mencioné anteriormente, han pasado más de 3 años desde que mi colega Andrey Karpov escribió el último artículo sobre el cheque de Roslyn "
Lanzamiento de Año Nuevo PVS-Studio 6.00: Escaneo de Roslyn ". Desde entonces, el analizador C # PVS-Studio tenía muchas características nuevas. En realidad, el artículo de Andrey era un caso de prueba, ya que en ese momento el analizador C # acababa de agregarse en PVS-Studio. A pesar de esto, logramos detectar errores en el proyecto Roslyn, que sin duda fue de alta calidad. Entonces, ¿qué ha cambiado en el analizador para el código C # en este momento que nos permitirá realizar un análisis más profundo?
Desde entonces, tanto el núcleo como la infraestructura se han desarrollado. Agregamos soporte para Visual Studio 2017 y Roslyn 2.0, y una integración profunda con MSBuild. El artículo de mi colega Paul Eremeev "
Soporte de Visual Studio 2017 y Roslyn 2.0 en PVS-Studio: a veces no es tan fácil de usar como parece " describe nuestro enfoque de integración con MSBuild y los motivos de esta decisión.
Por el momento, estamos trabajando activamente para pasar a Roslyn 3.0 de la misma manera que inicialmente admitimos Visual Studio 2017. Requiere el uso de nuestro propio conjunto de herramientas, incluido en el distributivo PVS-Studio como un "trozo", que es un MSBuild vacío archivo .exe A pesar del hecho de que parece una "muleta" (MSBuild API no es muy amigable para reutilizar en proyectos de terceros debido a la baja portabilidad de las bibliotecas), este enfoque ya nos ha ayudado a superar relativamente fácilmente múltiples actualizaciones de Roslyn en términos de Visual Studio 2017. Hasta ahora, estaba ayudando (incluso con algunos desafíos) a pasar por la actualización de Visual Studio 2019 y mantener una compatibilidad y un rendimiento completos para sistemas con versiones anteriores de MSBuild.
El núcleo del analizador también ha experimentado una serie de mejoras. Una de las características principales es un análisis interprocedural completo con consideración de los valores de los métodos de entrada y salida, evaluando (dependiendo de estos parámetros) el alcance de las ramas de ejecución y los puntos de retorno.
Estamos en camino de completar la tarea de monitorear los parámetros dentro de los métodos (por ejemplo, desreferencias potencialmente peligrosas) junto con guardar sus anotaciones automáticas. Para un diagnóstico que utiliza un mecanismo de flujo de datos, esto permitirá tener en cuenta situaciones peligrosas que se producen al pasar un parámetro en un método. Antes de esto, al analizar lugares tan peligrosos, no se generaba una advertencia, ya que no podíamos conocer todos los valores de entrada posibles en dicho método. Ahora podemos detectar el peligro, ya que en todos los lugares de llamar a este método, estos parámetros de entrada se tendrán en cuenta.
Nota: puede leer acerca de los mecanismos básicos del analizador, como el flujo de datos y otros en el artículo "
Tecnologías utilizadas en el analizador de código PVS-Studio para encontrar errores y vulnerabilidades potenciales ".
El análisis interprocedural en PVS-Studio C # no está limitado ni por los parámetros de entrada, ni por la profundidad. La única limitación son los métodos virtuales en las clases, abiertos para la herencia, así como también para entrar en recursión (el análisis se detiene cuando se topa con una llamada repetida del método ya evaluado). Al hacerlo, el método recursivo en sí mismo se evaluará eventualmente suponiendo que se desconoce el valor de retorno de su recursividad.
Otra gran característica nueva en el analizador C # se ha convertido en tener en cuenta la posible desreferencia de un puntero potencialmente nulo. Antes de eso, el analizador se quejó de una posible excepción de referencia nula, asegurándose de que en todas las ramas de ejecución el valor de la variable sea nulo. Por supuesto, estuvo mal en algunos casos, por eso el diagnóstico
V3080 había llamado previamente referencia nula potencial.
Ahora el analizador es consciente del hecho de que la variable podría ser nula en una de las ramas de ejecución (por ejemplo, bajo una cierta condición
if ). Si nota el acceso a dicha variable sin una verificación, emitirá la advertencia V3080, pero con un nivel de certeza más bajo, que si ve nulo en todas las ramas. Junto con el análisis interprocedural mejorado, dicho mecanismo permite encontrar errores que son muy difíciles de detectar. Aquí hay un ejemplo: imagine una larga cadena de llamadas a métodos, la última de las cuales no le es familiar. Bajo ciertas circunstancias, devuelve nulo en el bloque
catch , pero no se ha protegido de esto, como simplemente no lo ha sabido. En este caso, el analizador solo se queja cuando ve exactamente la asignación nula. En nuestra opinión, distingue cualitativamente nuestro enfoque de la característica de C # 8.0 como referencia de tipo anulable, que, de hecho, se limita a establecer verificaciones para nulo para cada método. Sin embargo, sugerimos la alternativa: realizar comprobaciones solo en lugares donde realmente puede ocurrir nulo, y nuestro analizador ahora puede buscar tales casos.
Por lo tanto, no demoremos demasiado el punto principal y pasemos a la toma de culpa: analicemos los resultados de la verificación de Roslyn. Primero, consideremos los errores encontrados debido a las características descritas anteriormente. En resumen, esta vez hubo muchas advertencias para el código de Roslyn. Creo que está relacionado con el hecho de que la plataforma está evolucionando muy activamente (en este punto, la base de código es de aproximadamente 2,770,000 líneas excluidas las vacías), y no hemos analizado este proyecto por mucho tiempo. Sin embargo, no hay tantos errores críticos, mientras que son de mayor interés para el artículo. Como de costumbre, excluí las pruebas del cheque, hay muchas en Roslyn.
Comenzaré con los errores V3080 del nivel medio de certeza, en el que el analizador ha detectado un posible acceso por referencia nula, pero no en todos los casos posibles (ramificaciones de código).
Posible desreferencia nula - MediaV3080 Posible desreferencia nula. Considere inspeccionar 'actual'. CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70
private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....))
Consideremos el método
GetNode . El analizador sugiere que el acceso por referencia nula es posible en la condición del bloque
while . A la variable se le asigna un valor en el cuerpo del bloque
while , que es el resultado del método
AsNode . En algunos casos, este valor será
nulo . Un buen ejemplo de análisis interprocedural en acción.
Ahora consideremos un caso similar, en el que el análisis interprocedural se realizó a través de dos llamadas a métodos.
V3080 Posible desreferencia nula. Considere inspeccionar 'directorio'. CommonCommandLineParser.cs 911
private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ?
La variable de
directorio en el cuerpo del método
ExpandFileNamePattern obtiene el valor del método
GetDirectoryName (string) . Eso, a su vez, devuelve el resultado del método sobrecargado
GetDirectoryName (string, bool) cuyo valor puede ser
nulo . Dado que el
directorio variable se usa sin una comprobación preliminar de nulo en el cuerpo del método
ExpandFileNamePattern , podemos proclamar que el analizador es correcto al emitir la advertencia. Esta es una construcción potencialmente insegura.
Otro fragmento de código con el error V3080, más precisamente, con dos errores, emitido para una sola línea de código. El análisis interprocedural no era necesario aquí.
V3080 Posible desreferencia nula. Considere la posibilidad de inspeccionar 'spanStartLocation'. TestWorkspace.cs 574
V3080 Posible desreferencia nula. Considere inspeccionar 'spanEndLocationExclusive'. TestWorkspace.cs 574
private void MapMarkupSpans(....) { .... foreach (....) { .... foreach (....) { .... int? spanStartLocation = null; int? spanEndLocationExclusive = null; foreach (....) { if (....) { if (spanStartLocation == null && positionInMarkup <= markupSpanStart && ....) { .... spanStartLocation = ....; } if (spanEndLocationExclusive == null && positionInMarkup <= markupSpanEndExclusive && ....) { .... spanEndLocationExclusive = ....; break; } .... } .... } tempMappedMarkupSpans[key]. Add(new TextSpan( spanStartLocation.Value,
Las variables
spanStartLocation y
spanEndLocationExclusive son del tipo
int anulables y se inicializan por
null . Más adelante en el código se les pueden asignar valores, pero solo bajo ciertas condiciones. En algunos casos, su valor sigue siendo
nulo . Después de eso, se accede a estas variables por referencia sin verificación preliminar de nulo, lo que indica el analizador.
El código de Roslyn contiene muchos de estos errores, más de 100. A menudo, el patrón de estos errores es el mismo. Hay algún tipo de método general, que potencialmente devuelve
nulo . El resultado de este método se usa en muchos lugares, a veces a través de docenas de llamadas a métodos intermedios o verificaciones adicionales. Es importante comprender que estos errores no son fatales, pero potencialmente pueden conducir al acceso por referencia nula. Si bien la detección de tales errores es bastante desafiante. Es por eso que en algunos casos uno debería considerar la refactorización de código, en cuyo caso si
nulo regresa, el método arrojará una excepción. De lo contrario, puede proteger su código solo con verificaciones generales, lo cual es bastante agotador y, a veces, poco confiable. De todos modos, está claro que cada caso específico requiere una solución basada en las especificaciones del proyecto.
Nota Sucede que en un punto dado no hay tales casos (datos de entrada), cuando el método devuelve
nulo y no hay un error real. Sin embargo, dicho código aún no es confiable, porque todo puede cambiar al introducir algunos cambios en el código.
Para abandonar el tema
V3080 , echemos un vistazo a los errores obvios de alto nivel de certeza, cuando el acceso por referencia nula es el más probable o incluso inevitable.
Posible desreferencia nula - AltaV3080 Posible desreferencia nula. Considere inspeccionar 'collectionType.Type'. AbstractConvertForToForEachCodeRefactoringProvider.cs 137
public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { .... var collectionType = semanticModel.GetTypeInfo(....); if (collectionType.Type == null && collectionType.Type.TypeKind == TypeKind.Error) { return; } .... }
Debido al error tipográfico en la condición (
&& se usa en lugar del operador
|| ), el código funciona de manera diferente a la prevista y el acceso a la variable
collectionType.Type se ejecutará cuando sea
nulo . La condición debe corregirse de la siguiente manera:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
Por cierto, las cosas pueden desarrollarse de otra manera: en la primera parte de la condición, los operadores
== y
! = Están en mal estado
. Entonces el código correcto se vería así:
if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....
Esta versión del código es menos lógica, pero también corrige el error. La solución final recae en la decisión de los autores del proyecto.
Otro error similar.
V3080 Posible desreferencia nula. Considere inspeccionar 'acción'. TextViewWindow_InProc.cs 372
private Func<IWpfTextView, Task> GetLightBulbApplicationAction(....) { .... if (action == null) { throw new InvalidOperationException( $"Unable to find FixAll in {fixAllScope.ToString()} code fix for suggested action '{action.DisplayText}'."); } .... }
El error se produce al generar el mensaje para la excepción. Le sigue el intento de acceder a la propiedad
action.DisplayText a través de la variable
action , que se sabe que es
nula .
Aquí viene el último error V3080 del nivel alto.
V3080 Posible desreferencia nula. Considere inspeccionar 'tipo'. ObjectFormatterHelpers.cs 91
private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }
El método es bastante pequeño, así que lo cito por completo. La condición en el bloque de
retorno es incorrecta. En algunos casos, al acceder a
type.FullName , puede producirse una excepción. Usaré paréntesis para aclararlo (no cambiarán el comportamiento):
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
Según la precedencia de las operaciones, el código funcionará exactamente así. En caso de que la variable de
tipo sea
nula , entraremos en el check de else, donde usaremos la referencia de
tipo nulo, después de haber verificado la variable
targetTypeName para
null . El código puede repararse, por ejemplo, de la siguiente manera:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Creo que es suficiente para revisar los errores de V3080. Ahora es el momento de ver otras cosas interesantes que el analizador PVS-Studio logró encontrar.
Error tipográficoV3005 La variable 'SourceCodeKind' se asigna a sí misma. DynamicFileInfo.cs 17
internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind;
Debido al error en el nombre de las variables, se realizó un error tipográfico en el constructor de la clase
DynamicFileInfo . Al campo
SourceCodeKind se le asigna su propio valor en lugar de utilizar el parámetro
sourceCodeKind . Para minimizar la probabilidad de tales errores, le recomendamos que utilice el prefijo de subrayado para los nombres de los parámetros en tales casos. Aquí hay un ejemplo de una versión corregida del código:
public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }
InadvertenciaV3006 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': lanzar una nueva InvalidOperationException (FOO). ProjectBuildManager.cs 61
~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
Bajo cierta condición, el destructor debe lanzar una excepción, pero no sucede mientras el objeto de excepción simplemente se crea. Se perdió la palabra clave de
lanzamiento . Aquí está la versión corregida del código:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
El problema con los destructores en C # y el lanzamiento de excepciones de ellos es un tema para otra discusión, que está más allá del alcance de este artículo.
Cuando el resultado no es importanteLos métodos, que recibieron el mismo valor en todos los casos, activaron un cierto número de advertencias
V3009 . En algunos casos, puede no ser crítico o el valor de retorno simplemente no se verifica en el código de llamada. Me salté tales advertencias. Pero algunos fragmentos de código parecían sospechosos. Aquí está uno de ellos:
V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. GoToDefinitionCommandHandler.cs 62
internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }
El método
TryExecuteCommand devuelve nada más que
verdadero . Al hacerlo, en el código de llamada, el valor devuelto participa en algunas comprobaciones.
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
Es difícil decir exactamente en qué medida ese comportamiento es peligroso. Pero si el resultado no es necesario, tal vez el tipo del valor de retorno debería cambiarse a vacío y uno debería hacer pequeñas modificaciones en el método de llamada. Esto hará que el código sea más legible y seguro.
Advertencias de analizador similares:
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. CommentUncommentSelectionCommandHandler.cs 86
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. JsonRpcClient.cs 138
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. AbstractFormatEngine.OperationApplier.cs 164
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'falso'. TriviaDataFactory.CodeShapeAnalyzer.cs 254
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. ObjectList.cs 173
- V3009 Es extraño que este método siempre devuelva uno y el mismo valor de 'verdadero'. ObjectList.cs 249
Comprobado lo incorrectoV3019 Posiblemente, una variable incorrecta se compara con nulo después de la conversión de tipo usando la palabra clave 'as'. Verifique las variables 'value', 'valueToSerialize'. RoamingVisualStudioProfileOptionPersister.cs 277
public bool TryPersist(OptionKey optionKey, object value) { .... var valueToSerialize = value as NamingStylePreferences; if (value != null) { value = valueToSerialize.CreateXElement().ToString(); } .... }
La variable de
valor se
convierte al tipo
NamingStylePreferences . El problema está en el control que sigue a esto. Incluso si la variable de
valor no es nula, no garantiza que la conversión de tipos haya sido exitosa y
valueToSerialize no contenga
nulo . Posible lanzamiento de la excepción
NullReferenceException . El código debe corregirse de la siguiente manera:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Otro error similar:
V3019 Posiblemente, una variable incorrecta se compara con nulo después de la conversión de tipo usando la palabra clave 'as'. Verifique las variables 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181
private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name ==
La variable
columnState se
convierte al tipo
ColumnState2 . Sin embargo, el resultado de la operación, que es la variable
columnState2, no se verifica para
nulo adicional. En cambio, la variable
columnState se verifica utilizando el operador
nulo condicional. ¿Por qué es peligroso este código? Al igual que en el ejemplo anterior, la conversión con el operador
as puede fallar y la variable será
nula, lo que dará como resultado una excepción. Por cierto, un error tipográfico puede ser el culpable aquí. Observe la condición en el bloque
if .
Tal vez, en lugar de
columnState? .Name, el autor quería escribir
columnState2? .Name . Es muy probable, considerando nombres de variables bastante defectuosos
columnState y
columnState2.Cheques redundantesSe emitió una gran cantidad de advertencias (más de 100) en construcciones no críticas pero potencialmente inseguras relacionadas con verificaciones redundantes. Por ejemplo, este es uno de ellos.
V3022 La expresión 'navInfo == null' siempre es falsa. AbstractSyncClassViewCommandHandler.cs 101
public bool ExecuteCommand(....) { .... IVsNavInfo navInfo = null; if (symbol != null) { navInfo = libraryService.NavInfoFactory.CreateForSymbol(....); } if (navInfo == null) { navInfo = libraryService.NavInfoFactory.CreateForProject(....); } if (navInfo == null)
Puede ser que no haya un error real aquí. Es solo una buena razón para demostrar que el "análisis interprocedural + análisis de flujo de datos" funciona en un remolque. El analizador sugiere que la segunda verificación
navInfo == null es redundante. De hecho, antes de eso, el valor asignado a
navInfo se obtendrá del método
libraryService.NavInfoFactory.CreateForProject , que construirá y devolverá un nuevo objeto de la clase
NavInfo . De ninguna manera volverá
nulo . Aquí surge la pregunta, ¿por qué el analizador no emitió una advertencia para la primera verificación
navInfo == null ? Hay algunas razones En primer lugar, si la variable de
símbolo es
nula , el valor
navInfo seguirá siendo una referencia nula también. En segundo lugar, incluso si
navInfo obtiene el valor del método
ibraryService.NavInfoFactory.CreateForSymbol , este valor también puede ser
nulo . Por lo tanto, la primera verificación
navInfo == null es realmente necesaria.
Controles insuficientesAhora la situación inversa de lo discutido anteriormente. Se
activaron varias advertencias
V3042 para el código, en el que es posible el acceso por referencia nula. Incluso uno o dos cheques pequeños podrían haber solucionado todo.
Consideremos otro fragmento de código interesante, que tiene dos de esos errores.
V3042 Posible NullReferenceException. El '?.' y '.' Los operadores se utilizan para acceder a los miembros del objeto 'receptor' Binder_Expressions.cs 7770
V3042 Posible NullReferenceException. El '?.' y '.' Los operadores se utilizan para acceder a los miembros del objeto 'receptor' Binder_Expressions.cs 7776
private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax !=
La variable del
receptor puede ser nula. El autor del código sabe de esto, ya que utiliza el operador
nulo condicional en la condición del bloque
if para acceder al
receptor ? .
Sintaxis . Además, la variable del
receptor se usa sin ninguna comprobación para acceder a
receptor.Tipo ,
receptor.Sintaxis y
receptor.TieneErrores . Estos errores deben corregirse:
private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver?.Type; if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver?.Syntax, 0, receiverType ?? CreateErrorType(), hasErrors: receiver?.HasErrors) { WasCompilerGenerated = true }; return receiver; }
También debemos asegurarnos de que el constructor admite obtener valores
nulos para sus parámetros o debemos realizar una refactorización adicional.
Otros errores similares:
- V3042 Posible NullReferenceException. El '?.' y '.' Los operadores se utilizan para acceder a los miembros del objeto 'contienenType' SyntaxGeneratorExtensions_Negate.cs 240
- V3042 Posible NullReferenceException. El '?.' y '.' Los operadores se utilizan para acceder a los miembros del objeto 'expresión' ExpressionSyntaxExtensions.cs 349
- V3042 Posible NullReferenceException. El '?.' y '.' Los operadores se utilizan para acceder a los miembros del objeto 'expresión' ExpressionSyntaxExtensions.cs 349
Error en la condicionV3057 La función '
Subcadena ' podría recibir el valor '-1' mientras se espera un valor no negativo. Inspeccione el segundo argumento. CommonCommandLineParser.cs 109
internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }
En caso de que la variable de
dos puntos sea 0, lo cual está bien de acuerdo con la condición del código, el método
Substring arrojará una excepción. Esto tiene que ser arreglado:
if (colon > 0)
Posible error tipográficoEl parámetro
V3065 't2' no se utiliza dentro del cuerpo del método. CSharpCodeGenerationHelpers.cs 84
private static TypeDeclarationSyntax ReplaceUnterminatedConstructs(....) { .... var updatedToken = lastToken.ReplaceTrivia(lastToken.TrailingTrivia, (t1, t2) => { if (t1.Kind() == SyntaxKind.MultiLineCommentTrivia) { var text = t1.ToString(); .... } else if (t1.Kind() == SyntaxKind.SkippedTokensTrivia) { return ReplaceUnterminatedConstructs(t1); } return t1; }); .... }
La expresión lambda acepta dos parámetros: t1 y t2. Sin embargo, solo se usa t1. Parece sospechoso, teniendo en cuenta el hecho de lo fácil que es cometer un error al usar variables con tales nombres.
InadvertenciaV3083 Invocación insegura del evento 'TagsChanged', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. PreviewUpdater.Tagger.cs 37
public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span));
El evento
TagsChanged se invoca de forma insegura. Entre la comprobación de
nulo y la invocación del evento, alguien puede darse de baja del mismo, luego se lanzará una excepción. Además, se realizan otras operaciones en el cuerpo del bloque
if justo antes de invocar el evento. Llamé a este error "Inadvertencia", porque este evento se maneja con más cuidado en otros lugares, de la siguiente manera:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
El uso de una variable de
controlador adicional evita el problema. En el método
OnTextBufferChanged, uno tiene que hacer ediciones para manejar el evento de manera segura.
Rangos de intersecciónV3092 Las intersecciones de rango son posibles dentro de expresiones condicionales. Ejemplo: if (A> 0 && A <5) {...} más if (A> 3 && A <9) {...}. ILBuilderEmit.cs 677
internal void EmitLongConstant(long value) { if (value >= int.MinValue && value <= int.MaxValue) { .... } else if (value >= uint.MinValue && value <= uint.MaxValue) { .... } else { .... } }
Para una mejor comprensión, permítanme reescribir este código, cambiando los nombres de las constantes con sus valores reales:
internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }
Probablemente, no hay un error real, pero la condición parece extraña. Su segunda parte (
si no ) se ejecutará solo para el rango de 2147483648 + 1 a 4294967295.
Otro par de advertencias similares:
- V3092 Las intersecciones de rango son posibles dentro de expresiones condicionales. Ejemplo: if (A> 0 && A <5) {...} más if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 109
- V3092 Las intersecciones de rango son posibles dentro de expresiones condicionales. Ejemplo: if (A> 0 && A <5) {...} más if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 66
Más sobre cheques para nulo (o falta de ellos)Un par de errores
V3095 en la comprobación de una variable para nulo justo después de su uso. El primero es ambiguo, consideremos el código.
V3095 El objeto 'displayName' se usó antes de que se verificara como nulo. Líneas de verificación: 498, 503. FusionAssemblyIdentity.cs 498
internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }
Se supone que la referencia
tee puede ser nula. Para esto, se realizó la verificación
Debug.Assert . No está claro por qué va después de usar una cadena. También debe tenerse en cuenta que para configuraciones diferentes de Debug, el compilador eliminará
Debug.Assert . ¿Significa que obtener una referencia nula solo es posible para Debug? Si no es así, ¿por qué el autor realizó la comprobación de
string.IsNullOrEmpty (string) , por ejemplo. Es la pregunta a los autores del código.
El siguiente error es más evidente.
V3095 El objeto 'scriptArgsOpt' se usó antes de que se verificara como nulo. Líneas de verificación: 321, 325. CommonCommandLineParser.cs 321
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg);
Creo que este código no necesita ninguna explicación. Déjame darte la versión fija:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
En el código de Roslyn, hubo 15 errores más similares:
- V3095 El objeto 'LocalFunctions' se usó antes de que se verificara contra nulo. Líneas de verificación: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289
- V3095 El objeto 'resolution.OverloadResolutionResult' se usó antes de que se verificara como nulo. Líneas de verificación: 579, 588. Binder_Invocation.cs 579
- V3095 El objeto 'resolution.MethodGroup' se usó antes de que se verificara como nulo. Líneas de verificación: 592, 621. Binder_Invocation.cs 592
- V3095 El objeto 'TouchFilesLogger' se usó antes de que se verificara como nulo. Líneas de verificación: 111, 126. CSharpCompiler.cs 111
- V3095 El objeto 'newExceptionRegionsOpt' se usó antes de que se verificara contra nulo. Líneas de verificación: 736, 743. AbstractEditAndContinueAnalyzer.cs 736
- V3095 El objeto 'símbolo' se usó antes de que se verificara como nulo. Líneas de verificación: 422, 427. AbstractGenerateConstructorService.Editor.cs 422
- V3095 El objeto '_state.BaseTypeOrInterfaceOpt' se usó antes de que se verificara como nulo. Líneas de verificación: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132
- V3095 El objeto 'elemento' se usó antes de que se verificara como nulo. Líneas de verificación: 232, 233. ProjectUtil.cs 232
- V3095 El objeto 'idiomas' se usó antes de que se verificara como nulo. Líneas de verificación: 22, 28. ExportCodeCleanupProvider.cs 22
- V3095 El objeto 'memberType' se usó antes de que se verificara contra nulo. Líneas de verificación: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
- V3095 El objeto 'validTypeDeclarations' se usó antes de que se verificara contra nulo. Líneas de verificación: 223, 228. SyntaxTreeExtensions.cs 223
- V3095 El objeto 'texto' se usó antes de que se verificara como nulo. Líneas de verificación: 376, 385. MSBuildWorkspace.cs 376
- V3095 El objeto 'nameOrMemberAccessExpression' se usó antes de que se verificara como nulo. Líneas de verificación: 206, 223. CSharpGenerateTypeService.cs 206
- V3095 El objeto 'simpleName' se usó antes de que se verificara como nulo. Líneas de verificación: 83, 85. CSharpGenerateMethodService.cs 83
- V3095 El objeto 'opción' se usó antes de que se verificara como nulo. Líneas de verificación: 23, 28. OptionKey.cs 23
Consideremos los errores de
V3105 . Aquí, el operador
nulo condicional se usa al inicializar la variable, pero además la variable se usa sin verificaciones para
nulo .
Dos advertencias indican el siguiente error:
V3105 La variable 'documentId' se usó después de que se asignó a través del operador condicional nulo. NullReferenceException es posible. CodeLensReferencesService.cs 138
V3105 La variable 'documentId' se usó después de que se asignó a través del operador condicional nulo. NullReferenceException es posible. CodeLensReferencesService.cs 139
private static async Task<ReferenceLocationDescriptor> GetDescriptorOfEnclosingSymbolAsync(....) { .... var documentId = solution.GetDocument(location.SourceTree)?.Id; return new ReferenceLocationDescriptor( .... documentId.ProjectId.Id, documentId.Id, ....); }
La variable
documentId se puede inicializar por
nulo . Como resultado, la creación de un objeto
ReferenceLocationDescriptor generará una excepción. El código debe ser arreglado:
return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
Los desarrolladores también deben cubrir la posibilidad de que las variables, pasadas a un constructor, sean
nulas.Otros errores similares en el código:
- V3105 La variable 'símbolo' se usó después de que se asignó a través del operador condicional nulo. NullReferenceException es posible. SymbolFinder_Hierarchy.cs 44
- V3105 La variable 'símbolo' se usó después de que se asignó a través del operador condicional nulo. NullReferenceException es posible. SymbolFinder_Hierarchy.cs 51
Prioridades y paréntesisV3123 Quizás el operador '?:' Funciona de una manera diferente a la esperada. Su prioridad es menor que la prioridad de otros operadores en su condición. Edit.cs 70
public bool Equals(Edit<TNode> other) { return _kind == other._kind && (_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode) && (_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode); }
La condición en el bloque de retorno no se evalúa según lo previsto por el desarrollador. Se supuso que la primera condición será
_kind == other._kin d, (es por eso que después de esta condición hay un salto de línea), y después de eso, los bloques de condiciones con el operador "
? " Se evaluarán en secuencia. De hecho, la primera condición es
_kind == other._kind && (_oldNode == null) . Esto se debe al hecho de que el operador
&& tiene mayor prioridad que el operador "
? ". Para solucionar esto, un desarrollador debe tomar todas las expresiones del operador "
? " Entre paréntesis:
return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Con esto concluye mi descripción de los errores encontrados.
ConclusiónA pesar de la gran cantidad de errores, que logré encontrar, en términos del tamaño del código del proyecto Roslyn (2,770,000 líneas), no es demasiado. Como Andrey escribió en un artículo anterior, también estoy listo para reconocer la alta calidad de este proyecto.
Me gustaría señalar que tales comprobaciones de código ocasionales no tienen nada que ver con la metodología del análisis estático y son casi inútiles. El análisis estático debe aplicarse con regularidad, y no caso por caso. De esta forma, muchos errores se corregirán en las primeras etapas y, por lo tanto, el costo de corregirlos será diez veces menor. Esta idea se expone con más detalle en esta pequeña
nota , por favor, échale un vistazo.
Puede comprobar usted mismo algunos errores tanto en este proyecto como en otro. Para hacer esto, solo necesita
descargar y probar nuestro analizador.