Geekly articles weekly

Überprüfen des Roslyn-Quellcodes

PVS-Studio gegen Roslyn

Hin und wieder kehren wir zu den Projekten zurück, die wir zuvor mit PVS-Studio überprüft haben, was zu deren Beschreibung in verschiedenen Artikeln führt. Zwei Gründe machen diese Comebacks für uns spannend. Erstens die Möglichkeit, den Fortschritt unseres Analysegeräts zu bewerten. Zweitens die Überwachung des Feedbacks der Projektautoren zu unserem Artikel und des Fehlerberichts, den wir ihnen normalerweise zur Verfügung stellen. Natürlich können Fehler ohne unsere Teilnahme korrigiert werden. Es ist jedoch immer schön, wenn unsere Bemühungen dazu beitragen, ein Projekt besser zu machen. Roslyn war keine Ausnahme. Der vorherige Artikel über diese Projektprüfung stammt aus dem 23. Dezember 2015. Angesichts der Fortschritte, die unser Analysegerät seitdem erzielt hat, ist dies eine ziemlich lange Zeit. Da der C # -Kern des PVS-Studio-Analysators auf Roslyn basiert, gibt es uns zusätzliches Interesse an diesem Projekt. Aus diesem Grund sind wir von der Codequalität dieses Projekts begeistert. Lassen Sie es uns jetzt noch einmal testen und einige neue und interessante Probleme herausfinden (aber hoffen wir, dass nichts Bedeutendes ist), die PVS-Studio finden kann.

Viele unserer Leser kennen Roslyn (oder die .NET Compiler-Plattform) wahrscheinlich gut. Kurz gesagt, es handelt sich um eine Reihe von Open Source-Compilern und eine API für die Code-Analyse von C # - und Visual Basic .NET-Sprachen von Microsoft. Der Quellcode des Projekts ist auf GitHub verfügbar.

Ich werde diese Plattform nicht detailliert beschreiben und empfehle allen interessierten Lesern, den Artikel meines Kollegen Sergey Vasiliev " Einführung in Roslyn und seine Verwendung in der Programmentwicklung " zu lesen. In diesem Artikel erfahren Sie nicht nur, welche Merkmale die Roslyn-Architektur aufweist, sondern auch, wie genau wir diese Plattform verwenden.

Wie ich bereits erwähnt habe, ist es mehr als drei Jahre her, dass mein Kollege Andrey Karpov den letzten Artikel über den Roslyn-Check " New Year PVS-Studio 6.00 Release: Scanning Roslyn " geschrieben hat. Seitdem hat der C # PVS-Studio-Analysator viele neue Funktionen erhalten. Eigentlich war Andreys Artikel ein Testfall, da zu diesem Zeitpunkt der C # -Analysator gerade in PVS-Studio hinzugefügt wurde. Trotzdem konnten wir Fehler im Roslyn-Projekt feststellen, das sicherlich von hoher Qualität war. Was hat sich in diesem Moment im Analysator für C # -Code geändert, sodass wir eine eingehendere Analyse durchführen können?

Seitdem haben sich sowohl der Kern als auch die Infrastruktur entwickelt. Wir haben Unterstützung für Visual Studio 2017 und Roslyn 2.0 sowie eine umfassende Integration in MSBuild hinzugefügt. Der Artikel meines Kollegen Paul Eremeev " Unterstützung von Visual Studio 2017 und Roslyn 2.0 in PVS-Studio: Manchmal ist es nicht so einfach, vorgefertigte Lösungen zu verwenden, wie es scheint " beschreibt unseren Ansatz zur Integration in MSBuild und die Gründe für diese Entscheidung.

Momentan arbeiten wir aktiv an der Umstellung auf Roslyn 3.0, genauso wie wir Visual Studio 2017 ursprünglich unterstützt haben. Dazu muss unser eigenes Toolset verwendet werden, das in der PVS-Studio-Distribution als "Stub" enthalten ist und ein leeres MSBuild ist EXE-Datei. Trotz der Tatsache, dass es wie eine „Krücke“ aussieht (MSBuild API ist aufgrund der geringen Portabilität von Bibliotheken nicht sehr benutzerfreundlich für die Wiederverwendung in Projekten von Drittanbietern), hat uns ein solcher Ansatz bereits geholfen, mehrere Roslyn-Updates in Bezug auf Visual Studio relativ nahtlos zu überwinden 2017. Bis jetzt war es hilfreich (trotz einiger Herausforderungen), das Visual Studio 2019-Update zu durchlaufen und die vollständige Abwärtskompatibilität und Leistung für Systeme mit älteren MSBuild-Versionen aufrechtzuerhalten.

