De vez en cuando, volvemos a proyectos que previamente probamos con PVS-Studio y escribimos artículos al respecto. Hay dos razones para hacer esto. En primer lugar, para comprender cuánto mejor se ha vuelto nuestro analizador. En segundo lugar, para rastrear si los autores del proyecto prestaron atención a nuestro artículo, así como al informe de errores que generalmente les proporcionamos. Por supuesto, los errores pueden corregirse sin nuestra participación. Pero siempre es bueno cuando exactamente nuestros esfuerzos ayudan a mejorar un proyecto. Roslyn no fue la excepción. Un artículo de revisión anterior sobre este proyecto data del 23 de diciembre de 2015. Esto es bastante tiempo, dado el camino que nuestro analizador ha recorrido en su desarrollo durante este tiempo. Para nosotros personalmente, Roslyn también es de interés adicional por el hecho de que el núcleo del analizador C # PVS-Studio se basa en él. Por lo tanto, estamos muy interesados en la calidad del código para este proyecto. Organizaremos una segunda verificación y descubriremos qué hay de nuevo e interesante (pero esperemos que nada significativo) PVS-Studio pueda encontrar allí.
Roslyn (o la plataforma del compilador .NET) probablemente sea familiar para muchos de nuestros lectores. En resumen, es un conjunto de compiladores de código abierto y API para análisis de código para 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, pero recomiendo a todos los interesados en el artículo de mi colega Sergey Vasiliev "
Introducción a Roslyn. Uso de herramientas de análisis estático para desarrollar ". En este artículo, puede aprender no solo sobre 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 tres años desde la redacción del último artículo de mi colega Andrei Karpov sobre el cheque de Roslyn "
Lanzamiento de Año Nuevo de PVS-Studio 6.00: chequeo de Roslyn ". Durante este tiempo, el analizador C # PVS-Studio ha adquirido muchas características nuevas. En general, el artículo de Andrey era una especie de "bola de prueba", porque el analizador C # solo se agregó a PVS-Studio en ese momento. A pesar de esto, incluso entonces, en un proyecto incondicionalmente de alta calidad, Roslyn logró encontrar errores interesantes. ¿Qué ha cambiado en el analizador para el código C # hasta ahora, lo que potencialmente permitirá un análisis más profundo?
En el pasado, tanto el núcleo del analizador como la infraestructura se han desarrollado. Se agregó soporte para Visual Studio 2017 y Roslyn 2.0, así como una profunda integración con MSBuild. Puede leer más sobre nuestro enfoque de integración con MSBuild y sobre los motivos que nos llevaron a aceptarlo en el artículo de mi colega Pavel Yeremeyev, "
Soporte para Visual Studio 2017 y Roslyn 2.0 en PVS-Studio: a veces, usar soluciones preparadas no es tan fácil como parece de un vistazo ".
Ahora estamos trabajando activamente en la transición a Roslyn 3.0 de acuerdo con el mismo esquema que inicialmente admitimos Visual Studio 2017, es decir, a través de nuestro propio conjunto de herramientas, que viene con el "código auxiliar" en el kit de distribución PVS-Studio como un archivo MSBuild.exe vacío. A pesar de que parece una "muleta" (la API de MSBuild no es muy amigable para su reutilización en proyectos de terceros debido a la baja portabilidad de las bibliotecas), este enfoque ya nos ha ayudado a revivir varias actualizaciones de Roslyn sin problemas durante la vida de Visual Studio 2017. y ahora, aunque con muchas superposiciones, sobrevive a la actualización a Visual Studio 2019, así como también mantiene la compatibilidad y el rendimiento anteriores en sistemas con versiones anteriores de MSBuild.
El núcleo del analizador también ha experimentado una serie de mejoras. Una de las principales innovaciones es un análisis interprocedural completo, teniendo en cuenta los valores de entrada y salida de los métodos, teniendo en cuenta, dependiendo de estos parámetros, la accesibilidad de las ramas de ejecución y los puntos de retorno.
La tarea de rastrear parámetros dentro de los métodos ya está cerca de completarse, al tiempo que conserva las anotaciones automáticas de lo que sucede con estos parámetros allí (por ejemplo, una desreferenciación potencialmente peligrosa). Esto permitirá que cualquier diagnóstico que utilice el mecanismo de flujo de datos tenga en cuenta las situaciones peligrosas que se producen al pasar un parámetro a un método. Anteriormente, al analizar lugares tan peligrosos, no se generaba una advertencia, ya que no podíamos conocer todos los valores de entrada posibles para dicho método. Ahora podemos detectar el peligro, ya que en todos los lugares donde se llama a este método, estos parámetros de entrada se tendrán en cuenta.
Nota: puede familiarizarse con los mecanismos principales del analizador, como el flujo de datos y otros, del 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 por los parámetros de entrada o la profundidad. La única limitación son los métodos virtuales en clases que no se cerraron por herencia y cayeron en recursión (nos detendremos cuando veamos en la pila una llamada repetida a un método ya calculado). Además, el método recursivo en sí mismo se calculará en última instancia suponiendo que se desconoce el valor de retorno de su autorrecurrencia.
Otra gran innovación en el analizador C # es la posibilidad de desreferenciar un puntero potencialmente nulo. Anteriormente, el analizador juró una posible excepción de referencia nula si estaba seguro de que en todas las ramas de ejecución el valor de la variable sería nulo. Por supuesto, a veces se equivocaba, por lo que los diagnósticos del
V3080 se llamaban anteriormente referencia nula potencial.
Ahora el analizador recuerda que la variable podría ser nula en una de las ramas de ejecución (por ejemplo, bajo cierta condición en if). Si ve acceso a dicha variable sin verificar, recibirá un mensaje V3080, pero con un nivel de importancia menor que si ve nulo en todas las ramas. En combinación con un análisis interprocedural mejorado, dicho mecanismo permite encontrar errores muy difíciles de detectar. Un ejemplo es una larga cadena de llamadas a métodos, la última de las cuales no está familiarizado, y que, por ejemplo, devuelve nulo bajo ciertas circunstancias, pero no se protegió de esto porque simplemente no lo sabía. En este caso, el analizador solo jura cuando ve con precisión la asignación de nulo. En nuestra opinión, esto distingue cualitativamente nuestro enfoque de una innovación de C # 8.0 como el tipo de referencia anulable, que, de hecho, se reduce a establecer verificaciones nulas en cada método. Ofrecemos una alternativa: hacer comprobaciones solo donde realmente puede existir nulo, y nuestro analizador ahora puede buscar tales situaciones.
Entonces, sin demora, pasemos a "debriefing": analizar los resultados de la verificación de Roslyn. Primero, veamos los errores encontrados gracias a las innovaciones descritas anteriormente. En general, esta vez se emitieron bastantes advertencias para el código de Roslyn. Creo que esto se debe al hecho de que la plataforma se está desarrollando de manera muy activa (la base de código actualmente se encuentra en aproximadamente 2,770,000 líneas de código, excluyendo las vacías), y no hemos hecho un análisis de este proyecto durante mucho tiempo. Sin embargo, no hay tantos errores críticos, es decir, son de interés para el artículo. Y sí, hay bastantes pruebas en Roslyn que, como siempre, excluí de las pruebas.
Comenzaré con los errores V3080, con un nivel de criticidad medio, en el que el analizador detectó un posible acceso a través de un enlace cero, 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(....))
Considere el método
GetNode . El analizador considera que el acceso por referencia nula es posible en la condición del bloque
while . En el cuerpo del bloque
while , a la variable
actual se le asignará un valor, el resultado de la ejecución del método
AsNode . Y este valor en algunos casos será
nulo . Un buen ejemplo de análisis interprocedural en acción.
Ahora considere 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, devolverá el resultado del
método GetDirectoryName (string, bool) sobrecargado,
cuyo valor puede ser
nulo . Como más adelante en el cuerpo del método
ExpandFileNamePattern , la variable de
directorio se usa sin verificar primero la igualdad
nula , podemos hablar sobre la legitimidad de la advertencia por parte del analizador. Este es un diseño potencialmente inseguro.
Otra pieza de código con el error V3080, más precisamente, inmediatamente con dos errores emitidos para una línea de código. Aquí, el análisis interprocedural no era necesario.
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 de tipo
nullable int y se inicializan en
nulo . Además en el código se les pueden asignar valores, pero solo si se cumplen ciertas condiciones. En algunos casos, su valor seguirá siendo igual a
nulo . Además en el código, se accede a estas variables por referencia sin verificar primero la igualdad
nula , como lo indica el analizador.
El código de Roslyn contiene bastantes errores de este tipo, más de 100. A menudo, el patrón de estos errores es el mismo. Hay algún método general que potencialmente devuelve
nulo . El resultado de este método se utiliza en una gran cantidad de 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 a través de un enlace nulo. Y detectar tales errores es muy difícil. Por lo tanto, en algunos casos, debe considerar refactorizar el código, en el que se generaría una excepción si el método devuelve
nulo . De lo contrario, puede asegurar su código solo con verificaciones totales, lo cual es bastante tedioso y poco confiable. Por supuesto, en cada caso, la decisión debe tomarse sobre la base del proyecto.
Nota Sucede que en este momento no hay situaciones (datos de entrada) en las que el método devuelva
nulo y no haya un error real. Sin embargo, dicho código aún no es confiable, ya que todo puede cambiar cuando se realizan cambios en el código.
Para cerrar el tema con
V3080 , echemos un vistazo a los errores obvios con el nivel de confianza alto, cuando el acceso a través de un enlace nulo es 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 a un error tipográfico en la condición (en lugar del operador
|| que usamos
&& ), el código no funciona en absoluto según lo previsto, y se accederá a la variable
collectionType.Type si es
nulo . La condición debe corregirse de la siguiente manera:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
Por cierto, la segunda variante del desarrollo de eventos también es posible: en la primera parte, las condiciones están mezcladas por los operadores
== y
! = . Entonces el código corregido sería:
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 decisión final depende 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}'."); } .... }
Se produjo un error al redactar un mensaje para una excepción. Al mismo tiempo, se intenta acceder a la propiedad
action.DisplayText a través de la variable
action , que obviamente es
nula .
Y el último error es el alto nivel V3080.
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 pequeño, así que le doy su código completo. La condición en el bloque de
retorno es incorrecta. En algunos casos, es posible lanzar una
NullReferenceException al acceder a
type.FullName . Utilizo corchetes (no cambiarán el comportamiento aquí) para aclarar la situación:
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
Así es como, de acuerdo con la prioridad de las operaciones, este código funcionará. Si la variable de
tipo es
nula ,
ingresamos a una comprobación de otra cosa, donde, asegurándonos de que la variable
targetTypeName es
nula , usamos la referencia de
tipo nulo. Puede arreglar el código, por ejemplo, así:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Creo que ahí es donde puede completar el estudio de los errores V3080 y ver qué más interesante analizador PVS-Studio encontró en el código de Roslyn.
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 a un nombre de variable fallido, 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 usar el parámetro
sourceCodeKind . Para minimizar la probabilidad de tales errores, se recomienda que utilice el prefijo de subrayado para los nombres de los parámetros en tales casos. Daré 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; }
DescuidoV3006 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 esto no sucede, y el objeto de excepción simplemente se crea. Se omitió la palabra clave
throw . Versión corregida del código:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
El tema de trabajar con destructores en C # y lanzar excepciones de ellos es un tema para una discusión separada, que está más allá del alcance de este artículo.
Cuando el resultado no es importanteSe
han recibido una serie de advertencias
V3009 para los métodos que devuelven el mismo valor en todos los casos. A veces esto no es crítico, o el código de retorno simplemente no se verifica en el código de llamada. Me perdí tales advertencias. Pero algunas piezas de código me parecieron sospechosas. Citaré 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 solo
verdadero , y nada más que
verdadero . Al mismo tiempo, el valor de retorno está involucrado en algunas verificaciones en el código de llamada:
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
Es difícil decir cuán peligroso es este comportamiento. Pero si no se necesita el resultado, puede valer la pena reemplazar el tipo de retorno con void y realizar cambios mínimos en el método de llamada. Esto hará que el código sea más comprensible y seguro.
Otras advertencias 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
No verificadoV3019 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(); } .... }
El
valor de la variable es de tipo
NamingStylePreferences . El problema es seguir esta verificación. Incluso si la variable de
valor no es nula, esto no garantiza que la conversión de tipo haya sido exitosa y que
valueToSerialize no contenga
nulo . Se
puede lanzar una
NullReferenceException . El código debe repararse de la siguiente manera:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Y un error similar más.
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 es de tipo
ColumnState2 . Sin embargo, el resultado de la operación, la variable
columnState2 , ya no se verifica como
nulo . En cambio, la variable
columnState se verifica utilizando la declaración
nula condicional. ¿Cuál es el peligro de este código? Como en el ejemplo anterior, la conversión de tipos con el operador
as puede fallar, y la variable
columnState2 será
nula , lo que generará una excepción. Por cierto, un error tipográfico puede ser el culpable. Tenga en cuenta la condición en el bloque
if . Quizás en lugar de
columnState? .Name quisieran escribir
columnState2? .Name . Esto es muy probable dado los nombres de variables bastante desafortunados
columnState y columnState2.Cheques redundantesSe emitió una cantidad bastante grande de advertencias (más de 100) para construcciones no críticas, pero potencialmente inseguras, asociadas con verificaciones redundantes. Por ejemplo, daré 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)
Quizás no haya un error real aquí. Solo una buena razón para demostrar la combinación de tecnologías "análisis interprocedural + análisis de flujo de datos" en acción. El analizador considera que la segunda verificación
navInfo == null es redundante. De hecho, antes de eso, el valor para asignar
navInfo se obtendrá del método
libraryService.NavInfoFactory.CreateForProject , que construirá y devolverá un nuevo objeto de la clase
NavInfo . Pero no es
nulo de ninguna manera. La pregunta es, ¿por qué el analizador no
generó una advertencia para la primera verificación
navInfo == null ? Hay una explicación para esto. En primer lugar, si la variable del
símbolo resulta ser
nula , entonces el valor de
navInfo seguirá siendo una referencia nula. En segundo lugar, incluso si
navInfo obtiene el valor del método
libraryService.NavInfoFactory.CreateForSymbol , este valor puede ser
nulo . Por lo tanto, la primera comprobación de
navInfo == null es realmente necesaria.
No hay suficientes chequesY ahora la situación es lo contrario de lo anterior. Se
recibieron varias advertencias
V3042 para el código al que se podía acceder mediante una referencia nula. Además, solo uno o dos cheques pequeños podrían arreglarlo todo.
Considere una pieza interesante de código que contiene 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 esto porque usa el operador
nulo condicional en la condición del bloque
if para acceder al
receptor? .Sintaxis . Además en el código, el
receptor variable
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 debe asegurarse de que el constructor
BoundConditionalReceiver admite obtener valores
nulos para sus parámetros o 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 condiciónV3057 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); } .... }
Si la variable de
dos puntos es 0, lo que permite una condición en el código, el método
Substring arrojará una
excepción ArgumentOutOfRangeException . Corrección requerida:
if (colon > 0)
El error tipográfico es posibleEl 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; }); .... }
Se pasan dos parámetros a la expresión lambda: t1 y t2. Sin embargo, solo se usa t1. Parece sospechoso, considerando lo fácil que es cometer un error al usar variables con estos nombres.
DescuidoV3083 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 desencadena de forma insegura. Entre verificar la igualdad
nula y llamar a un evento, pueden tener tiempo para darse de baja y luego se lanzará una excepción. Además, en el cuerpo del bloque
if , inmediatamente antes de la llamada del evento, se realizan otras operaciones. Llamé a este error "Inatención", porque en otros lugares del código trabajan con este evento con mayor precisión, por ejemplo, así:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
El uso de la variable del
controlador opcional elimina el problema. En el método
OnTextBufferChanged , debe realizar cambios para la misma operación segura con el evento.
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, volveré a escribir este fragmento de código, reemplazando los nombres 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. La segunda parte (
si no ) se realizará solo para el rango de valores de 2147483648 + 1 a 4294967295.
Un par más de estas advertencias:
- 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 información sobre las verificaciones de igualdad nula (o la falta de ellas)Unos pocos errores
V3095 sobre la comprobación de una variable para
nulo después de usarlo. El primero es ambiguo, considere 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 de
tee puede ser nula. Para hacer esto, verifique
Debug.Assert . No está claro por qué va después de usar la cadena. También debe tenerse en
cuenta que para otras configuraciones que no sean Debug, el compilador
eliminará Debug.Assert del código. ¿Esto significa que solo para Debug es posible obtener una referencia nula? Y si esto no es así, ¿por qué no comprobó
string.IsNullOrEmpty (string) , por ejemplo. Estas son preguntas para los autores del código.
El siguiente error es más obvio.
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 requiere explicación. Daré la versión corregida:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
El código de Roslyn encontró otros 15 errores de este tipo:
- 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. Check lines: 223, 228. SyntaxTreeExtensions.cs 223
- V3095 The 'text' object was used before it was verified against null. Check lines: 376, 385. MSBuildWorkspace.cs 376
- V3095 The 'nameOrMemberAccessExpression' object was used before it was verified against null. Check lines: 206, 223. CSharpGenerateTypeService.cs 206
- V3095 The 'simpleName' object was used before it was verified against null. Check lines: 83, 85. CSharpGenerateMethodService.cs 83
- V3095 The 'option' object was used before it was verified against null. Check lines: 23, 28. OptionKey.cs 23
Considere los errores de V3105 . Aquí usamos el operador nulo condicional al inicializar la variable, y de aquí en adelante en el código se usa la variable sin verificar la igualdad nula .El siguiente error se señala inmediatamente mediante dos advertencias.V3105 La variable 'documentId' se usó después de que se asignó a través del operador condicional nulo. NullReferenceException es posible. CodeLensReferencesService.cs 138V3105 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 a nulo . Como resultado, la creación de ReferenceLocationDescriptor terminará arrojando una excepción. El código necesita ser reparado: return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
Además, más adelante en el código, es necesario prever la posibilidad de igualdad de variables nulas pasadas al constructor.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 corchetesV3123 Quizás el operador '?:' Funciona de una manera diferente a la esperada. Su prioridad es inferior a 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 calcula en absoluto como pensó el desarrollador. Se supuso que la primera condición sería _kind == other._kin d (por lo tanto, el ajuste de línea se realizó después de esta condición), y luego los bloques de condición con el operador " ? " Se calcularían secuencialmente . De hecho, la primera condición será _kind == other._kind && (_oldNode == null) . Esto se debe a que el operador && tiene una prioridad más alta que el operador " ? ". Para corregir el error, es necesario poner entre paréntesis todas las expresiones del operador " ? ": return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Esto concluye la descripción de los errores encontrados.ConclusionesA pesar de la cantidad significativa de errores que pude detectar, en términos del tamaño del código del proyecto Roslyn (2,770,000 líneas), esta será una cantidad bastante pequeña. Al igual que Andrei en el artículo anterior, también estoy listo para reconocer la alta calidad de este proyecto.Quiero señalar que tales comprobaciones de código ocasionales no tienen nada que ver con la metodología del análisis estático y prácticamente no aportan ningún beneficio. El análisis estático debe aplicarse regularmente, y no de un caso a otro. Luego, muchos errores se corregirán en las primeras etapas y, por lo tanto, el costo de corregirlos será diez veces menor. Esta idea se describe con más detalle en este pequeño artículo , con el que le pido que se familiarice.Puede buscar independientemente más errores tanto en el proyecto considerado como en cualquier otro. Para hacer esto, solo necesita descargar y probar nuestro analizador.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Sergey Khrenov. Comprobación del código fuente de Roslyn .