De temps en temps, nous revenons à des projets que nous avions précédemment testés avec PVS-Studio et avons écrit des articles à ce sujet. Il y a deux raisons à cela. Tout d'abord, pour comprendre à quel point notre analyseur est devenu meilleur. Deuxièmement, pour vérifier si les auteurs du projet ont prêté attention à notre article, ainsi qu'au rapport d'erreur que nous leur fournissons habituellement. Bien sûr, les erreurs peuvent être corrigées sans notre participation. Mais c'est toujours agréable quand exactement nos efforts contribuent à améliorer un projet. Roslyn ne faisait pas exception. Un précédent article de synthèse sur ce projet remonte au 23 décembre 2015. C'est assez long, compte tenu du chemin parcouru par notre analyseur dans son développement pendant cette période. Pour nous personnellement, Roslyn présente également un intérêt supplémentaire du fait que le cœur de l'analyseur C # PVS-Studio est basé sur celui-ci. Par conséquent, nous sommes très intéressés par la qualité du code de ce projet. Nous organiserons un deuxième contrôle et découvrirons ce qui est nouveau et intéressant (mais espérons que rien d'important) PVS-Studio puisse y trouver.
Roslyn (ou la plate-forme du compilateur .NET) est probablement familier à beaucoup de nos lecteurs. En bref, il s'agit d'un ensemble de compilateurs open source et d'API pour l'analyse de code pour les 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, mais je recommanderais à tous ceux qui sont intéressés un article de mon collègue Sergey Vasiliev, "
Introduction à Roslyn. Utiliser des outils d'analyse statique pour développer ". À partir de cet article, vous pouvez en apprendre non seulement sur les fonctionnalités de l'architecture Roslyn, mais également sur la façon dont nous utilisons exactement cette plate-forme.
Comme je l'ai mentionné plus tôt, plus de trois ans se sont écoulés depuis la rédaction du dernier article de mon collègue Andrei Karpov sur le chèque de Roslyn "
Sortie du Nouvel An de PVS-Studio 6.00: vérification de Roslyn ". Pendant ce temps, l'analyseur C # PVS-Studio a acquis de nombreuses nouvelles fonctionnalités. En général, l'article d'Andrey était une sorte de «test ball», car l'analyseur C # n'était alors ajouté qu'à PVS-Studio. Malgré cela, même alors, dans un projet de qualité inconditionnelle, Roslyn a réussi à trouver des erreurs intéressantes. Qu'est-ce qui a changé dans l'analyseur pour le code C # jusqu'à présent, ce qui permettra potentiellement une analyse plus approfondie?
Au cours des dernières années, le cœur de l'analyseur et l'infrastructure se sont développés. Un support a été ajouté pour Visual Studio 2017 et Roslyn 2.0, ainsi qu'une intégration approfondie avec MSBuild. Vous pouvez en savoir plus sur notre approche de l'intégration avec MSBuild et sur les raisons qui nous ont amenés à l'accepter dans l'article de mon collègue Pavel Yeremeyev, "
Prise en charge de Visual Studio 2017 et Roslyn 2.0 dans PVS-Studio: parfois, l'utilisation de solutions toutes faites n'est pas aussi simple qu'il n'y paraît en un coup d'œil . "
Maintenant, nous travaillons activement sur la transition vers Roslyn 3.0 selon le même schéma que nous avons initialement pris en charge Visual Studio 2017, c'est-à-dire via notre propre ensemble d'outils, qui est fourni dans le kit de distribution PVS-Studio avec un «stub» sous la forme d'un fichier MSBuild.exe vide. Malgré le fait qu'elle ressemble à une "béquille" (l'API MSBuild n'est pas très conviviale pour une réutilisation dans des projets de troisième génération en raison de la faible portabilité des bibliothèques), cette approche nous a déjà aidé à revivre plusieurs mises à jour de Roslyn relativement sans douleur pendant la durée de Visual Studio 2017. et maintenant, bien qu'avec de nombreuses superpositions, survivez à la mise à niveau vers Visual Studio 2019, tout en conservant une compatibilité descendante et des performances complètes sur les systèmes avec des versions plus anciennes de MSBuild.
Le cœur de l'analyseur a également subi un certain nombre d'améliorations. L'une des principales innovations est une analyse interprocédurale à part entière, prenant en compte les valeurs d'entrée et de sortie des méthodes, prenant en compte, en fonction de ces paramètres, l'accessibilité des branches d'exécution et des points de retour.
La tâche de suivi des paramètres à l'intérieur des méthodes est déjà presque terminée, tout en préservant les annotations automatiques de ce qui se passe avec ces paramètres (par exemple, un déréférencement potentiellement dangereux). Cela permettra à tout diagnostic utilisant le mécanisme de flux de données de prendre en compte les situations dangereuses qui se produisent lors du passage d'un paramètre à une méthode. Auparavant, 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 pour une telle méthode. Nous pouvons maintenant détecter le danger, car dans tous les endroits où cette méthode est appelée, ces paramètres d'entrée seront pris en compte.
Remarque: vous pouvez vous familiariser avec les principaux mécanismes de l'analyseur, tels que le flux de données et autres, à partir de l'article "
Technologies utilisées dans l'analyseur de code PVS-Studio pour trouver les erreurs et les vulnérabilités potentielles ".
L'analyse interprocédurale dans PVS-Studio C # n'est pas limitée par les paramètres d'entrée ou la profondeur. La seule limitation est les méthodes virtuelles dans les classes qui n'ont pas été fermées pour l'héritage et qui tombent dans la récursivité (arrêtons quand nous avons vu sur la pile un appel répété à une méthode déjà calculée). De plus, la méthode récursive elle-même sera finalement calculée en supposant que la valeur de retour de son auto-récursivité est inconnue.
Une autre grande innovation dans l'analyseur C # est la possibilité de déréférencer un pointeur potentiellement nul. Auparavant, l'analyseur jurait à une éventuelle exception de référence nulle s'il était sûr que dans toutes les branches d'exécution, la valeur de la variable serait nulle. Bien sûr, il se trompait parfois, de sorte que le diagnostic du
V3080 était auparavant appelé référence nulle potentielle.
L'analyseur se souvient maintenant que la variable peut être nulle dans l'une des branches d'exécution (par exemple, sous une certaine condition dans if). S'il voit l'accès à une telle variable sans vérification, il recevra un message V3080, mais à un niveau d'importance moindre que s'il voit null dans toutes les branches. En combinaison avec une analyse interprocédurale améliorée, un tel mécanisme permet de trouver des erreurs très difficiles à détecter. Un exemple est une longue chaîne d'appels de méthode, dont le dernier n'est pas familier et qui, par exemple, renvoie null dans certaines circonstances, mais vous ne vous en êtes pas protégé car vous ne le saviez tout simplement pas. Dans ce cas, l'analyseur ne jure que lorsqu'il voit précisément l'affectation de null. À notre avis, cela distingue qualitativement notre approche d'une telle innovation de C # 8.0 en tant que type de référence nullable, qui, en fait, se résume à définir des contrôles nuls dans chaque méthode. Nous offrons une alternative - pour faire des vérifications uniquement là où la valeur nulle peut vraiment arriver, et notre analyseur est maintenant capable de rechercher de telles situations.
Alors, sans plus tarder, passons au «débriefing» - analysant les résultats du contrôle Roslyn. Voyons d'abord les erreurs trouvées grâce aux innovations décrites ci-dessus. En général, plusieurs avertissements ont été émis pour le code Roslyn cette fois. Je pense que cela est dû au fait que la plate-forme se développe très activement (la base de code s'élève actuellement à environ 2 770 000 lignes de code, à l'exclusion des lignes vides), et nous n'avons pas fait d'analyse de ce projet depuis longtemps. Cependant, il n'y a pas autant d'erreurs critiques, à savoir qu'elles intéressent l'article. Et oui, il y a pas mal de tests à Roslyn que j'ai, comme d'habitude, exclus des tests.
Je vais commencer par les erreurs V3080, avec un niveau de criticité moyen, dans lequel l'analyseur a détecté un accès possible via une liaison 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(....))
Considérez la méthode
GetNode . L'analyseur considère que l'accès par référence nulle est possible dans l'état du bloc
while . Dans le corps du bloc
while , la variable
courante se verra attribuer une valeur - le résultat de l'exécution de la méthode
AsNode . Et cette valeur dans certains cas 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, renverra le résultat de la
méthode GetDirectoryName surchargée
(chaîne, bool) , dont la valeur peut être
nulle . Étant donné que plus loin dans le corps de la méthode
ExpandFileNamePattern , la variable de
répertoire est utilisée sans vérifier d'abord
l' égalité
nulle , nous pouvons parler de la légitimité de l'avertissement par l'analyseur. Il s'agit d'une conception potentiellement dangereuse.
Un autre morceau de code avec l'erreur V3080, plus précisément, immédiatement avec deux erreurs émises pour une ligne de code. Ici, l'analyse interprocédurale n'était pas nécessaire.
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
nullable int et sont initialisées à
null . Plus loin dans le code, des valeurs peuvent leur être attribuées, mais uniquement si certaines conditions sont remplies. Dans certains cas, leur valeur restera égale à
null . Plus loin dans le code, ces variables sont accessibles par référence sans d'abord vérifier
l' égalité
nulle , comme l'indique l'analyseur.
Le code Roslyn contient un certain nombre de ces erreurs, plus de 100. Souvent, le modèle de ces erreurs est le même. Il existe une méthode générale qui retourne potentiellement
null . Le résultat de cette méthode est utilisé dans un grand nombre d'endroits, parfois via des dizaines d'appels de méthodes intermédiaires ou des vérifications supplémentaires. Il est important de comprendre que ces erreurs ne sont pas fatales, mais elles peuvent potentiellement conduire à un accès via un lien nul. Et détecter de telles erreurs est très difficile. Par conséquent, dans certains cas, vous devez envisager de refactoriser le code, dans lequel une exception serait levée si la méthode retournait
null . Sinon, vous ne pouvez sécuriser votre code qu'avec des contrôles totaux, ce qui est assez fastidieux et peu fiable. Bien entendu, dans chaque cas, la décision doit être prise en fonction des caractéristiques du projet.
Remarque Il arrive qu'à l'heure actuelle il n'y a pas de situations (données d'entrée) dans lesquelles la méthode retourne
null et il n'y a pas de véritable erreur. Cependant, un tel code n'est toujours pas fiable, car tout peut changer lorsque des modifications sont apportées au code.
Pour fermer le sujet avec
V3080 , jetons un coup d'œil aux erreurs évidentes avec le niveau de confiance élevé, lorsque l'accès via un lien nul est plus probable ou même inévitable.
Déréférence nulle possible - ÉlevéeV3080 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 d'une faute de frappe dans la condition (au lieu de l'opérateur
|| que nous avons utilisé
&& ), le code ne fonctionne pas du tout comme prévu, et la variable
collectionType.Type sera accessible si elle est
nulle . La condition doit être corrigée comme suit:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
Soit dit en passant, la deuxième variante du développement des événements est également possible: dans la première partie, les conditions sont mélangées par les opérateurs
== et
! = . Le code corrigé serait alors:
if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....
Cette version du code est moins logique, mais corrige également l'erreur. La décision finale appartient aux auteurs du projet.
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}'."); } .... }
Une erreur a été commise lors de la rédaction d'un message pour une exception. Dans le même temps, une tentative d'accès à la propriété
action.DisplayText est effectuée via la variable d'
action , qui est évidemment
nulle .
Et la dernière erreur est le V3080 High Level.
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 petite, donc je donne tout son code. La condition dans le bloc de
retour est incorrecte. Dans certains cas, il est possible de
lever une
NullReferenceException lors de l'accès à
type.FullName . J'utilise des crochets (ils ne changeront pas de comportement ici) pour clarifier la situation:
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
C'est ainsi que, selon la priorité des opérations, ce code fonctionnera. Si la variable de
type est
null , nous entrons dans une vérification else, où, en nous assurant que la variable
targetTypeName est
null , nous utilisons la référence de
type null. Vous pouvez corriger le code, par exemple, comme ceci:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Je pense que c'est là que vous pouvez terminer l'étude des erreurs V3080 et voir ce que l'analyseur PVS-Studio intéressant a réussi à trouver dans le code Roslyn.
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 d'un nom de variable non réussi, 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, il est recommandé d'utiliser le préfixe de soulignement pour les noms de paramètres dans de tels cas. Je vais donner 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; }
InsoucianceV3006 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 et l'objet exception est simplement créé. Le mot clé
throw a été omis. Version corrigée du code:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
La question de travailler avec des destructeurs en C # et d'en lever des exceptions est un sujet pour une discussion séparée, qui dépasse le cadre de cet article.
Quand le résultat n'est pas importantUn certain nombre d'avertissements
V3009 ont été reçus pour les méthodes qui retournent la même valeur dans tous les cas. Parfois, cela n'est pas critique, ou le code retour n'est tout simplement pas vérifié dans le code appelant. J'ai raté de tels avertissements. Mais quelques morceaux de code me semblaient suspects. Je citerai l'un d'eux:
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 renvoie uniquement
true et rien que
true . Dans le même temps, la valeur de retour est impliquée dans certaines vérifications du code appelant:
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
Il est difficile de dire à quel point ce comportement est dangereux. Mais si le résultat n'est pas nécessaire, il peut être utile de remplacer le type de retour par void et d'apporter des modifications minimales à la méthode d'appel. Cela rendra le code plus compréhensible et plus sûr.
Autres avertissements similaires:
- 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
Non vérifié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 '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
valeur de la variable est de type
NamingStylePreferences . Le problème vient de cette vérification. Même si la variable de
valeur n'est pas nulle, cela ne garantit pas que la conversion de type a réussi et que
valueToSerialize ne contient pas
null . Une
exception NullReferenceException peut être levée . Le code doit être corrigé comme suit:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Et encore une erreur 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 de type
ColumnState2 . Toutefois, le résultat de l'opération, la variable
columnState2 , n'est pas davantage vérifié pour
null . Au lieu de cela, la variable
columnState est vérifiée à l'aide de l'instruction
null conditionnelle. Quel est le danger de ce code? Comme dans l'exemple précédent, la
conversion de type à l'aide de l'opérateur
as peut échouer et la variable
columnState2 sera
nulle , ce qui
générera une exception. Soit dit en passant, une faute de frappe peut être à blâmer. Notez la condition dans le bloc
if . Peut-être qu'au lieu de
columnState? .Name, ils voulaient écrire
columnState2? .Name . Cela est très probable compte tenu des noms de variables plutôt malheureux
columnState et columnState2.Contrôles redondantsUn nombre assez élevé d'avertissements (plus de 100) ont été émis pour des constructions non critiques, mais potentiellement dangereuses associées à des contrôles redondants. Par exemple, je vais en donner un.
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 n'y a peut-être pas de véritable erreur ici. Juste une bonne raison de démontrer la combinaison des technologies «analyse interprocédurale + analyse de flux de données» en action. L'analyseur considère que la deuxième vérification
navInfo == null est redondante. En effet, avant cela, la valeur d'affectation de
navInfo sera obtenue à partir de la méthode
libraryService.NavInfoFactory.CreateForProject , qui va construire et retourner un nouvel objet de la classe
NavInfo . Mais pas
nul en aucune façon. La question est, pourquoi l'analyseur n'a-t-il pas
généré d' avertissement pour la première vérification
navInfo == null ? Il y a une explication à cela. Premièrement, si la variable de
symbole s'avère
nulle , la valeur de
navInfo restera une référence nulle. Deuxièmement, même si
navInfo obtient la valeur de la méthode
libraryService.NavInfoFactory.CreateForSymbol , cette valeur peut être
nulle . La première vérification de
navInfo == null est donc vraiment nécessaire.
Pas assez de contrôlesEt maintenant, la situation est l'inverse de ce qui précède. Plusieurs avertissements
V3042 ont été reçus pour le code accessible par une référence nulle. De plus, seulement un ou deux petits chèques pouvaient tout réparer.
Considérez un morceau 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? .Syntax . Plus loin dans le code, le
récepteur variable
est utilisé 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; }
Vous devez également vous assurer que le constructeur
BoundConditionalReceiver prend en charge l'obtention de valeurs
nulles pour ses paramètres, ou 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 de 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); } .... }
Si la variable
deux -
points est 0, ce qui autorise une condition dans le code, la méthode
Substring lèvera une
ArgumentOutOfRangeException . Correction requise:
if (colon > 0)
La faute de frappe est 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; }); .... }
Deux paramètres sont passés à l'expression lambda: t1 et t2. Cependant, seul t1 est utilisé. Cela semble suspect, compte tenu de la facilité avec laquelle il est possible de se tromper lors de l'utilisation de variables portant ces noms.
InsoucianceV3083 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 déclenché de manière non sécurisée. Entre la vérification de
l' égalité
nulle et l'appel d'un événement, ils peuvent avoir le temps de se désinscrire, puis une exception sera levée. De plus, dans le corps du bloc
if , juste avant l'appel de l'événement, d'autres opérations sont effectuées. J'ai appelé cette erreur «inattention», car à d'autres endroits du code, ils fonctionnent avec plus de précision avec cet événement, par exemple, comme ceci:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
L'utilisation de la variable de
gestionnaire facultative élimine le problème. Dans la méthode
OnTextBufferChanged ,
vous devez apporter des modifications pour la même opération sûre avec l'événement.
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, je vais réécrire ce fragment de code, en remplaçant les noms constants par 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. La deuxième partie (
sinon si ) sera effectuée uniquement pour la plage de valeurs de 2147483648 + 1 à 4294967295.
Quelques avertissements supplémentaires:
- 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 d'égalité nulle (ou leur absence)Quelques erreurs
V3095 sur la vérification de la
null d' une variable après
l' avoir utilisée. Le premier est ambigu, considérez 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 ce faire, vérifiez
Debug.Assert . On ne sait pas pourquoi il va après avoir utilisé la chaîne. Il convient également de
noter que pour d'autres configurations autres que Debug, le compilateur
supprimera Debug.Assert du code. Est-ce à dire que ce n'est que pour le débogage qu'il est possible d'obtenir une référence nulle? Et si ce n'est pas le cas, alors pourquoi ne pas vérifier
string.IsNullOrEmpty (string) , par exemple. Ce sont des questions pour les 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 morceau de code ne nécessite aucune explication. Je vais donner la version corrigée:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
Le code Roslyn a trouvé 15 autres erreurs de ce type:
- 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. 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
Considérez les erreurs de V3105 . Ici, nous utilisons l'opérateur nul conditionnel lors de l'initialisation de la variable, et ci-après dans le code, la variable est utilisée sans vérifier l' égalité nulle .L'erreur suivante est signalée immédiatement par deux avertissements.V3105 La variable 'documentId' a été utilisée après son affectation via un opérateur conditionnel nul. NullReferenceException est possible. CodeLensReferencesService.cs 138V3105 La variable 'documentId' a été utilisée après avoir été affectée via l'opérateur de condition nulle. 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 à null . Par conséquent, la création du ReferenceLocationDescriptor finira par lever une exception. Le code doit être corrigé: return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
De plus, plus loin dans le code, il est nécessaire de prévoir la possibilité d'égalité des variables nulles passées au constructeur.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 crochetsV3123 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 du tout calculée comme le pensait le développeur. Il a été supposé que la première condition serait _kind == other._kin d (par conséquent, l'habillage de ligne a été effectué après cette condition), puis les blocs de condition avec l'opérateur " ? " Seraient calculés séquentiellement . En fait, la première condition sera _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 corriger l'erreur, il est nécessaire de mettre entre crochets toutes les expressions de l'opérateur " ? ": return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Ceci conclut la description des erreurs trouvées.ConclusionsMalgré le nombre important d'erreurs que j'ai pu détecter, en termes de taille du code du projet Roslyn (2 770 000 lignes), ce sera un montant assez faible. Comme Andrei dans l'article précédent, je suis également prêt à reconnaître la haute qualité de ce projet.Je tiens à noter que ces vérifications de code occasionnelles n'ont rien à voir avec la méthodologie de l'analyse statique et n'apportent pratiquement aucun avantage. L'analyse statique doit être appliquée régulièrement, et non au cas par cas. Ensuite, de nombreuses erreurs seront corrigées dès les premières étapes et, par conséquent, le coût de leur correction sera dix fois moins élevé. Cette idée est décrite plus en détail dans ce petit article , que je vous demande de vous familiariser avec.Vous pouvez rechercher indépendamment d'autres erreurs à la fois dans le projet considéré et dans tout autre. Pour ce faire, il vous suffit de télécharger et d'essayer notre analyseur.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Sergey Khrenov. Vérification du code source de Roslyn .