Der Analysatorkern hat auch eine Reihe von Verbesserungen erfahren. Eines der Hauptmerkmale ist eine vollständige interprozedurale Analyse unter Berücksichtigung der Werte der Eingabe- und Ausgabemethoden, wobei die Erreichbarkeit der Ausführungszweige und Rückgabepunkte (abhängig von diesen Parametern) bewertet wird.

Wir sind auf dem Weg, die Aufgabe der Überwachung der Parameter innerhalb der Methoden (z. B. potenziell gefährliche Dereferenzen) sowie das Speichern ihrer automatischen Anmerkungen abzuschließen. Bei einer Diagnose, die einen Datenflussmechanismus verwendet, können gefährliche Situationen berücksichtigt werden, die beim Übergeben eines Parameters in einer Methode auftreten. Zuvor wurde bei der Analyse derart gefährlicher Orte keine Warnung generiert, da wir bei einer solchen Methode nicht alle möglichen Eingabewerte kennen konnten. Jetzt können wir die Gefahr erkennen, da an allen Stellen, an denen diese Methode aufgerufen wird, diese Eingabeparameter berücksichtigt werden.

Hinweis: Weitere Informationen zu grundlegenden Analysemechanismen wie Datenfluss und anderen finden Sie im Artikel " Technologien, die im PVS-Studio-Codeanalysator zum Auffinden von Fehlern und potenziellen Schwachstellen verwendet werden ".

Die interprozedurale Analyse in PVS-Studio C # ist weder durch Eingabeparameter noch durch die Tiefe begrenzt. Die einzige Einschränkung sind virtuelle Methoden in Klassen, die für die Vererbung offen sind und in die Rekursion geraten (die Analyse wird beendet, wenn sie auf einen wiederholten Aufruf der bereits evaluierten Methode stößt). Dabei wird die rekursive Methode selbst schließlich unter der Annahme bewertet, dass der Rückgabewert ihrer Rekursion unbekannt ist.

Eine weitere großartige neue Funktion im C # -Analysator ist die Berücksichtigung einer möglichen Dereferenzierung eines potenziell Nullzeigers. Zuvor beschwerte sich der Analysator über eine mögliche Nullreferenzausnahme, wobei sichergestellt wurde, dass in allen Ausführungszweigen der Variablenwert null ist. Natürlich war es in einigen Fällen falsch, deshalb hatte die V3080- Diagnose zuvor eine potenzielle Nullreferenz genannt.

Jetzt ist sich der Analysator der Tatsache bewusst, dass die Variable in einem der Ausführungszweige null sein kann (z. B. unter einer bestimmten if- Bedingung). Wenn der Zugriff auf eine solche Variable ohne Prüfung bemerkt wird, wird die Warnung V3080 ausgegeben, jedoch mit geringerer Sicherheit, als wenn in allen Zweigen null angezeigt wird. Zusammen mit der verbesserten interprozeduralen Analyse ermöglicht ein solcher Mechanismus das Auffinden von Fehlern, die sehr schwer zu erkennen sind. Hier ein Beispiel - stellen Sie sich eine lange Kette von Methodenaufrufen vor, von denen der letzte Ihnen unbekannt ist. Unter bestimmten Umständen wird im catch- Block null zurückgegeben, aber Sie haben sich nicht davor geschützt, wie Sie einfach nicht gewusst haben. In diesem Fall beschwert sich der Analysator nur, wenn er genau die Nullzuweisung sieht. Unserer Ansicht nach unterscheidet es unseren Ansatz qualitativ von einem solchen Merkmal von C # 8.0 als nullfähige Typreferenz, das sich tatsächlich darauf beschränkt, für jede Methode Prüfungen auf null zu setzen. Wir empfehlen jedoch die Alternative, Überprüfungen nur an Stellen durchzuführen, an denen tatsächlich Null auftreten kann, und unser Analysator kann jetzt nach solchen Fällen suchen.

