Das PVS-Studio-Team entwickelt nun aktiv einen statischen Analysator für C#-Code. Die erste Version wird für Ende 2015 erwartet. Und jetzt ist es meine Aufgabe, ein paar Artikel zu schreiben, um C#-Programmierer vorab auf unser Tool aufmerksam zu machen. Ich habe heute ein aktualisiertes Installationsprogramm, sodass wir jetzt PVS-Studio mit aktivierter C#-Unterstützung installieren und sogar Quellcode analysieren können. Ohne weiteres Zögern entschied ich mich, das Programm zu scannen, das ich gerade zur Hand hatte. Dies war zufällig das Umbraco-Projekt. Natürlich können wir von der aktuellen Version des Analysers nicht allzu viel erwarten, aber seine Funktionalität hat mir gereicht, um diesen kleinen Artikel zu schreiben.
Umbraco
Umbraco ist eine Open-Source-Content-Management-Systemplattform zur Veröffentlichung von Inhalten im World Wide Web und in Intranets. Es ist in C# geschrieben und seit Version 4.5 ist das gesamte System unter einer MIT-Lizenz verfügbar.
Das Projekt ist mittelgroß, aber sein C#-Anteil ist ziemlich klein, während der Großteil des Codes in JavaScript geschrieben ist. Insgesamt besteht das Projekt aus 3200 ".cs"-Dateien, die insgesamt 15 MByte ausmachen. Die Anzahl der C#-Codezeilen beträgt 400 KLOC.
Über PVS-Studio 6.00
Die Analyse für diesen Artikel wurde mit der Alpha-Version von PVS-Studio 6.00 durchgeführt. Die Veröffentlichung wird zwei wesentliche Änderungen enthalten:
- C#-Unterstützung hinzugefügt.
- Deaktivierte Unterstützung für VS2005 und VS2008. Der kleinen Anzahl unserer Benutzer, die noch mit diesen IDEs arbeiten, wird empfohlen, weiterhin Version 5.31 oder nächste Versionen zu verwenden, wenn sie beabsichtigen, Fehler zu beheben.
Die Preispolitik ändert sich nicht. Wir stellen kein neues Produkt her; Wir erweitern lediglich die Fähigkeiten der bestehenden, indem wir einfach die Unterstützung für eine weitere Programmiersprache einführen. Bisher konnten Sie mit PVS-Studio Projekte scannen, die in den Sprachen C, C++, C++/CLI und C++/CX geschrieben wurden. Jetzt haben Sie die Möglichkeit, auch C#-Projekte zu analysieren. Dies wirkt sich in keiner Weise auf den Preis aus. Diejenigen, die das Tool zur Analyse von C++-Code bereits erworben haben, können auch C#-Code analysieren.
Warum C#?
Auf Konferenzen argumentierte ich oft, dass das Erstellen eines C#-Analyzers nicht nach einem interessanten Job aussah. Viele Fehler, die C++ eigen sind, sind in C# einfach unmöglich. Und das ist wirklich so. C# hat beispielsweise keine Funktionen wie memset(); daher leidet es nicht unter den damit verbundenen Unmengen an Problemen (siehe Beispiele für memset():V511, V512, V575, V579, V597, V598).
Aber nach und nach änderte ich meine Meinung. Sie sehen, die meisten von PVS-Studio entdeckten Fehler haben eher mit der Nachlässigkeit der Programmierer als mit Sprachspezifika zu tun. Mit Nachlässigkeit meine ich Tippfehler und schlechte Modifikationen von Copy-Paste-Code. Darin ist der PVS-Studio-Analysator wirklich gut, und wir dachten, was in C++ geholfen hat, würde auch in C# helfen.
Die C#-Sprache schützt Sie nicht vor der Eingabe eines falschen Variablennamens oder dem „letzten Zeileneffekt“, der mit mangelnder Aufmerksamkeit zu tun hat.
Eine weitere wichtige Sache, die uns veranlasste, einen C#-Analyzer zu entwickeln, war die Veröffentlichung von Roslyn. Ohne sie wäre die Entwicklung einfach zu kostspielig gewesen.
Roslyn ist eine Open-Source-Plattform zur Analyse und Kompilierung von C#- und Visual Basic-Sprachen. Roslyn führt zwei grundlegende Operationen aus:Es erstellt einen Syntaxbaum (Parsing) und kompiliert ihn. Darüber hinaus können Sie den Quellcode analysieren, ihn rekursiv durchlaufen, Visual Studio-Projekte handhaben und den Code zur Laufzeit ausführen.
Interessante Fehler im Projekt gefunden
Für C++ ist meine Lieblingsdiagnose V501. Jetzt hat es auch ein Gegenstück im C#-Modul - V3001. Beginnen wir mit diesem hier.
Codebeispiel Nr. 1
Es gibt ein Attribut namens "focalPoint":
[DataMember(Name = "focalPoint")]
public ImageCropFocalPoint FocalPoint { get; set; }Dieses Attribut ist vom Typ „ImageCropFocalPoint“, der wie folgt definiert ist:
public class ImageCropFocalPoint
{
[DataMember(Name = "left")]
public decimal Left { get; set; }
[DataMember(Name = "top")]
public decimal Top { get; set; }
}Es ist schwer, einen Fehler zu machen, wenn man mit einem solchen Attribut arbeitet, nicht wahr? Nun, der Autor dieses Codes hat einen gemacht - ein trauriger Tippfehler in der Methode HasFocalPoint():
public bool HasFocalPoint()
{
return FocalPoint != null &&
FocalPoint.Top != 0.5m && FocalPoint.Top != 0.5m;
}'Oben' wird zweimal überprüft, während 'Links' überhaupt nicht überprüft wird.
Diagnosemeldung von PVS-Studio:V3001 Es gibt identische Unterausdrücke 'FocalPoint.Top !=0.5m' Links und rechts vom '&&'-Operator. ImageCropDataSet.cs 58
Codebeispiel Nr. 2
protected virtual void OnBeforeNodeRender(ref XmlTree sender,
ref XmlTreeNode node,
EventArgs e)
{
if (node != null && node != null)
{
if (BeforeNodeRender != null)
BeforeNodeRender(ref sender, ref node, e);
}
}Diagnosemeldung von PVS-Studio:V3001 Es gibt identische Teilausdrücke 'node !=null' links und rechts vom '&&'-Operator. BaseTree.cs 503
Die 'Knoten'-Referenz wird zweimal geprüft. Vermutlich sollte auch der 'Absender'-Hinweis überprüft werden.
Codebeispiel Nr. 3
public void Set (ExifTag key, string value)
{
if (items.ContainsKey (key))
items.Remove (key);
if (key == ExifTag.WindowsTitle || // <=
key == ExifTag.WindowsTitle || // <=
key == ExifTag.WindowsComment ||
key == ExifTag.WindowsAuthor ||
key == ExifTag.WindowsKeywords ||
key == ExifTag.WindowsSubject) {
items.Add (key, new WindowsByteString (key, value));
....
}Diagnosemeldung von PVS-Studio:V3001 Es gibt identische Unterausdrücke 'key ==ExifTag.WindowsTitle' links und rechts vom '||' Operator. ExifPropertyCollection.cs 78
'key' wird zweimal mit der Konstante 'ExifTag.WindowsTitle' verglichen. Wie schwerwiegend dieser Fehler ist, kann ich nicht sagen. Vielleicht ist eines der Häkchen einfach überflüssig und kann entfernt werden. Es ist aber auch möglich, dass der Vergleich über eine andere Variable erfolgen soll.
Codebeispiel Nr. 4
Hier ist ein weiteres Beispiel, bei dem ich nicht sicher bin, ob ein echter Fehler vorliegt. Dieser Code ist jedoch immer noch eine Überprüfung wert.
Wir haben eine Aufzählung mit 4 benannten Konstanten:
public enum DBTypes
{
Integer,
Date,
Nvarchar,
Ntext
}Aus irgendeinem Grund behandelt die Methode SetProperty() nur 3 Optionen. Nochmals, ich sage nicht, dass dies ein Fehler ist. Aber der Analysator schlägt vor, dieses Fragment zu überprüfen, und ich stimme ihm vollkommen zu.
public static Content SetProperty(....)
{
....
switch (((DefaultData)property.PropertyType.
DataTypeDefinition.DataType.Data).DatabaseType)
{
case DBTypes.Ntext:
case DBTypes.Nvarchar:
property.Value = preValue.Id.ToString();
break;
case DBTypes.Integer:
property.Value = preValue.Id;
break;
}
....
}Diagnosemeldung von PVS-Studio:V3002 Die switch-Anweisung deckt nicht alle Werte des Enums 'DBTypes' ab:Date. ContentExtensions.cs 286
Codebeispiel Nr. 5
public TinyMCE(IData Data, string Configuration)
{
....
if (p.Alias.StartsWith("."))
styles += p.Text + "=" + p.Alias;
else
styles += p.Text + "=" + p.Alias;
....
}Diagnosemeldung von PVS-Studio:V3004 Die 'then'-Anweisung entspricht der 'else'-Anweisung. TinyMCE.cs 170
Codebeispiel Nr.6, Nr.7
Am Anfang des Artikels habe ich gesagt, dass C# Sie nicht vor dem „Letzte-Zeile-Effekt“ schützt. Hier ist ein Beispiel, um das zu beweisen:
public void SavePassword(IMember member, string password)
{
....
member.RawPasswordValue = result.RawPasswordValue;
member.LastPasswordChangeDate = result.LastPasswordChangeDate;
member.UpdateDate = member.UpdateDate;
}Diagnosemeldung von PVS-Studio:V3005 Die Variable 'member.UpdateDate' ist sich selbst zugewiesen. MemberService.cs 114
Der Programmierer kopierte Klassenmitglieder aus dem Objekt „Ergebnis“ nach „Mitglied“. Aber am Ende entspannte er sich und kopierte unwissentlich das Mitglied 'member.UpdateDate' in sich selbst.
Eine weitere Sache, die mich an diesem Code misstrauisch macht, ist, dass die Methode SavePassword() mit Passwörtern umgeht, und das bedeutet, dass man damit besonders vorsichtig sein muss.
Dasselbe Codefragment befindet sich in der Datei UserService.cs (siehe Zeile 269). Meine Vermutung ist, dass der Programmierer es einfach ungeprüft dorthin kopiert hat.
Codebeispiel Nr. 8
private bool ConvertPropertyValueByDataType(....)
{
if (string.IsNullOrEmpty(string.Format("{0}", result)))
{
result = false;
return true;
}
....
return true;
....
return true;
....
return true;
....
return true;
....
....
return true;
}Diagnosemeldung von PVS-Studio:V3009 Merkwürdig ist, dass diese Methode immer ein und denselben Wert „true“ zurückgibt. DynamicNode.cs 695
Die Methode verwendet viele 'if'- und 'return'-Anweisungen. Was für mich nicht richtig aussieht, ist, dass alle 'return'-Anweisungen 'true' zurückgeben. Ist da nicht irgendwo ein Bug? Was ist, wenn einige davon 'false' zurückgeben sollten?
Codebeispiel Nr. 9
Lassen Sie uns nun Ihre Aufmerksamkeit testen:Versuchen Sie, einen Fehler im folgenden Codefragment zu finden. Untersuchen Sie einfach die Methode, aber lesen Sie nicht meine Erklärung danach. Um zu verhindern, dass Sie es versehentlich lesen, habe ich ein Trennzeichen eingefügt (ein Einhornbild :).
public static string GetTreePathFromFilePath(string filePath)
{
List<string> treePath = new List<string>();
treePath.Add("-1");
treePath.Add("init");
string[] pathPaths = filePath.Split('/');
pathPaths.Reverse();
for (int p = 0; p < pathPaths.Length; p++)
{
treePath.Add(
string.Join("/", pathPaths.Take(p + 1).ToArray()));
}
string sPath = string.Join(",", treePath.ToArray());
return sPath;
}
Abbildung 1. Code von Erklärung trennen.
Diagnosemeldung von PVS-Studio:V3010 Der Rückgabewert der Funktion 'Reverse' muss verwendet werden. DeepLink.cs 19
Beim Aufruf der Methode Reverse() wollte der Programmierer das Array 'pathPaths' ändern. (S)er wurde wahrscheinlich dadurch in die Irre geführt, dass eine solche Operation völlig korrekt ist, wenn wir mit Listen (List
Ein korrekter Weg, dies zu tun, wäre wie folgt:
string[] pathPaths = filePath.Split('/');
pathPaths = pathPaths.Reverse().ToArray();Oder sogar so:
string[] pathPaths = filePath.Split('/').Reverse().ToArray();Codebeispiel Nr. 10
Der PVS-Studio-Analysator gab einige V3013-Warnungen aus, die einige Methoden meldeten, deren Körper seltsam ähnlich aussahen. Meiner Meinung nach sind das alles Fehlalarme. Nur eine der Warnungen ist wahrscheinlich einen Blick wert:
public void GetAbsolutePathDecoded(string input, string expected)
{
var source = new Uri(input, UriKind.RelativeOrAbsolute);
var output = source.GetSafeAbsolutePathDecoded();
Assert.AreEqual(expected, output);
}
public void GetSafeAbsolutePathDecoded(string input, string expected)
{
var source = new Uri(input, UriKind.RelativeOrAbsolute);
var output = source.GetSafeAbsolutePathDecoded();
Assert.AreEqual(expected, output);
}Diagnosemeldung von PVS-Studio:V3013 Es ist merkwürdig, dass der Hauptteil der Funktion „GetAbsolutePathDecoded“ vollständig dem Hauptteil der Funktion „GetSafeAbsolutePathDecoded“ entspricht. UriExtensionsTests.cs 141
Innerhalb der Methode GetAbsolutePathDecoded() müssen wir möglicherweise
verwendensource. GetAbsolutePathDecoded()statt
source.GetSafeAbsolutePathDecoded()Ich bin mir da nicht sicher, aber diese Stelle sollte inspiziert werden.
Häufig gestellte Fragen
Der Artikel ist für ein neues Publikum gedacht, daher erwarte ich eine Reihe von Fragen, die die Leute vielleicht stellen möchten. Ich werde versuchen, diese Fragen im Voraus zu beantworten.
Haben Sie die Fehler, die Sie gefunden haben, den Projektentwicklern gemeldet?
Ja, wir versuchen dies ständig.
Lassen Sie PVS-Studio auf sich selbst laufen?
Ja.
Unterstützt PVS-Studio Mono?
Nein.
Ausführlichere Antworten auf diese und andere Fragen finden Sie im Beitrag "Leser-FAQ zu Artikeln über PVS-Studio".
Schlussfolgerung
Es gibt nicht viele Fehler in diesem Projekt. Unsere C++-orientierten Leser wissen, warum das so ist, aber da wir noch C#-Programmierer in unser Lager locken und locken müssen, werde ich hier einige wichtige Punkte klarstellen:
- Ein statischer Analysator ist ein Werkzeug für den regelmäßigen Gebrauch. Sein Zweck ist es, Fehler in der frühesten Entwicklungsphase zu finden. Das gelegentliche Ausführen macht wenig Sinn, da die Verwendung auf diese Weise nur dabei hilft, unkritische Fehler oder Fehler in selten ausgeführtem Code zu erkennen. Der Grund ist, dass zwischen diesen Läufen die eigentlichen Fehler mit enormem Aufwand behoben werden. Sie werden von Programmierern gefunden, die dann Stunden damit verbringen, den Code zu debuggen; sie werden von Testern entdeckt; oder, was am schlimmsten ist, sie werden von Benutzern gemeldet. Viele dieser Fehler könnten sofort gefunden und behoben werden, wenn Sie den Analysator regelmäßig verwenden. Behandeln Sie PVS-Studio also als Erweiterung der Warnungen des C#-Compilers. Hoffentlich überprüfen Sie nicht einmal im Jahr die Liste der Compiler-Warnungen, oder? All diese Dinge werden ausführlicher im Artikel "Leo Tolstoi und die statische Codeanalyse" behandelt.
- In unseren Artikeln erwähnen wir nur die Codefragmente, die wir interessant und erwähnenswert finden. Wir diskutieren im Allgemeinen keine Fälle, in denen der Analysator ernsthaft einen Fehler in einem Code vermutet, obwohl er eigentlich sauber ist. Wir nennen einen solchen Code „riechenden Code“. Wenn Sie PVS-Studio verwenden, sollten Sie solche Fragmente besser überprüfen. Aber sie in Artikeln zu diskutieren, ist nebensächlich.
- Wir haben dieses Element nicht für den C++-Teil des Analysators, aber es ist für C# relevant. Bisher sind nur wenige Diagnosen für dieses Modul implementiert, aber wir entwickeln uns schnell weiter. Lass unser C#-Einhorn einfach ein bisschen wachsen - und dann zeigt es dir, wie cool es ist!
Vielen Dank für das Lesen dieses Artikels und mögen Ihre Programme fehlerfrei bleiben!