De temps en temps, nous revenons aux projets que nous avons précédemment vérifiés à l'aide de PVS-Studio, ce qui se traduit par leurs descriptions dans divers articles. Deux raisons rendent ces retours passionnants pour nous. Tout d'abord, l'occasion d'évaluer les progrès de notre analyseur. Deuxièmement, suivre les retours des auteurs du projet sur notre article et le rapport d'erreurs que nous leur fournissons généralement. Bien sûr, les erreurs peuvent être corrigées sans notre participation. Cependant, c'est toujours agréable lorsque nos efforts contribuent à améliorer un projet. Roslyn ne faisait pas exception. L'article précédent sur cette vérification de projet remonte au 23 décembre 2015. C'est assez long, compte tenu des progrès réalisés par notre analyseur depuis lors. Étant donné que le cœur C # de l'analyseur PVS-Studio est basé sur Roslyn, il nous donne un intérêt supplémentaire pour ce projet. En conséquence, nous tenons autant que la moutarde à la qualité du code de ce projet. Maintenant, testons-le à nouveau et découvrons quelques problèmes nouveaux et intéressants (mais espérons que rien de significatif) que PVS-Studio sera en mesure de trouver.
Beaucoup de nos lecteurs connaissent probablement Roslyn (ou .NET Compiler Platform). En bref, il s'agit d'un ensemble de compilateurs open source et d'une API pour l'analyse de code des langages C # et Visual Basic .NET de Microsoft. Le code source du projet est disponible sur
GitHub .
Je ne donnerai pas une description détaillée de cette plate-forme et je recommanderai de lire l'article de mon collègue Sergey Vasiliev "
Introduction à Roslyn et son utilisation dans le développement de programmes " à tous les lecteurs intéressés. À partir de cet article, vous pouvez découvrir non seulement les caractéristiques de l'architecture de Roslyn, mais aussi la façon dont nous utilisons exactement cette plate-forme.
Comme je l'ai mentionné plus tôt, cela fait plus de 3 ans que mon collègue Andrey Karpov a écrit le dernier article sur le chèque de Roslyn "
Nouvelle année PVS-Studio 6.00 Release: Scanning Roslyn ". Depuis lors, l'analyseur C # PVS-Studio avait de nombreuses nouvelles fonctionnalités. En fait, l'article d'Andrey était un cas de test, car à ce moment-là, l'analyseur C # venait d'être ajouté dans PVS-Studio. Malgré cela, nous avons réussi à détecter des erreurs dans le projet Roslyn, qui était certainement de haute qualité. Alors, qu'est-ce qui a changé dans l'analyseur de code C # à ce moment qui nous permettra d'effectuer une analyse plus approfondie?
Depuis lors, le cœur et l'infrastructure se sont développés. Nous avons ajouté la prise en charge de Visual Studio 2017 et Roslyn 2.0, ainsi qu'une intégration approfondie avec MSBuild. L'article de mon collègue Paul Eremeev "
Prise en charge de Visual Studio 2017 et Roslyn 2.0 dans PVS-Studio: parfois ce n'est pas si facile à utiliser des solutions prêtes à l'emploi que cela puisse paraître " décrit notre approche de l'intégration avec MSBuild et les raisons de cette décision.
Pour le moment, nous travaillons activement à passer à Roslyn 3.0 de la même manière que nous avons initialement pris en charge Visual Studio 2017. Cela nécessite l'utilisation de notre propre ensemble d'outils, inclus dans la distribution PVS-Studio en tant que "stub", qui est un MSBuild vide Fichier .exe. Malgré le fait qu'il ressemble à une "béquille" (l'API MSBuild n'est pas très conviviale pour la réutilisation dans des projets tiers en raison de la faible portabilité des bibliothèques), une telle approche nous a déjà aidé à surmonter relativement facilement plusieurs mises à jour de Roslyn en termes de Visual Studio 2017. Jusqu'à présent, il aidait (même avec certains défis) à passer par la mise à jour de Visual Studio 2019 et à maintenir une compatibilité descendante et des performances complètes pour les systèmes avec des versions MSBuild plus anciennes.
Le cœur de l'analyseur a également subi un certain nombre d'améliorations. L'une des principales caractéristiques est une analyse interprocédurale complète prenant en compte les valeurs des méthodes d'entrée et de sortie, évaluant (en fonction de ces paramètres) l'accessibilité des branches d'exécution et des points de retour.
Nous sommes sur le point de terminer la tâche de surveillance des paramètres à l'intérieur des méthodes (par exemple, les déréférences potentiellement dangereuses) et d'enregistrer leurs auto-annotations. Pour un diagnostic utilisant un mécanisme de flux de données, cela permettra de prendre en compte des situations dangereuses, survenant lors du passage d'un paramètre dans une méthode. Avant cela, lors de l'analyse de ces endroits dangereux, aucun avertissement n'était généré, car nous ne pouvions pas connaître toutes les valeurs d'entrée possibles dans une telle méthode. Maintenant, nous pouvons détecter le danger, comme dans tous les endroits où l'on appelle cette méthode, ces paramètres d'entrée seront pris en compte.
Remarque: vous pouvez lire sur les mécanismes de base de l'analyseur, tels que le flux de données et autres dans l'article "
Technologies utilisées dans l'analyseur de code PVS-Studio pour trouver des bogues et des vulnérabilités potentielles ".
L'analyse interprocédurale dans PVS-Studio C # n'est limitée ni par les paramètres d'entrée, ni par la profondeur. La seule limitation réside dans les méthodes virtuelles dans les classes, ouvertes à l'héritage, ainsi que dans la récursivité (l'analyse s'arrête lorsqu'elle tombe sur un appel répété de la méthode déjà évaluée). Ce faisant, la méthode récursive elle-même sera finalement évaluée en supposant que la valeur de retour de sa récursivité est inconnue.
Une autre grande nouvelle fonctionnalité de l'analyseur C # est devenue la prise en compte d'une éventuelle déréférence d'un pointeur potentiellement nul. Avant cela, l'analyseur s'est plaint d'une éventuelle exception de référence nulle, étant assuré que dans toutes les branches d'exécution, la valeur de la variable serait nulle. Bien sûr, c'était faux dans certains cas, c'est pourquoi le diagnostic du
V3080 avait précédemment appelé référence nulle potentielle.
L'analyseur est maintenant conscient du fait que la variable peut être nulle dans l'une des branches d'exécution (par exemple, sous une certaine condition
if ). S'il remarque l'accès à une telle variable sans vérification, il émettra l'avertissement V3080, mais à un niveau de certitude inférieur, que s'il voit null dans toutes les branches. Parallèlement à l'analyse interprocédurale améliorée, un tel mécanisme permet de trouver des erreurs très difficiles à détecter. Voici un exemple - imaginez une longue chaîne d'appels de méthode, dont le dernier ne vous est pas familier. Dans certaines circonstances, il retourne null dans le bloc
catch , mais vous ne l'avez pas protégé, car vous ne le savez pas. Dans ce cas, l'analyseur ne se plaint que lorsqu'il voit exactement l'affectation nulle. À notre avis, il distingue qualitativement notre approche de cette fonctionnalité de C # 8.0 en tant que référence de type nullable, ce qui, en fait, se limite à définir des contrôles pour null pour chaque méthode. Cependant, nous suggérons l'alternative - d'effectuer des vérifications uniquement dans les endroits où la valeur null peut réellement se produire, et notre analyseur peut désormais rechercher de tels cas.
Alors, ne retardons pas trop longtemps le point principal et passons au blâme - en analysant les résultats du contrôle Roslyn. Tout d'abord, considérons les erreurs trouvées en raison des fonctionnalités décrites ci-dessus. En somme, il y a eu beaucoup d'avertissements pour le code Roslyn cette fois. Je pense que cela est lié au fait que la plate-forme évolue très activement (à ce stade, la base de code est d'environ 2 770 000 lignes hors vide), et nous n'avons pas analysé ce projet depuis longtemps. Néanmoins, il n'y a pas autant d'erreurs critiques, alors qu'elles présentent le plus d'intérêt pour l'article. Comme d'habitude, j'ai exclu les tests du chèque, il y en a beaucoup à Roslyn.
Je vais commencer par les erreurs V3080 de niveau de certitude Moyen, dans lesquelles l'analyseur a détecté un accès possible par référence nulle, mais pas dans tous les cas possibles (branches de code).
Déréférence nulle possible - MoyenneV3080 Déréférence nulle possible. Envisagez d'inspecter «actuel». CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70
private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....))
Prenons la méthode
GetNode . L'analyseur suggère que l'accès par référence nulle est possible dans l'état du bloc
while . La variable reçoit une valeur dans le corps du bloc
while , qui est le résultat de la méthode
AsNode . Dans certains cas, cette valeur sera
nulle . Un bon exemple d'analyse interprocédurale en action.
Considérons maintenant un cas similaire, dans lequel l'analyse interprocédurale a été effectuée via deux appels de méthode.
V3080 Déréférence nulle possible. Pensez à inspecter le «répertoire». CommonCommandLineParser.cs 911
private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ?
La variable de
répertoire dans le corps de la méthode
ExpandFileNamePattern obtient la valeur de la méthode
GetDirectoryName (chaîne) . Cela, à son tour, renvoie le résultat de la méthode surchargée
GetDirectoryName (chaîne, bool) dont la valeur peut être
nulle . Étant donné que le
répertoire des variables est utilisé sans vérification préalable de null dans le corps de la méthode
ExpandFileNamePattern - nous pouvons proclamer que l'analyseur a correctement émis l'avertissement. Il s'agit d'une construction potentiellement dangereuse.
Un autre fragment de code avec l'erreur V3080, plus précisément, avec deux erreurs, émis pour une seule ligne de code. L'analyse interprocédurale n'était pas nécessaire ici.
V3080 Déréférence nulle possible. Pensez à inspecter «spanStartLocation». TestWorkspace.cs 574
V3080 Déréférence nulle possible. Pensez à inspecter «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,
Les variables
spanStartLocation et
spanEndLocationExclusive sont de type
int nullable et sont initialisées par
null . Plus loin dans le code, des valeurs peuvent leur être attribuées, mais uniquement sous certaines conditions. Dans certains cas, leur valeur reste
nulle . Après cela, ces variables sont accessibles par référence sans vérification préalable de null, ce que l'analyseur indique.
Le code Roslyn contient un grand nombre de ces erreurs, plus de 100. Souvent, le modèle de ces erreurs est le même. Il existe une sorte de méthode générale qui retourne potentiellement
null . Le résultat de cette méthode est utilisé à de nombreux endroits, parfois par le biais de dizaines d'appels de méthode intermédiaires ou de vérifications supplémentaires. Il est important de comprendre que ces erreurs ne sont pas fatales, mais qu'elles peuvent potentiellement conduire à un accès par référence nulle. Tout en détectant de telles erreurs est assez difficile. C'est pourquoi dans certains cas, il faut envisager une refactorisation de code, auquel cas si la valeur
null est renvoyée, la méthode lève une exception. Sinon, vous ne pouvez sécuriser votre code qu'avec des vérifications générales, ce qui est assez fatigant et parfois peu fiable. Quoi qu'il en soit, il est clair que chaque cas spécifique nécessite une solution basée sur les spécifications du projet.
Remarque Il se trouve qu'à un moment donné, il n'y a pas de tels cas (données d'entrée), lorsque la méthode retourne
null et qu'il n'y a pas d'erreur réelle. Cependant, un tel code n'est toujours pas fiable, car tout peut changer lors de l'introduction de certaines modifications de code.
Afin d'abandonner le sujet
V3080 , regardons les erreurs évidentes de niveau de haute certitude, lorsque l'accès par référence nulle est le plus probable, voire inévitable.
Déréférence nulle possible - ÉlevéV3080 Déréférence nulle possible. Pensez à inspecter '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; } .... }
En raison de la faute de frappe dans la condition (
&& est utilisé à la place de l'opérateur
|| ), le code fonctionne différemment que prévu et l'accès à la variable
collectionType.Type sera exécuté lorsqu'il est
nul . La condition doit être corrigée comme suit:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
Soit dit en passant, les choses peuvent se dérouler d'une autre manière: dans la première partie de la condition, les opérateurs
== et
! = Sont foirés
. Ensuite, le code correct ressemblerait à ceci:
if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....
Cette version du code est moins logique, mais corrige également l'erreur. La solution finale appartient aux auteurs du projet de décider.
Une autre erreur similaire.
V3080 Déréférence nulle possible. Envisagez d'inspecter «l'action». 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}'."); } .... }
L'erreur se produit lors de la génération du message pour l'exception. Elle est suivie de la tentative d'accès à la propriété
action.DisplayText via la variable d'
action , connue pour être
nulle .
Voici la dernière erreur V3080 du niveau haut.
V3080 Déréférence nulle possible. Envisagez d'inspecter le «type». ObjectFormatterHelpers.cs 91
private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }
La méthode est assez petite, donc je la cite entièrement. La condition dans le bloc de
retour est incorrecte. Dans certains cas, lors de l'accès à
type.FullName , une exception peut se produire. Je vais utiliser des parenthèses pour que ce soit clair (ils ne changeront pas le comportement):
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
Selon la priorité des opérations, le code fonctionnera exactement comme ceci. Dans le cas où la variable de
type est
nulle , nous tomberons dans le else-check, où nous utiliserons la référence de
type null, après avoir vérifié la variable
targetTypeName pour
null . Le code peut être fixé, par exemple, comme suit:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Je pense que c'est suffisant pour revoir les erreurs du V3080. Il est maintenant grand temps de voir d'autres choses intéressantes que l'analyseur PVS-Studio a réussi à trouver.
TypoV3005 La variable 'SourceCodeKind' est assignée à elle-même. DynamicFileInfo.cs 17
internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind;
En raison de l'échec du nommage des variables, une faute de frappe a été effectuée dans le constructeur de la classe
DynamicFileInfo . Le champ
SourceCodeKind se voit attribuer sa propre valeur au lieu d'utiliser le paramètre
sourceCodeKind . Pour minimiser la probabilité de telles erreurs, nous vous recommandons d'utiliser le préfixe de soulignement pour les noms de paramètres dans de tels cas. Voici un exemple d'une version corrigée du code:
public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }
Par inadvertanceV3006 L'objet a été créé mais il n'est pas utilisé. Le mot clé 'throw' peut être manquant: lancez une nouvelle exception InvalidOperationException (FOO). ProjectBuildManager.cs 61
~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
Sous une certaine condition, le destructeur doit lever une exception, mais cela ne se produit pas pendant que l'objet d'exception est simplement créé. Le mot clé
throw a été manqué. Voici la version corrigée du code:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
Le problème avec les destructeurs en C # et leur levée d'exceptions est un sujet pour une autre discussion, qui dépasse le cadre de cet article.
Quand le résultat n'est pas importantLes méthodes, qui ont reçu la même valeur dans tous les cas, ont déclenché un certain nombre d'avertissements
V3009 . Dans certains cas, cela peut ne pas être critique ou la valeur de retour n'est tout simplement pas vérifiée dans le code appelant. J'ai ignoré ces avertissements. Mais quelques extraits de code semblaient suspects. En voici un:
V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. GoToDefinitionCommandHandler.cs 62
internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }
La méthode
TryExecuteCommand ne renvoie que
true . Ce faisant, dans le code appelant, la valeur retournée est impliquée dans certains contrôles.
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
Il est difficile de dire exactement dans quelle mesure un tel comportement est dangereux. Mais si le résultat n'est pas nécessaire, peut-être que le type de la valeur de retour devrait être changé en void et que l'on devrait faire de petites modifications dans la méthode d'appel. Cela rendra le code plus lisible et sécurisé.
Avertissements similaires de l'analyseur:
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. CommentUncommentSelectionCommandHandler.cs 86
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. JsonRpcClient.cs 138
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. AbstractFormatEngine.OperationApplier.cs 164
- V3009 Il est étrange que cette méthode retourne toujours une seule et même valeur de 'false'. TriviaDataFactory.CodeShapeAnalyzer.cs 254
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. ObjectList.cs 173
- V3009 Il est étrange que cette méthode renvoie toujours une seule et même valeur de 'true'. ObjectList.cs 249
Vérifié la mauvaise choseV3019 Il est possible qu'une variable incorrecte soit comparée à null après la conversion de type à l'aide du mot clé 'as'. Vérifiez les 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
valeur est
convertie en type
NamingStylePreferences . Le problème réside dans la vérification qui suit. Même si la variable de
valeur n'était pas nulle, cela ne garantit pas que le transtypage de type a réussi et
valueToSerialize ne contient pas
null . Possibilité de
lever l'exception
NullReferenceException . Le code doit être corrigé comme suit:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Un autre bug similaire:
V3019 Il est possible qu'une variable incorrecte soit comparée à null après la conversion de type à l'aide du mot clé 'as'. Vérifiez les variables 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181
private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name ==
La variable
columnState est
convertie en type
ColumnState2 . Cependant, le résultat de l'opération, qui est la variable
columnState2, n'est plus vérifié pour
null . Au lieu de cela, la variable
columnState est vérifiée à l'aide de l'opérateur
null conditionnel. Pourquoi ce code est-il dangereux? Tout comme dans l'exemple précédent, la conversion avec l'opérateur
as peut échouer et la variable sera
nulle, ce qui entraînera une exception. Soit dit en passant, une faute de frappe peut être à blâmer ici. Jetez un œil à la condition dans le bloc
if .
Peut-être, au lieu de
columnState? .Name, l'auteur voulait écrire
columnState2? .Name . C'est très probable, compte tenu des noms de variable plutôt défectueux
columnState et
columnState2.Contrôles redondantsUn grand nombre d'avertissements (plus de 100) ont été émis sur des constructions non critiques, mais potentiellement dangereuses, liées à des contrôles redondants. Par exemple, c'est l'un d'entre eux.
V3022 L' expression 'navInfo == null' est toujours fausse. 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)
Il se peut qu'il n'y ait pas de bug réel ici. C'est juste une bonne raison de démontrer que "l'analyse interprocédurale + l'analyse du flux de données" fonctionne en remorque. L'analyseur suggère que la deuxième vérification
navInfo == null est redondante. En effet, avant cela, la valeur affectée à
navInfo sera obtenue à partir de la méthode
libraryService.NavInfoFactory.CreateForProject , qui va construire et retourner un nouvel objet de la classe
NavInfo . En aucun cas, il retournera
null . Ici, la question se pose, pourquoi l'analyseur n'a-t-il pas émis d'avertissement pour la première vérification
navInfo == null ? Il y a quelques raisons. Premièrement, si la variable de
symbole est
nulle , la valeur
navInfo restera également une référence nulle. Deuxièmement, même si
navInfo obtient la valeur de la méthode
ibraryService.NavInfoFactory.CreateForSymbol , cette valeur peut également être
nulle . Ainsi, la première vérification
navInfo == null est vraiment nécessaire.
Contrôles insuffisantsMaintenant, la situation inverse de la discussion ci-dessus. Plusieurs avertissements
V3042 ont été déclenchés pour le code, dans lesquels l'accès par référence nulle est possible. Même un ou deux petits chèques auraient pu tout réparer.
Prenons un autre fragment de code intéressant, qui contient deux de ces erreurs.
V3042 Exception NullReferenceException possible. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'récepteur' Binder_Expressions.cs 7770
V3042 Exception NullReferenceException possible. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'récepteur' Binder_Expressions.cs 7776
private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax !=
La variable
récepteur peut être nulle. L'auteur du code le sait, car il utilise l'opérateur
nul conditionnel dans la condition du bloc
if pour accéder au
récepteur ? .
Syntaxe . De plus, la variable
receiver est utilisée sans aucun contrôle pour accéder à
receiver.Type ,
receiver.Syntax et
receiver.HasErrors . Ces erreurs doivent être corrigées:
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; }
Nous devons également être sûrs que le constructeur prend en charge l'obtention de valeurs
nulles pour ses paramètres ou nous devons effectuer un refactoring supplémentaire.
Autres erreurs similaires:
- V3042 Exception NullReferenceException possible. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'containerType' SyntaxGeneratorExtensions_Negate.cs 240
- V3042 Exception NullReferenceException possible. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'expression' ExpressionSyntaxExtensions.cs 349
- V3042 Exception NullReferenceException possible. Le '?.' et '.' les opérateurs sont utilisés pour accéder aux membres de l'objet 'expression' ExpressionSyntaxExtensions.cs 349
Erreur dans la conditionV3057 La fonction 'Sous-chaîne' pourrait recevoir la valeur '-1' alors qu'une valeur non négative est attendue. Inspectez le deuxième argument. CommonCommandLineParser.cs 109
internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }
Dans le cas où la variable
deux -
points est 0, ce qui est
correct selon la condition dans le code, la méthode
Substring lèvera une exception. Cela doit être corrigé:
if (colon > 0)
Faute de frappe possibleV3065 Le paramètre 't2' n'est pas utilisé dans le corps de la méthode. 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; }); .... }
L'expression lambda accepte deux paramètres: t1 et t2. Cependant, seul t1 est utilisé. Cela semble suspect, compte tenu du fait qu'il est facile de faire une erreur lors de l'utilisation de variables avec de tels noms.
Par inadvertanceV3083 Invocation non
sûre de l'événement 'TagsChanged', NullReferenceException est possible. Pensez à affecter un événement à une variable locale avant de l'invoquer. PreviewUpdater.Tagger.cs 37
public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span));
L'événement
TagsChanged est invoqué de manière non sécurisée. Entre la vérification de
null et l'invocation de l'événement, quelqu'un peut se désinscrire de celui-ci, puis une exception sera levée. De plus, d'autres opérations sont effectuées dans le corps du bloc
if juste avant d'invoquer l'événement. J'ai appelé cette erreur "Inadvertence", car cet événement est géré plus soigneusement dans d'autres endroits, comme suit:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
L'utilisation d'une variable de
gestionnaire supplémentaire évite le problème. Dans la méthode
OnTextBufferChanged, il faut effectuer des modifications afin de gérer l'événement en toute sécurité.
Gammes d'intersectionV3092 Les intersections de plage sont possibles dans les expressions conditionnelles. Exemple: if (A> 0 && A <5) {...} else 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 { .... } }
Pour une meilleure compréhension, permettez-moi de réécrire ce code, en changeant les noms des constantes avec leurs valeurs réelles:
internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }
Probablement, il n'y a pas de véritable erreur, mais la condition semble étrange. Sa deuxième partie (
sinon si ) sera exécutée uniquement pour la plage de 2147483648 + 1 à 4294967295.
Un autre couple d'avertissements similaires:
- V3092 Les intersections de plage sont possibles dans les expressions conditionnelles. Exemple: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 109
- V3092 Les intersections de plage sont possibles dans les expressions conditionnelles. Exemple: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 66
En savoir plus sur les contrôles pour null (ou leur absence)Quelques erreurs
V3095 sur la vérification d'une variable pour null juste après son utilisation. Le premier est ambigu, considérons le code.
V3095 L'objet 'displayName' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 498, 503. FusionAssemblyIdentity.cs 498
internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }
Il est supposé que la référence
displayName peut être nulle. Pour cela, la vérification
Debug.Assert a été effectuée. On ne sait pas pourquoi il va après avoir utilisé une chaîne. Il faut également tenir compte du fait que pour les configurations différentes de Debug, le compilateur supprimera
Debug.Assert . Cela signifie-t-il que l'obtention d'une référence nulle n'est possible que pour le débogage? Si ce n'est pas le cas, pourquoi l'auteur a-t-il effectué la vérification de
string.IsNullOrEmpty (string) , par exemple. C'est la question aux auteurs du code.
L'erreur suivante est plus évidente.
V3095 L'objet 'scriptArgsOpt' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 321, 325. CommonCommandLineParser.cs 321
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg);
Je pense que ce code n'a pas besoin d'explications. Permettez-moi de vous donner la version fixe:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
Dans le code Roslyn, il y avait 15 autres erreurs similaires:
- V3095 L'objet 'LocalFunctions' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289
- V3095 L'objet 'resolution.OverloadResolutionResult' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 579, 588. Binder_Invocation.cs 579
- V3095 L'objet 'resolution.MethodGroup' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 592, 621. Binder_Invocation.cs 592
- V3095 L'objet 'touchedFilesLogger' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 111, 126. CSharpCompiler.cs 111
- V3095 L'objet 'newExceptionRegionsOpt' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 736, 743. AbstractEditAndContinueAnalyzer.cs 736
- V3095 L'objet 'symbole' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 422, 427. AbstractGenerateConstructorService.Editor.cs 422
- V3095 L'objet '_state.BaseTypeOrInterfaceOpt' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132
- V3095 L'objet 'element' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 232, 233. ProjectUtil.cs 232
- V3095 L'objet 'languages' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 22, 28. ExportCodeCleanupProvider.cs 22
- V3095 L'objet 'memberType' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
- V3095 L'objet 'validTypeDeclarations' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 223, 228. SyntaxTreeExtensions.cs 223
- V3095 L'objet 'texte' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 376, 385. MSBuildWorkspace.cs 376
- V3095 L'objet 'nameOrMemberAccessExpression' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 206, 223. CSharpGenerateTypeService.cs 206
- V3095 L'objet 'simpleName' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 83, 85. CSharpGenerateMethodService.cs 83
- V3095 L'objet 'option' a été utilisé avant d'être vérifié par rapport à null. Vérifiez les lignes: 23, 28. OptionKey.cs 23
Prenons les erreurs
V3105 . Ici, l'opérateur
nul conditionnel est utilisé lors de l'initialisation de la variable, mais en outre, la variable est utilisée sans vérification de
null .
Deux avertissements indiquent l'erreur suivante:
V3105 La variable 'documentId' a été utilisée après son affectation via un opérateur conditionnel nul. NullReferenceException est possible. CodeLensReferencesService.cs 138
V3105 La variable 'documentId' a été utilisée après son affectation via un opérateur conditionnel nul. NullReferenceException est possible. 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 peut être initialisée par
null . Par conséquent, la création d'un objet
ReferenceLocationDescriptor entraînera la levée d'une exception. Le code doit être fixe:
return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
Les développeurs devraient également couvrir la possibilité que les variables, passées à un constructeur, soient
nulles.Autres erreurs similaires dans le code:
- V3105 La variable 'symbol' a été utilisée après avoir été affectée via un opérateur conditionnel nul. NullReferenceException est possible. SymbolFinder_Hierarchy.cs 44
- V3105 La variable 'symbol' a été utilisée après avoir été affectée via un opérateur conditionnel nul. NullReferenceException est possible. SymbolFinder_Hierarchy.cs 51
Priorités et parenthèsesV3123 L'opérateur '?:'
Fonctionne peut-être d'une manière différente de celle attendue. Sa priorité est inférieure à la priorité des autres opérateurs dans son état. 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 condition dans le bloc de retour n'est pas évaluée comme prévu par le développeur. Il a été supposé que la première condition sera
_kind == other._kin d, (c'est pourquoi après cette condition il y a un saut de ligne), et ensuite que les blocs de conditions avec l'opérateur "
? " Seront évalués en séquence. En fait, la première condition est
_kind == other._kind && (_oldNode == null) . Cela est dû au fait que l'opérateur
&& a une priorité plus élevée que l'opérateur "
? ". Pour résoudre ce problème, un développeur doit prendre toutes les expressions de l'opérateur "
? " Entre parenthèses:
return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Ceci conclut ma description des erreurs trouvées.
ConclusionMalgré le grand nombre d'erreurs, que j'ai réussi à trouver, en termes de taille du code du projet Roslyn (2 770 000 lignes), ce n'est pas trop. Comme Andrey l'a écrit dans un article précédent, je suis également prêt à reconnaître la haute qualité de ce projet.
Je voudrais noter que de telles vérifications de code occasionnelles n'ont rien à voir avec la méthodologie de l'analyse statique et sont presque inutiles. L'analyse statique doit être appliquée régulièrement, et non au cas par cas. De cette façon, de nombreuses erreurs seront corrigées dès les premières étapes, et donc le coût de leur correction sera dix fois moins élevé. Cette idée est présentée plus en détail dans cette petite
note , veuillez la vérifier.
Vous pouvez vérifier vous-même quelques erreurs dans ce projet et dans un autre. Pour ce faire, il vous suffit de
télécharger et d'essayer notre analyseur.