Lassen Sie uns den Hauptpunkt also nicht zu lange aufschieben und uns dem Schuldsturm widmen - indem wir die Ergebnisse des Roslyn-Checks analysieren. Betrachten wir zunächst die Fehler, die aufgrund der oben beschriebenen Funktionen festgestellt wurden. Insgesamt gab es diesmal ziemlich viele Warnungen für den Roslyn-Code. Ich denke, es hängt mit der Tatsache zusammen, dass sich die Plattform sehr aktiv weiterentwickelt (zu diesem Zeitpunkt umfasst die Codebasis etwa 2.770.000 Zeilen ohne Leerzeichen), und wir haben dieses Projekt nicht lange analysiert. Trotzdem gibt es nicht so viele kritische Fehler, obwohl sie für den Artikel am interessantesten sind. Wie üblich habe ich Tests von der Prüfung ausgeschlossen, es gibt ziemlich viele in Roslyn.

Ich beginne mit V3080-Fehlern mittlerer Sicherheitsstufe, bei denen der Analysator einen möglichen Zugriff durch Nullreferenz erkannt hat, jedoch nicht in allen möglichen Fällen (Code-Verzweigungen).

Mögliche Null-Dereferenzierung - Mittel

V3080 Mögliche Null-Dereferenzierung. Betrachten Sie die Überprüfung der "aktuellen". CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70

private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....)) // <= { .... var nodeOrToken = current.ChildThatContainsPosition(....); .... current = nodeOrToken.AsNode(); // <= } .... } public SyntaxNode AsNode() { if (_token != null) { return null; } return _nodeOrParent; }

Betrachten wir die Methode GetNode . Der Analysator schlägt vor, dass der Zugriff durch Nullreferenz im Zustand des while- Blocks möglich ist . Der Variablen wird im Hauptteil des while- Blocks ein Wert zugewiesen, der ein Ergebnis der AsNode- Methode ist. In einigen Fällen ist dieser Wert null . Ein gutes Beispiel für eine interprozedurale Analyse in Aktion.

Betrachten wir nun einen ähnlichen Fall, in dem die Interprozeduranalyse über zwei Methodenaufrufe durchgeführt wurde.

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie das Verzeichnis. CommonCommandLineParser.cs 911

 private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ? // <= baseDirectory : FileUtilities.ResolveRelativePath(directory, baseDirectory); .... } public static string GetDirectoryName(string path) { return GetDirectoryName(path, IsUnixLikePlatform); } internal static string GetDirectoryName(string path, bool isUnixLike) { if (path != null) { .... } return null; }

Die Verzeichnisvariable im Hauptteil der ExpandFileNamePattern- Methode erhält den Wert von der Methode GetDirectoryName (Zeichenfolge) . Dies gibt wiederum das Ergebnis der überladenen Methode GetDirectoryName (string, bool) zurück, deren Wert null sein kann . Da das Variablenverzeichnis ohne vorläufige Prüfung auf null im Hauptteil der Methode ExpandFileNamePattern verwendet wird , können wir den Analysator für die Ausgabe der Warnung als korrekt proklamieren. Dies ist eine möglicherweise unsichere Konstruktion.

Ein weiteres Codefragment mit dem Fehler V3080, genauer gesagt mit zwei Fehlern, wurde für eine einzelne Codezeile ausgegeben. Die interprozedurale Analyse wurde hier nicht benötigt.

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie 'spanStartLocation'. TestWorkspace.cs 574

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie '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, // <= spanEndLocationExclusive.Value - // <= spanStartLocation.Value)); } } .... }

Die Variablen spanStartLocation und spanEndLocationExclusive sind vom Typ nullable int und werden mit null initialisiert. Weiter entlang des Codes können ihnen Werte zugewiesen werden, jedoch nur unter bestimmten Bedingungen. In einigen Fällen bleibt ihr Wert null . Danach wird auf diese Variablen ohne vorherige Überprüfung auf Null zugegriffen, was der Analysator anzeigt.

Der Roslyn-Code enthält eine Menge solcher Fehler, mehr als 100. Oft ist das Muster dieser Fehler das gleiche. Es gibt eine Art allgemeine Methode, die möglicherweise null zurückgibt. Das Ergebnis dieser Methode wird an vielen Stellen verwendet, manchmal durch Dutzende von Zwischenmethodenaufrufen oder zusätzliche Überprüfungen. Es ist wichtig zu verstehen, dass diese Fehler nicht schwerwiegend sind, aber möglicherweise zu einem Zugriff durch Nullreferenz führen können. Das Erkennen solcher Fehler ist zwar eine ziemliche Herausforderung. Aus diesem Grund sollte in einigen Fällen ein Code-Refactoring in Betracht gezogen werden. In diesem Fall löst die Methode eine Ausnahme aus, wenn null zurückgegeben wird. Andernfalls können Sie Ihren Code nur mit allgemeinen Überprüfungen sichern, die ziemlich anstrengend und manchmal unzuverlässig sind. Wie auch immer, es ist klar, dass jeder spezifische Fall eine Lösung erfordert, die auf Projektspezifikationen basiert.

