Von Zeit zu Zeit kehren wir zu Projekten zurück, die wir zuvor mit PVS-Studio getestet und Artikel darüber geschrieben haben. Dafür gibt es zwei Gründe. Erstens, um zu verstehen, wie viel besser unser Analysator geworden ist. Zweitens, um zu verfolgen, ob die Autoren des Projekts auf unseren Artikel sowie auf den Fehlerbericht geachtet haben, den wir ihnen normalerweise zur Verfügung stellen. Natürlich können Fehler ohne unsere Teilnahme korrigiert werden. Aber es ist immer schön, wenn genau unsere Bemühungen dazu beitragen, ein Projekt besser zu machen. Roslyn war keine Ausnahme. Ein früherer Übersichtsartikel zu diesem Projekt stammt aus dem 23. Dezember 2015. Dies ist eine ziemlich lange Zeit, angesichts des Weges, den unser Analysator in dieser Zeit in seiner Entwicklung zurückgelegt hat. Für uns persönlich ist Roslyn auch deshalb von zusätzlichem Interesse, weil der Kern des C # -Analysators PVS-Studio darauf basiert. Daher sind wir sehr an der Qualität des Codes für dieses Projekt interessiert. Wir werden einen zweiten Check arrangieren und herausfinden, was neu und interessant ist (aber hoffen wir, dass nichts Bedeutendes), was PVS-Studio dort finden kann.
Roslyn (oder die .NET Compiler-Plattform) ist wahrscheinlich vielen unserer Leser bekannt. Kurz gesagt, es handelt sich um eine Reihe von Open Source-Compilern und APIs für die Codeanalyse für die C # - und Visual Basic .NET-Sprachen von Microsoft. Der Quellcode für das Projekt ist auf
GitHub verfügbar.
Ich werde diese Plattform nicht detailliert beschreiben, aber ich würde jedem Interessierten einen Artikel meines Kollegen Sergey Vasiliev empfehlen: "
Einführung in Roslyn. Verwenden statischer Analysewerkzeuge zur Entwicklung ." In diesem Artikel erfahren Sie nicht nur, welche Funktionen die Roslyn-Architektur bietet, sondern auch, wie genau wir diese Plattform verwenden.
Wie ich bereits erwähnt habe, sind mehr als drei Jahre vergangen, seit mein Kollege Andrei Karpov den letzten Artikel über den Roslyn-Check "
Neujahrsveröffentlichung von PVS-Studio 6.00: Check Roslyn " geschrieben hat. In dieser Zeit hat der C # PVS-Studio-Analysator viele neue Funktionen erhalten. Im Allgemeinen war Andreys Artikel eine Art „Testball“, da der C # -Analysator erst damals zu PVS-Studio hinzugefügt wurde. Trotzdem gelang es Roslyn selbst dann in einem bedingungslos hochwertigen Projekt, interessante Fehler zu finden. Was hat sich im Analysator für C # -Code bisher geändert, was möglicherweise eine tiefere Analyse ermöglicht?
In der Vergangenheit haben sich sowohl der Analysatorkern als auch die Infrastruktur entwickelt. Unterstützung für Visual Studio 2017 und Roslyn 2.0 sowie eine umfassende Integration in MSBuild wurden hinzugefügt. Weitere Informationen zu unserem Ansatz zur Integration in MSBuild und zu den Gründen, aus denen wir ihn akzeptiert haben, finden Sie in dem Artikel meines Kollegen Pavel Yeremeyev: "
Unterstützung für Visual Studio 2017 und Roslyn 2.0 in PVS-Studio: Manchmal ist die Verwendung vorgefertigter Lösungen nicht so einfach, wie es scheint auf einen Blick . "
Jetzt arbeiten wir aktiv an der Umstellung auf Roslyn 3.0 nach demselben Schema, das wir ursprünglich für Visual Studio 2017 unterstützt haben, dh über unser eigenes Toolset, das im PVS-Studio-Distributionskit mit einem „Stub“ in Form einer leeren MSBuild.exe-Datei enthalten ist. Trotz der Tatsache, dass es wie eine „Krücke“ aussieht (die MSBuild-API ist aufgrund der geringen Portabilität der Bibliotheken nicht sehr benutzerfreundlich für die Wiederverwendung in Third-Patry-Projekten), hat uns dieser Ansatz bereits geholfen, mehrere Roslyn-Updates während der Lebensdauer von Visual Studio 2017 relativ schmerzlos erneut zu erleben. und jetzt, obwohl mit vielen Überlagerungen, überleben Sie das Upgrade auf Visual Studio 2019 und behalten die volle Abwärtskompatibilität und Leistung auf Systemen mit älteren Versionen von MSBuild bei.
Der Analysatorkern hat auch eine Reihe von Verbesserungen erfahren. Eine der Hauptinnovationen ist eine umfassende interprozedurale Analyse, die die Eingabe- und Ausgabewerte der Methoden berücksichtigt und in Abhängigkeit von diesen Parametern die Erreichbarkeit der Ausführungszweige und Rückgabepunkte berücksichtigt.
Die Aufgabe, Parameter innerhalb der Methoden zu verfolgen, ist bereits kurz vor dem Abschluss, während automatische Anmerkungen für die dortigen Parameter beibehalten werden (z. B. eine potenziell gefährliche Dereferenzierung). Auf diese Weise kann bei jeder Diagnose mithilfe des Datenflussmechanismus gefährliche Situationen berücksichtigt werden, die beim Übergeben eines Parameters an eine Methode auftreten. Bisher wurde bei der Analyse derart gefährlicher Orte keine Warnung generiert, da wir nicht alle möglichen Eingabewerte für eine solche Methode kennen konnten. Jetzt können wir die Gefahr erkennen, da diese Eingabeparameter an allen Stellen, an denen diese Methode aufgerufen wird, berücksichtigt werden.
Hinweis: Sie können sich mit den wichtigsten Analysemechanismen wie Datenfluss und anderen aus dem Artikel "
Im PVS-Studio Code Analyzer zum Auffinden von Fehlern und potenziellen Schwachstellen verwendete Technologien " vertraut machen.
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 nicht wegen Vererbung geschlossen wurden und in Rekursion geraten (wir werden aufhören, wenn wir auf dem Stapel einen wiederholten Aufruf einer bereits berechneten Methode sehen). Darüber hinaus wird die rekursive Methode selbst letztendlich unter der Annahme berechnet, dass der Rückgabewert ihrer Selbstrekursion unbekannt ist.
Eine weitere große Neuerung im C # -Analysator ist die Möglichkeit, einen potenziell Nullzeiger zu dereferenzieren. Zuvor schwor der Analysator eine mögliche Nullreferenzausnahme, wenn er sicher war, dass in allen Ausführungszweigen der Wert der Variablen null sein würde. Natürlich hat er sich manchmal geirrt, daher wurde die
V3080- Diagnose zuvor als potenzielle
Nullreferenz bezeichnet .
Jetzt merkt sich der Analysator, dass die Variable in einem der Ausführungszweige null sein kann (z. B. unter einer bestimmten Bedingung in if). Wenn er den Zugriff auf eine solche Variable ohne Überprüfung sieht, erhält er eine V3080-Nachricht, jedoch mit einer geringeren Wichtigkeit als wenn er in allen Zweigen null sieht. In Kombination mit einer verbesserten interprozeduralen Analyse ermöglicht ein solcher Mechanismus das Auffinden sehr schwer zu erkennender Fehler. Ein Beispiel ist eine lange Kette von Methodenaufrufen, mit denen Sie zuletzt nicht vertraut sind und die beispielsweise unter bestimmten Umständen im Fang null zurückgeben, aber Sie haben sich nicht davor geschützt, weil Sie es einfach nicht wussten. In diesem Fall schwört der Analysator nur, wenn er die Zuweisung von Null genau sieht. Unserer Meinung nach unterscheidet dies unseren Ansatz qualitativ von einer solchen Innovation von C # 8.0 als nullbarem Referenztyp, die tatsächlich darauf hinausläuft, bei jeder Methode Nullprüfungen festzulegen. Wir bieten eine Alternative an - nur dort zu prüfen, wo wirklich null kommen kann, und unser Analysator kann jetzt nach solchen Situationen suchen.
Fahren wir also unverzüglich mit der „Nachbesprechung“ fort und analysieren die Ergebnisse des Roslyn-Checks. Schauen wir uns zunächst die Fehler an, die dank der oben beschriebenen Innovationen festgestellt wurden. Im Allgemeinen wurden diesmal einige Warnungen für den Roslyn-Code ausgegeben. Ich denke, dies liegt an der Tatsache, dass sich die Plattform sehr aktiv entwickelt (die Codebasis umfasst derzeit etwa 2.770.000 Codezeilen, ausgenommen leere), und wir haben dieses Projekt lange Zeit nicht analysiert. Es gibt jedoch nicht so viele kritische Fehler, nämlich sie sind für den Artikel von Interesse. Und ja, es gibt in Roslyn einige Tests, die ich wie üblich vom Testen ausgeschlossen habe.
Ich beginne mit V3080-Fehlern mit mittlerer Kritikalität, bei denen der Analysator einen möglichen Zugriff über eine Nullverbindung festgestellt hat, jedoch nicht in allen möglichen Fällen (Code-Verzweigungen).
Mögliche Null-Dereferenzierung - MittelV3080 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(....))
Betrachten Sie die
GetNode- Methode. Der Analysator ist der Ansicht, dass der Zugriff per Nullreferenz im Zustand des
while- Blocks möglich ist. Im Hauptteil des
while- Blocks wird der
aktuellen Variablen ein Wert zugewiesen - das Ergebnis der Ausführung der
AsNode- Methode. Und dieser Wert ist in einigen Fällen
null . Ein gutes Beispiel für eine interprozedurale Analyse in Aktion.
Betrachten Sie 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) ?
Die
Verzeichnisvariable im Hauptteil der
ExpandFileNamePattern- Methode
ruft den Wert von der
GetDirectoryName (string) -Methode ab. Dies gibt wiederum das Ergebnis der überladenen
GetDirectoryName- Methode
(Zeichenfolge, Bool) zurück , deren Wert möglicherweise
null ist . Da weiter im Hauptteil der
ExpandFileNamePattern- Methode die
Verzeichnisvariable verwendet wird, ohne zuvor auf
Nullgleichheit zu prüfen, können wir über die Rechtmäßigkeit der Warnung durch den Analysator sprechen. Dies ist ein möglicherweise unsicheres Design.
Ein weiterer Code mit Fehler V3080, genauer gesagt, sofort mit zwei Fehlern, die für eine Codezeile ausgegeben wurden. Hier war keine interprozedurale Analyse erforderlich.
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,
Die
Variablen spanStartLocation und
spanEndLocationExclusive sind vom Typ
nullable int und werden mit
null initialisiert. Weiter im Code können ihnen Werte zugewiesen werden, jedoch nur, wenn bestimmte Bedingungen erfüllt sind. In einigen Fällen bleibt ihr Wert gleich
Null . Weiter im Code wird auf diese Variablen als Referenz zugegriffen, ohne vorher auf Nullgleichheit zu prüfen, wie der Analysator anzeigt.
Roslyn-Code enthält einige solcher Fehler, mehr als 100. Oft ist das Muster dieser Fehler das gleiche. Es gibt eine allgemeine Methode, die möglicherweise
null zurückgibt. Das Ergebnis dieser Methode wird an einer großen Anzahl von 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 zum Zugriff über einen Null-Link führen können. Und solche Fehler zu erkennen ist sehr schwierig. In einigen Fällen sollten Sie daher in Betracht ziehen, den Code umzugestalten. In diesem Fall wird eine Ausnahme ausgelöst, wenn die Methode
null zurückgibt . Andernfalls können Sie Ihren Code nur mit vollständigen Überprüfungen sichern, was ziemlich mühsam und unzuverlässig ist. Natürlich sollte die Entscheidung in jedem Fall auf der Grundlage der Merkmale des Projekts getroffen werden.
Hinweis Es kommt vor, dass es derzeit keine Situationen (Eingabedaten) gibt, in denen die Methode
null zurückgibt und kein wirklicher Fehler vorliegt. Ein solcher Code ist jedoch immer noch nicht zuverlässig, da sich alles ändern kann, wenn Änderungen am Code vorgenommen werden.
Um das Thema mit
V3080 zu schließen ,
werfen wir einen Blick auf die offensichtlichen Fehler mit dem hohen Konfidenzniveau, wenn der Zugriff über einen Null-Link wahrscheinlicher oder sogar unvermeidlich ist.
Mögliche Null-Dereferenzierung - HochV3080 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 eines Tippfehlers in der Bedingung (anstelle des von uns verwendeten Operators
|| && ) funktioniert der Code nicht wie beabsichtigt, und auf die Variable
collectionType.Type wird zugegriffen, wenn sie
null ist . Der Zustand muss wie folgt korrigiert werden:
if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....
Übrigens ist auch die zweite Variante der Ereignisentwicklung möglich: Im ersten Teil werden die Bedingungen von den Operatoren
== und
! = Verwechselt . Dann wäre der korrigierte Code:
if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....
Diese Version des Codes ist weniger logisch, korrigiert aber auch den Fehler. Die endgültige Entscheidung 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}'."); } .... }
Beim Verfassen einer Nachricht für eine Ausnahme ist ein Fehler aufgetreten. Gleichzeitig wird versucht, über die
Aktionsvariable , die offensichtlich
null ist, auf die Eigenschaft
action.DisplayText zuzugreifen.
Und der letzte Fehler ist der V3080 High Level.
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 klein, also gebe ich den gesamten Code. Die Bedingung im
Rückgabeblock ist falsch. In einigen Fällen ist es möglich, beim Zugriff auf
type.FullName eine
NullReferenceException auszulösen . Ich benutze Klammern (sie werden das Verhalten hier nicht ändern), um die Situation zu klären:
return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);
Auf diese Weise funktioniert dieser Code entsprechend der Priorität der Operationen. Wenn die Typvariable
null ist , führen wir eine else-Prüfung durch, bei der wir die Null-
Typreferenz verwenden , um sicherzustellen, dass die Variable
targetTypeName null ist. Sie können den Code beispielsweise folgendermaßen korrigieren:
return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);
Ich denke, hier können Sie die Untersuchung von V3080-Fehlern abschließen und sehen, was der andere interessante PVS-Studio-Analysator im Roslyn-Code gefunden hat.
TippfehlerV3005 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;
Aufgrund eines erfolglosen Variablennamens 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, wird empfohlen, in solchen Fällen das Unterstrichpräfix für Parameternamen zu verwenden. Ich werde ein Beispiel für eine korrigierte Version des Codes geben:
public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }
NachlässigkeitV3006 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 sollte der Destruktor eine Ausnahme auslösen. Dies ist jedoch nicht der Fall, und das Ausnahmeobjekt wird einfach erstellt. Das Schlüsselwort
throw wurde weggelassen. Korrigierte Version des Codes:
~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }
Das Problem, mit Destruktoren in C # zu arbeiten und Ausnahmen von ihnen auszulösen, ist ein Thema für eine separate Diskussion, die den Rahmen dieses Artikels sprengt.
Wenn das Ergebnis nicht wichtig istFür Methoden, die in allen Fällen den gleichen Wert zurückgeben, wurde eine Reihe von
V3009- Warnungen empfangen. Manchmal ist dies nicht kritisch oder der Rückkehrcode wird im aufrufenden Code einfach nicht überprüft. Ich habe solche Warnungen verpasst. Aber ein paar Code-Teile schienen mir verdächtig. Ich werde einen von ihnen zitieren:
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
TryExecuteCommand- Methode gibt nur
true und nichts als
true zurück . Gleichzeitig ist der Rückgabewert an einigen Überprüfungen des aufrufenden Codes beteiligt:
public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }
Es ist schwer zu sagen, wie gefährlich dieses Verhalten ist. Wenn das Ergebnis jedoch nicht benötigt wird, kann es sinnvoll sein, den Rückgabetyp durch void zu ersetzen und minimale Änderungen an der aufrufenden Methode vorzunehmen. Dadurch wird der Code verständlicher und sicherer.
Andere ähnliche Warnungen:
- 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
Nicht geprüftV3019 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(); } .... }
Der Variablenwert ist vom Typ
NamingStylePreferences . Das Problem folgt dieser Überprüfung. Selbst wenn die
Wertvariable nicht null ist, garantiert dies nicht, dass die Typkonvertierung erfolgreich war und dass
valueToSerialize keine
Null enthält. Eine
NullReferenceException kann ausgelöst werden . Der Code muss wie folgt korrigiert werden:
var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }
Und noch ein ä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 ==
Die Variable
columnState ist vom Typ
ColumnState2 . Das Ergebnis der Operation, die Variable
columnState2 , wird jedoch nicht weiter auf
null geprüft. Stattdessen wird die Variable
columnState mithilfe der bedingten
Null- Anweisung überprüft. Was ist die Gefahr dieses Codes? Wie im vorherigen Beispiel kann die Typumwandlung mit dem Operator
as fehlschlagen, und die Variable
columnState2 ist
null , wodurch eine Ausnahme
ausgelöst wird. Übrigens kann ein Tippfehler schuld sein. Beachten Sie die Bedingung im
if- Block. Vielleicht
wollten sie anstelle von
columnState? .Name columnState2? .Name schreiben. Dies ist sehr wahrscheinlich angesichts der eher unglücklichen Variablennamen
columnState und columnState2.Redundante SchecksEine ziemlich große Anzahl von Warnungen (über 100) wurde für unkritische, aber möglicherweise unsichere Konstruktionen ausgegeben, die mit redundanten Überprüfungen verbunden sind. Zum Beispiel werde ich einen von ihnen geben.
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)
Vielleicht gibt es hier keinen wirklichen Fehler. Nur ein guter Grund, die Kombination der Technologien „Interprocedural Analysis + Data Flow Analysis“ in Aktion zu demonstrieren. Der Analysator ist der Ansicht, dass die zweite Prüfung
navInfo == null redundant ist.
Zuvor wird der Wert für die Zuweisung von
navInfo von der
libraryService.NavInfoFactory.CreateForProject- Methode abgerufen, die ein neues Objekt der
NavInfo- Klasse erstellt und
zurückgibt . Aber in keiner Weise
null . Die Frage ist, warum der Analysator keine Warnung für die erste Prüfung
navInfo == null generiert hat . Dafür gibt es eine Erklärung. Erstens, wenn sich herausstellt, dass die
Symbolvariable null ist , bleibt der Wert von
navInfo eine
Nullreferenz . Zweitens kann dieser Wert
null sein , selbst wenn
navInfo den Wert von der
libraryService.NavInfoFactory.CreateForSymbol- Methode erhält. Die erste Überprüfung von
navInfo == null ist also wirklich notwendig.
Nicht genug SchecksUnd jetzt ist die Situation umgekehrt. Für Code, auf den über eine
Nullreferenz zugegriffen werden konnte, wurden mehrere
V3042- Warnungen empfangen. Außerdem konnten nur ein oder zwei kleine Schecks alles reparieren.
Stellen Sie sich einen interessanten Code vor, der zwei solche Fehler enthält.
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 !=
Die
Empfängervariable kann
null sein . Der Autor des Codes weiß dies, weil er den bedingten
Nulloperator in der Bedingung des
if- Blocks verwendet, um auf den
Empfänger? .Syntax zuzugreifen . Weiter im Code wird der variable
Empfänger ohne jegliche Überprüfung verwendet, um auf
Empfänger zuzugreifen. Typ,
Empfänger .
Syntax und
Empfänger .
HatErrors . Diese Fehler müssen behoben 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; }
Sie müssen auch sicherstellen, dass der
BoundConditionalReceiver- Konstruktor das Abrufen von
Nullwerten für seine Parameter unterstützt, oder 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 im ZustandV3057 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 eine Bedingung im Code
zulässt ,
löst die
Substring- Methode eine
ArgumentOutOfRangeException aus . Korrektur erforderlich:
if (colon > 0)
Tippfehler ist möglichV3065 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; }); .... }
Zwei Parameter werden an den Lambda-Ausdruck übergeben: t1 und t2. Es wird jedoch nur t1 verwendet. Es sieht verdächtig aus, wenn man bedenkt, wie einfach es ist, Fehler bei der Verwendung von Variablen mit diesen Namen zu machen.
NachlässigkeitV3083 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 unsicher ausgelöst. Zwischen der Überprüfung auf
Nullgleichheit und dem Aufrufen eines Ereignisses haben sie möglicherweise Zeit, sich von ihm abzumelden. Dann wird eine Ausnahme ausgelöst. Darüber hinaus werden im Hauptteil des
if- Blocks unmittelbar vor dem Aufruf des Ereignisses einige andere Operationen ausgeführt. Ich habe diesen Fehler "Unaufmerksamkeit" genannt, weil sie an anderen Stellen im Code mit diesem Ereignis genauer arbeiten, zum Beispiel wie folgt:
private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }
Die Verwendung der optionalen
Handlervariablen beseitigt das Problem. In der
OnTextBufferChanged- Methode müssen
Sie Änderungen für denselben sicheren Vorgang mit dem Ereignis vornehmen.
SchnittpunkteV3092 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 { .... } }
Zum besseren Verständnis werde ich dieses Codefragment neu schreiben und die Konstantennamen durch ihre tatsächlichen Werte ersetzen:
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. Der zweite Teil (
sonst wenn ) wird nur für den Wertebereich von 2147483648 + 1 bis 4294967295 ausgeführt.
Noch ein paar dieser 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 Null-Gleichheitsprüfungen (oder deren Fehlen)Einige
V3095- Fehler beim Überprüfen einer Variablen auf
Null nach ihrer Verwendung. Der erste ist mehrdeutig, betrachten Sie 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 die
displayName- Referenz null sein kann. Überprüfen Sie dazu
Debug.Assert . Es ist nicht klar, warum es nach der Verwendung der Zeichenfolge geht. Es sollte auch
beachtet werden
, dass der Compiler bei anderen Konfigurationen als Debug
Debug.Assert überhaupt aus dem Code entfernt. Bedeutet dies, dass nur für Debug eine Nullreferenz möglich ist? Und wenn dies nicht der
Fall ist, warum wurde dann nicht
string.IsNullOrEmpty (string) überprüft
? Dies sind Fragen 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);
Ich denke, dieser Code bedarf keiner Erklärung. Ich werde die korrigierte Version geben:
internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }
Der Roslyn-Code hat weitere 15 solcher Fehler gefunden:
- 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. 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
Berücksichtigen Sie die Fehler von V3105 . Dies verwendet die bedingten Operator null , wenn die Variable initialisiert wird , aber später in der Codegröße wird ohne prüft auf Gleichheit verwendet null .Der nächste Fehler wird sofort durch zwei Warnungen angezeigt.V3105 Die Variable 'documentId' wurde verwendet, nachdem sie durch einen nullbedingten Operator zugewiesen wurde. NullReferenceException ist möglich. CodeLensReferencesService.cs 138V3105 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 auf null initialisiert werden . Infolgedessen wird beim Erstellen des ReferenceLocationDescriptor eine Ausnahme ausgelöst. Der Code muss behoben werden: return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);
Weiter im Code muss die Möglichkeit der Gleichheit von Nullvariablen vorgesehen werden, die an den Konstruktor übergeben werden.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 KlammernV3123 Möglicherweise arbeitet der Operator '?:' Anders als erwartet. Seine Priorität ist niedriger als die Priorität anderer Betreiber in seinem 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 überhaupt nicht berechnet, wie der Entwickler gedacht hat. Es wurde angenommen, dass die erste Bedingung _kind == other._kin d sein würde (daher wurde nach dieser Bedingung ein Zeilenumbruch durchgeführt), und dann würden Bedingungsblöcke mit dem Operator " ? " Sequentiell berechnet . Tatsächlich ist die erste Bedingung _kind == other._kind && (_oldNode == null) . Dies liegt daran, dass der Operator && eine höhere Priorität hat als der Operator " ? ". Um den Fehler zu beheben, müssen alle Ausdrücke des Operators " ? " In Klammern gesetzt werden : return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));
Damit ist die Beschreibung der gefundenen Fehler abgeschlossen.SchlussfolgerungenTrotz der erheblichen Anzahl von Fehlern, die ich in Bezug auf die Größe des Roslyn-Projektcodes (2.770.000 Zeilen) feststellen konnte, wird dies eine recht kleine Menge sein. Wie Andrei im vorherigen Artikel bin auch ich bereit, die hohe Qualität dieses Projekts zu erkennen.Ich möchte darauf hinweisen, dass solche gelegentlichen Codeprüfungen nichts mit der Methodik der statischen Analyse zu tun haben und praktisch keinen Nutzen bringen. Die statische Analyse sollte regelmäßig und nicht von Fall zu Fall durchgeführt werden. Dann werden viele Fehler in den frühesten Stadien korrigiert, und daher sind die Kosten für ihre Behebung zehnmal niedriger. Diese Idee wird in diesem kleinen Artikel ausführlicher beschrieben , mit dem Sie sich vertraut machen sollen.Sie können sowohl im betrachteten Projekt als auch in jedem anderen unabhängig voneinander nach weiteren Fehlern suchen. Dazu müssen Sie nur unseren Analysator herunterladen und ausprobieren.
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Sergey Khrenov. Überprüfen des Roslyn-Quellcodes .