Hinweis Es kommt also vor, dass es zu einem bestimmten Zeitpunkt keine solchen Fälle (Eingabedaten) gibt, wenn die Methode null zurückgibt und kein tatsächlicher Fehler vorliegt. Ein solcher Code ist jedoch immer noch nicht zuverlässig, da sich bei der Einführung einiger Codeänderungen alles ändern kann.

Um das Thema V3080 zu löschen , betrachten wir offensichtliche Fehler mit hoher Sicherheit, wenn der Zugriff per Nullreferenz am wahrscheinlichsten oder sogar unvermeidlich ist.

Mögliche Null-Dereferenzierung - Hoch

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie '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; } .... }

Aufgrund des Tippfehlers in der Bedingung ( && wird anstelle des Operators || verwendet ) funktioniert der Code anders als beabsichtigt und der Zugriff auf die Variable collectionType.Type wird ausgeführt, wenn sie null ist . Der Zustand sollte wie folgt korrigiert werden:

 if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....

Übrigens können sich die Dinge auf andere Weise entfalten: Im ersten Teil der Bedingung sind die Operatoren == und ! = Durcheinander . Dann würde der richtige Code so aussehen:

 if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....

Diese Version des Codes ist weniger logisch, korrigiert aber auch den Fehler. Die endgültige Lösung liegt bei den Autoren des Projekts.

Ein weiterer ähnlicher Fehler.

V3080 Mögliche Null-Dereferenzierung. Überlegen Sie, ob Sie die Aktion überprüfen möchten. 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}'."); } .... }

Der Fehler tritt beim Generieren der Nachricht für die Ausnahme auf. Es folgt der Versuch, über die Aktionsvariable , die als null bekannt ist, auf die action.DisplayText- Eigenschaft zuzugreifen.

Hier kommt der letzte V3080-Fehler des High-Levels.

V3080 Mögliche Null-Dereferenzierung. Überprüfen Sie den Typ. ObjectFormatterHelpers.cs 91

 private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }

Die Methode ist ziemlich klein, deshalb zitiere ich sie ganz. Die Bedingung im Rückgabeblock ist falsch. In einigen Fällen kann beim Zugriff auf type.FullName eine Ausnahme auftreten. Ich werde Klammern verwenden, um es klar zu machen (sie werden das Verhalten nicht ändern):

 return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);

Entsprechend der Priorität der Operationen funktioniert der Code genau so. Falls die Typvariable null ist , fallen wir in die else-Prüfung, in der wir die Typ- Null-Referenz verwenden, nachdem wir die Variable targetTypeName auf null überprüft haben. Der Code kann beispielsweise wie folgt festgelegt werden:

 return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);

Ich denke, es reicht aus, um V3080-Fehler zu überprüfen. Jetzt ist es höchste Zeit, andere interessante Dinge zu sehen, die der PVS-Studio-Analysator gefunden hat.

Tippfehler

V3005 Die Variable 'SourceCodeKind' wird sich selbst zugewiesen. DynamicFileInfo.cs 17

 internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind; // <= TextLoader = textLoader; DocumentServiceProvider = documentServiceProvider; } .... }

Aufgrund fehlgeschlagener Benennung von Variablen wurde im Konstruktor der DynamicFileInfo- Klasse ein Tippfehler gemacht. Dem Feld SourceCodeKind wird ein eigener Wert zugewiesen, anstatt den Parameter sourceCodeKind zu verwenden . Um die Wahrscheinlichkeit solcher Fehler zu minimieren, empfehlen wir, in solchen Fällen das Unterstrichpräfix für die Parameternamen zu verwenden. Hier ist ein Beispiel für eine korrigierte Version des Codes:

 public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }

Versehen

V3006 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw new InvalidOperationException (FOO). ProjectBuildManager.cs 61

 ~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }

Unter bestimmten Bedingungen muss der Destruktor eine Ausnahme auslösen, dies geschieht jedoch nicht, während das Ausnahmeobjekt einfach erstellt wird. Das Schlüsselwort throw wurde übersehen. Hier ist die korrigierte Version des Codes:

 ~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }

Das Problem mit Destruktoren in C # und das Auslösen von Ausnahmen ist ein Thema für eine weitere Diskussion, die den Rahmen dieses Artikels sprengt.

Wenn das Ergebnis nicht wichtig ist

Methoden, die in allen Fällen den gleichen Wert erhielten, lösten eine bestimmte Anzahl von V3009- Warnungen aus. In einigen Fällen kann dies nicht kritisch sein oder der Rückgabewert wird im aufrufenden Code einfach nicht überprüft. Ich habe solche Warnungen übersprungen. Aber ein paar Codefragmente schienen verdächtig. Hier ist einer von ihnen:

V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. GoToDefinitionCommandHandler.cs 62

 internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }

Die Methode TryExecuteCommand gibt nur true zurück . Dabei wird im aufrufenden Code der zurückgegebene Wert in einige Prüfungen einbezogen.

 public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }

Es ist schwer genau zu sagen, inwieweit ein solches Verhalten gefährlich ist. Wenn das Ergebnis jedoch nicht benötigt wird, sollte der Typ des Rückgabewerts möglicherweise in void geändert und die aufrufende Methode geringfügig geändert werden. Dadurch wird der Code lesbarer und sicherer.

Ähnliche Warnungen des Analysators:

  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. CommentUncommentSelectionCommandHandler.cs 86
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. JsonRpcClient.cs 138
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. AbstractFormatEngine.OperationApplier.cs 164
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'false' zurückgibt. TriviaDataFactory.CodeShapeAnalyzer.cs 254
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. ObjectList.cs 173
  • V3009 Es ist seltsam, dass diese Methode immer ein und denselben Wert von 'true' zurückgibt. ObjectList.cs 249

Überprüfte das Falsche

V3019 Möglicherweise wird eine falsche Variable nach der Typkonvertierung mit dem Schlüsselwort 'as' mit null verglichen. Überprüfen Sie die Variablen 'value', 'valueToSerialize'. RoamingVisualStudioProfileOptionPersister.cs 277

 public bool TryPersist(OptionKey optionKey, object value) { .... var valueToSerialize = value as NamingStylePreferences; if (value != null) { value = valueToSerialize.CreateXElement().ToString(); } .... }

Die Wertvariable wird in den Typ NamingStylePreferences umgewandelt . Das Problem liegt in der folgenden Überprüfung. Selbst wenn die Wertvariable nicht null war, kann nicht garantiert werden, dass die Typumwandlung erfolgreich war, und valueToSerialize enthält keine Null . Mögliches Auslösen der Ausnahme NullReferenceException . Der Code muss wie folgt korrigiert werden:

 var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }

Ein weiterer ähnlicher Fehler:

V3019 Möglicherweise wird eine falsche Variable nach der Typkonvertierung mit dem Schlüsselwort 'as' mit null verglichen. Überprüfen Sie die Variablen 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181

 private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name == // <= StandardTableColumnDefinitions2.Definition) { newColumns.Add(new ColumnState2( columnState2.Name, // <= ....)); } .... } .... }

Die Variable columnState wird in den Typ ColumnState2 umgewandelt . Das Operationsergebnis, bei dem es sich um die Variable columnState2 handelt, wird jedoch nicht weiter auf null geprüft. Stattdessen wird die Variable columnState mit dem bedingten Nulloperator überprüft. Warum ist dieser Code gefährlich? Genau wie im vorherigen Beispiel kann das Casting mit dem Operator as fehlschlagen und die Variable ist null, was zu einer Ausnahme führt. Übrigens kann hier ein Tippfehler schuld sein. Schauen Sie sich den Zustand im if- Block an.

Vielleicht wollte der Autor anstelle von columnState? .Name columnState2? .Name schreiben. Es ist sehr wahrscheinlich, wenn man die ziemlich fehlerhaften Variablennamen columnState und columnState2 berücksichtigt.

Redundante Schecks

Bei unkritischen, aber möglicherweise unsicheren Konstruktionen im Zusammenhang mit redundanten Überprüfungen wurde eine große Anzahl von Warnungen ausgegeben (mehr als 100). Zum Beispiel ist dies einer von ihnen.

V3022 Der Ausdruck 'navInfo == null' ist immer falsch. 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) // <= { return true; } .... } public IVsNavInfo CreateForSymbol(....) { .... return null; } public IVsNavInfo CreateForProject(....) { return new NavInfo(....); }

Möglicherweise gibt es hier keinen tatsächlichen Fehler. Dies ist nur ein guter Grund, die "Interprozedurale Analyse + Datenflussanalyse" im Schlepptau zu demonstrieren. Der Analysator schlägt vor, dass die zweite Prüfung navInfo == null redundant ist. Zuvor wird der NavInfo zugewiesene Wert von der Methode libraryService.NavInfoFactory.CreateForProject abgerufen, die ein neues Objekt der NavInfo- Klasse erstellt und zurückgibt . Auf keinen Fall wird null zurückgegeben . Hier stellt sich die Frage, warum der Analysator keine Warnung für die erste Prüfung navInfo == null ausgegeben hat . Es gibt einige Gründe. Erstens, wenn die Symbolvariable null ist , bleibt der navInfo- Wert auch eine Nullreferenz . Zweitens kann dieser Wert auch null sein , selbst wenn navInfo den Wert von der Methode ibraryService.NavInfoFactory.CreateForSymbol erhält. Daher wird die erste Prüfung navInfo == null wirklich benötigt.

Unzureichende Kontrollen

Nun ist die umgekehrte Situation von der oben diskutierten. Für den Code wurden mehrere V3042- Warnungen ausgelöst, bei denen ein Zugriff per Nullreferenz möglich ist. Sogar ein oder zwei kleine Schecks hätten alles reparieren können.

Betrachten wir ein weiteres interessantes Codefragment, das zwei solche Fehler aufweist.

V3042 Mögliche NullReferenceException. Das '?.' und '.' Operatoren werden für den Zugriff auf Mitglieder des 'Empfänger'-Objekts Binder_Expressions.cs 7770 verwendet

V3042 Mögliche NullReferenceException. Das '?.' und '.' Operatoren werden für den Zugriff auf Mitglieder des 'Empfänger'-Objekts Binder_Expressions.cs 7776 verwendet

 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; }

Die Empfängervariable kann null sein. Der Autor des Codes weiß davon, da er den bedingten Nulloperator in der Bedingung des if- Blocks verwendet, um auf den Empfänger zuzugreifen . Syntax . Ferner wird die Empfängervariable ohne Überprüfung verwendet, um auf Empfänger zuzugreifen. Typ, Empfänger . Syntax und Empfänger . HatErrors . Diese Fehler müssen korrigiert werden:

 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; }

Wir müssen auch sicherstellen, dass der Konstruktor das Abrufen von Nullwerten für seine Parameter unterstützt, oder wir müssen zusätzliches Refactoring durchführen.

Andere ähnliche Fehler:

  • V3042 Mögliche NullReferenceException. Das '?.' und '.' Operatoren werden für den Zugriff auf Mitglieder des Objekts 'includesType' SyntaxGeneratorExtensions_Negate.cs 240 verwendet
  • V3042 Mögliche NullReferenceException. Das '?.' und '.' Operatoren werden für den Zugriff auf Mitglieder des 'expression'-Objekts ExpressionSyntaxExtensions.cs 349 verwendet
  • V3042 Mögliche NullReferenceException. Das '?.' und '.' Operatoren werden für den Zugriff auf Mitglieder des 'expression'-Objekts ExpressionSyntaxExtensions.cs 349 verwendet

Fehler in der Bedingung

V3057 Die Funktion 'Teilzeichenfolge' könnte den Wert '-1' empfangen, während ein nicht negativer Wert erwartet wird. Untersuchen Sie das zweite Argument. CommonCommandLineParser.cs 109

 internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }

Wenn die Doppelpunktvariable 0 ist, was gemäß der Bedingung im Code in Ordnung ist , löst die Substring- Methode eine Ausnahme aus. Dies muss behoben werden:

 if (colon > 0)

Möglicher Tippfehler

V3065 Der Parameter 't2' wird im Körper der Methode nicht verwendet. 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; }); .... }

Der Lambda-Ausdruck akzeptiert zwei Parameter: t1 und t2. Es wird jedoch nur t1 verwendet. Es sieht verdächtig aus, wenn man bedenkt, wie einfach es ist, bei der Verwendung von Variablen mit solchen Namen einen Fehler zu machen.

Versehen

V3083 Unsicherer Aufruf des Ereignisses 'TagsChanged', NullReferenceException ist möglich. Überlegen Sie, ob Sie einer lokalen Variablen ein Ereignis zuweisen möchten, bevor Sie sie aufrufen. PreviewUpdater.Tagger.cs 37

 public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span)); // <= } } }

Das TagChanged- Ereignis wird auf unsichere Weise aufgerufen. Zwischen der Überprüfung auf null und dem Aufrufen des Ereignisses kann sich jemand abmelden, und es wird eine Ausnahme ausgelöst. Darüber hinaus werden unmittelbar vor dem Aufrufen des Ereignisses andere Operationen im Hauptteil des if- Blocks ausgeführt. Ich habe diesen Fehler "Versehen" genannt, weil dieses Ereignis an anderen Stellen wie folgt sorgfältiger behandelt wird:

 private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }

Die Verwendung einer zusätzlichen Handlervariablen verhindert das Problem. In der Methode OnTextBufferChanged müssen Änderungen vorgenommen werden, um das Ereignis sicher zu behandeln.

Schnittpunkte

V3092 Bereichsschnittpunkte sind innerhalb von bedingten Ausdrücken möglich. Beispiel: 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 { .... } }

Lassen Sie mich zum besseren Verständnis diesen Code neu schreiben und die Namen der Konstanten mit ihren tatsächlichen Werten ändern:

 internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }

Wahrscheinlich gibt es keinen wirklichen Fehler, aber der Zustand sieht seltsam aus. Sein zweiter Teil ( sonst wenn ) wird nur für den Bereich von 2147483648 + 1 bis 4294967295 ausgeführt.

Noch ein paar ähnliche Warnungen:

  • V3092 Bereichsschnittpunkte sind innerhalb von bedingten Ausdrücken möglich. Beispiel: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 109
  • V3092 Bereichsschnittpunkte sind innerhalb von bedingten Ausdrücken möglich. Beispiel: if (A> 0 && A <5) {...} else if (A> 3 && A <9) {...}. LocalRewriter_Literal.cs 66

Weitere Informationen zu Überprüfungen auf Null (oder deren Fehlen)

Einige V3095- Fehler bei der Überprüfung einer Variablen auf Null direkt nach ihrer Verwendung. Der erste ist nicht eindeutig, betrachten wir den Code.

V3095 Das Objekt 'displayName' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 498, 503. FusionAssemblyIdentity.cs 498

 internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }

Es wird angenommen, dass der Referenzanzeigename null sein kann. Hierzu wurde die Prüfung Debug.Assert durchgeführt. Es ist nicht klar, warum es nach der Verwendung einer Zeichenfolge geht. Es muss auch berücksichtigt werden, dass der Compiler bei anderen Konfigurationen als Debug Debug.Assert überhaupt entfernt . Bedeutet dies, dass das Abrufen einer Nullreferenz nur für Debug möglich ist? Wenn dies nicht der Fall ist, warum hat der Autor beispielsweise die Zeichenfolge.IsNullOrEmpty (Zeichenfolge) überprüft ? Dies ist die Frage an die Autoren des Codes.

Der folgende Fehler ist offensichtlicher.

V3095 Das Objekt 'scriptArgsOpt' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 321, 325. CommonCommandLineParser.cs 321

 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg); // <= continue; } if (scriptArgsOpt != null) { .... } .... } }

Ich denke, dieser Code braucht keine Erklärungen. Lassen Sie mich Ihnen die feste Version geben:

 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }

Im Roslyn-Code gab es 15 weitere ähnliche Fehler:

  • V3095 Das Objekt 'LocalFunctions' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289
  • V3095 Das Objekt 'Resolution.OverloadResolutionResult' wurde verwendet, bevor es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 579, 588. Binder_Invocation.cs 579
  • V3095 Das Objekt 'Resolution.MethodGroup' wurde verwendet, bevor es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 592, 621. Binder_Invocation.cs 592
  • V3095 Das Objekt 'touchFilesLogger' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 111, 126. CSharpCompiler.cs 111
  • V3095 Das Objekt 'newExceptionRegionsOpt' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 736, 743. AbstractEditAndContinueAnalyzer.cs 736
  • V3095 Das 'Symbol'-Objekt wurde verwendet, bevor es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 422, 427. AbstractGenerateConstructorService.Editor.cs 422
  • V3095 Das Objekt '_state.BaseTypeOrInterfaceOpt' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132
  • V3095 Das Objekt 'element' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 232, 233. ProjectUtil.cs 232
  • V3095 Das 'Sprachen'-Objekt wurde verwendet, bevor es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 22, 28. ExportCodeCleanupProvider.cs 22
  • V3095 Das Objekt 'memberType' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
  • V3095 Das Objekt 'validTypeDeclarations' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 223, 228. SyntaxTreeExtensions.cs 223
  • V3095 Das 'Text'-Objekt wurde verwendet, bevor es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 376, 385. MSBuildWorkspace.cs 376
  • V3095 Das Objekt 'nameOrMemberAccessExpression' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 206, 223. CSharpGenerateTypeService.cs 206
  • V3095 Das Objekt 'simpleName' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 83, 85. CSharpGenerateMethodService.cs 83
  • V3095 Das Objekt 'option' wurde verwendet, bevor es gegen null verifiziert wurde. Überprüfen Sie die Zeilen: 23, 28. OptionKey.cs 23

Betrachten wir V3105- Fehler. Hier wird der bedingte Nulloperator beim Initialisieren der Variablen verwendet, aber weiterhin wird die Variable ohne Prüfung auf Null verwendet .

Zwei Warnungen weisen auf folgenden Fehler hin:

V3105 Die Variable 'documentId' wurde verwendet, nachdem sie durch einen nullbedingten Operator zugewiesen wurde. NullReferenceException ist möglich. CodeLensReferencesService.cs 138

V3105 Die Variable 'documentId' wurde verwendet, nachdem sie durch einen nullbedingten Operator zugewiesen wurde. NullReferenceException ist möglich. CodeLensReferencesService.cs 139

 private static async Task<ReferenceLocationDescriptor> GetDescriptorOfEnclosingSymbolAsync(....) { .... var documentId = solution.GetDocument(location.SourceTree)?.Id; return new ReferenceLocationDescriptor( .... documentId.ProjectId.Id, documentId.Id, ....); }

Die Variable documentId kann mit null initialisiert werden. Wenn Sie ein Objekt ReferenceLocationDescriptor erstellen, wird eine Ausnahme ausgelöst. Der Code muss behoben sein:

 return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);

Entwickler sollten auch die Möglichkeit abdecken, dass Variablen, die an einen Konstruktor übergeben werden, null sind.

Andere ähnliche Fehler im Code:

  • V3105 Die Variable 'symbol' wurde verwendet, nachdem sie durch einen nullbedingten Operator zugewiesen wurde. NullReferenceException ist möglich. SymbolFinder_Hierarchy.cs 44
  • V3105 Die Variable 'symbol' wurde verwendet, nachdem sie durch einen nullbedingten Operator zugewiesen wurde. NullReferenceException ist möglich. SymbolFinder_Hierarchy.cs 51

Prioritäten und Klammern

V3123 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Ihre Priorität ist niedriger als die Priorität anderer Betreiber in ihrem Zustand. 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); }

Die Bedingung im Rückgabeblock wird nicht wie vom Entwickler beabsichtigt ausgewertet. Es wurde angenommen, dass die erste Bedingung _kind == other._kin d ist (aus diesem Grund gibt es nach dieser Bedingung einen Zeilenumbruch), und danach werden die Bedingungsblöcke mit dem Operator " ? " Nacheinander ausgewertet. Tatsächlich ist die erste Bedingung _kind == other._kind && (_oldNode == null) . Dies liegt an der Tatsache, dass der Operator && eine höhere Priorität als der Operator " ? " Hat . Um dies zu beheben, sollte ein Entwickler alle Ausdrücke des Operators " ? " In Klammern setzen:

 return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));

Damit ist meine Beschreibung der gefundenen Fehler abgeschlossen.

Fazit

Trotz der großen Anzahl von Fehlern, die ich in Bezug auf die Größe des Roslyn-Projektcodes (2.770.000 Zeilen) gefunden habe, ist es nicht zu viel. Wie Andrey in einem früheren Artikel schrieb, bin ich auch bereit, die hohe Qualität dieses Projekts anzuerkennen.

Ich möchte darauf hinweisen, dass solche gelegentlichen Codeprüfungen nichts mit der Methodik der statischen Analyse zu tun haben und fast nicht hilfreich sind. Statische Analysen sollten regelmäßig und nicht von Fall zu Fall durchgeführt werden. Auf diese Weise werden viele Fehler in den frühesten Stadien korrigiert, und daher sind die Kosten für deren Behebung zehnmal geringer. Diese Idee wird in dieser kleinen Notiz ausführlicher beschrieben. Bitte probieren Sie sie aus.

Sie können einige Fehler sowohl in diesem als auch in einem anderen Projekt überprüfen. Dazu müssen Sie nur unseren Analysator herunterladen und ausprobieren.

Source: https://habr.com/ru/post/de446588/

More articles:


All Articles