Endlich geht ein so schwieriges Jahr 2020 zu Ende, was bedeutet, dass es Zeit ist, Bilanz zu ziehen! In diesem Jahr hat das PVS-Studio-Team viele Artikel verfasst, die sich mit verschiedenen Fehlern befassten, die der Analysator in Open-Source-Projekten gefunden hat. Die interessantesten davon finden Sie hier in den TOP-Fehlern, die 2020 in C # -Projekten gefunden wurden. Viel Spaß beim Anschauen!
Wie die Spitze gebildet wurde
Diese Liste enthält meiner Meinung nach die interessantesten Auslöser, über die meine Kollegen und ich in Artikeln für 2020 geschrieben haben. Ein wichtiges Auswahlkriterium war der Grad der Sicherheit, dass tatsächlich ein Fehler im entsprechenden Codefragment gemacht wurde. Und natürlich wurde bei der Auswahl und Platzierung von Orten die „Interessantheit“ des Auslösers berücksichtigt, aber dies ist bereits meine subjektive Meinung - Sie können dies jederzeit in den Kommentaren herausfordern.
Ich habe versucht, das Top so vielfältig wie möglich zu gestalten: sowohl in Bezug auf PVS-Studio-Nachrichten als auch in Bezug auf Projekte, für die Code-Warnungen ausgegeben wurden. Die Liste enthält Erkennungen zu den Quellen von 8 verifizierten Projekten. Gleichzeitig werden die Diagnoseregeln so gut wie nie wiederholt - Sie können hier nur V3022 und zweimal treffen V3106 (und nein, sie wurden nicht von mir gemacht, aber anscheinend sind dies meine Favoriten). Somit ist hier Abwechslung garantiert :).
Nun, fangen wir an! Top 10!
10. Platz - Neue alte Lizenz
Unsere Top-Antwort geht aus einem Artikel einer sehr guten Person über die Überprüfung von C # -Projekten für Linux und MacOS hervor, in dem das RavenDB-Projekt als Beispiel verwendet wurde:
private static void UpdateEnvironmentVariableLicenseString(....)
{
....
if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
return;
....
}
Analyzer-Warnung : V3066 Möglicherweise falsche Reihenfolge der Argumente, die an die Methode 'ValidateLicense' übergeben wurden: 'newLicense' und 'oldLicense'. LicenseHelper.cs (177) Raven.Server
Es scheint, was ist hier falsch? Der Code lässt sich recht gut kompilieren. Warum hat der Analysator entschieden, dass zuerst die alte und dann die neue Lizenz übertragen werden muss ? Du hast es erraten, oder? Werfen wir einen Blick auf die ValidateLicense- Deklaration :
private static bool ValidateLicense(License oldLicense,
RSAParameters rsaParameters,
License newLicense)
Wow, es ist wahr - zuerst ist die alte in den Parametern und erst dann die neue Lizenz. Sagen Sie mir, kann Ihre dynamische Analyse dies erfassen? :)
In jedem Fall ist die Auslösung interessant. Vielleicht ist die Reihenfolge dort nicht wirklich wichtig, aber es ist besser, solche Fragmente noch einmal zu überprüfen, stimme zu?
9. Platz - 'FirstOrDefault' und unerwartete 'Null'
Auf dem 9. Platz stand die Antwort aus dem zu Beginn des laufenden Jahres verfassten Artikel " Play in" osu! ", Fehler nicht vergessen ":
public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
var ruleset = rulesets.GetRuleset(OnlineRulesetID);
var mods = Mods != null ? ruleset.CreateInstance()
.GetAllMods().Where(....)
.ToArray() : Array.Empty<Mod>();
....
}
Sehen Sie den Fehler? Und sie ist! Was sagt der Analysator?
Analyzer-Warnung : V3146 [CWE-476] Mögliche Null-Dereferenzierung von 'Regelsatz'. Der 'FirstOrDefault' kann den Standardwert Null zurückgeben. APILegacyScoreInfo.cs 24
Ja, ja, ich habe wieder nicht alle notwendigen Informationen auf einmal gegeben. Tatsächlich können Sie in diesem Code wirklich nichts Verdächtiges sehen. Immerhin ist FirstOrDefault , von dem der Analysator erzählt, in der Definition der GetRuleset- Methode enthalten :
public RulesetInfo GetRuleset(int id) =>
AvailableRulesets.FirstOrDefault(....);
Schreckliches Geschäft! Die Methode gibt RulesetInfo zurück, wenn sie eine geeignete findet. Und wenn nicht? Geben Sie ruhig null zurück . Und es wird bereits an der Stelle geschossen, an der das Ergebnis des Anrufs verwendet wird. In diesem Fall beim Aufrufen von ruleset.CreateInstance () .
Es kann sich die Frage stellen: Was ist, wenn es niemals Null gibt ? Was ist, wenn die Sammlung immer das erforderliche Element enthält? Wenn der Entwickler sich dessen sicher ist, warum nicht First anstelle von FirstOrDefault verwenden ?
8. Platz - Hallo von Python
Der letzte Auslöser aus den ersten drei wurde für den RunUO-Projektcode ausgegeben. Ein Artikel darüber, wie man es überprüft, wurde im Februar geschrieben und ist hier verfügbar .
Das gefundene Fragment ist äußerst verdächtig, obwohl es schwierig ist, sicher zu sagen, ob es sich um einen Fehler handelt:
public override void OnCast()
{
if ( Core.AOS )
{
damage = m.Hits / 2;
if ( !m.Player )
damage = Math.Max( Math.Min( damage, 100 ), 15 );
damage += Utility.RandomMinMax( 0, 15 );
}
else { .... }
}
Analysator Warnung : V3043 Die Betriebslogik des Codes entspricht nicht seiner Formatierung. Die Anweisung wird rechts eingerückt, aber immer ausgeführt. Möglicherweise fehlen geschweifte Klammern. Earthquake.cs 57
Richtig - es geht um Einrückung! Es scheint , dass die Linie Schaden + = Utility.RandomMinMax (0, 15) sollte nur dann ausgeführt worden sind , wenn m.Player ist falsch... In ähnlicher Weise würde Python-Code funktionieren, bei dem der Einzug nicht nur der Schönheit dient, sondern auch die Logik der Anwendung definiert. Aber der C # -Compiler ist in dieser Angelegenheit anderer Meinung! Ich frage mich, was die Meinung des Entwicklers war.
In dieser Situation gibt es im Allgemeinen zwei Optionen. Entweder fehlen hier wirklich geschweifte Klammern, und alles funktioniert falsch oder alles funktioniert richtig, aber im Laufe der Zeit wird es definitiv eine Person geben, die dies als Fehler betrachtet und es "behebt".
Vielleicht irre ich mich und es gibt wirklich Zeiten, in denen es in Ordnung ist, so etwas zu schreiben. Wenn Ihnen solche Fälle bekannt sind, schreiben Sie bitte einen Kommentar - ich wäre sehr interessiert, mehr über solche Fälle zu erfahren.
7. Platz - Perfekt oder Perfekt - das ist die Frage!
Es wird immer schwieriger, Warnungen an bestimmten Stellen zu verteilen, und wir gehen zum zweiten Alarm in dieser Liste aus dem Artikel über osu über! ...
Wie lange dauert es, bis Sie den Fehler sehen?
protected override void CheckForResult(....)
{
....
ApplyResult(r =>
{
if ( holdNote.hasBroken
&& (result == HitResult.Perfect || result == HitResult.Perfect))
result = HitResult.Good;
....
});
}
Analyzer-Warnung : V3001 Links und rechts von '||' befinden sich identische Unterausdrücke 'result == HitResult.Perfect'. Operator. DrawableHoldNote.cs 266 Nicht
viel, ich denke, Sie müssen nur die PVS-Studio-Warnung lesen. Entwickler, die statische Analysen verwenden, tun dies normalerweise :). Man könnte über die vorherigen Plätze in der Spitze streiten, aber hier ist der Fehler offensichtlich. Es ist schwer zu sagen, welches spezifische Element der HitResult- Aufzählung hier anstelle des zweiten Perfekts (oder anstelle des ersten) hätte verwendet werden sollen, aber am Ende wurde eindeutig etwas falsch geschrieben. Nun, es ist okay - der Fehler wurde gefunden, was bedeutet, dass er leicht behoben werden kann.
6. Platz - null (nicht) bestanden!
Der 6. Platz war eine sehr coole Antwort auf Code aus dem Open XML SDK-Projekt. Den Artikel zum Überprüfen finden Sie hier .
Der Entwickler wollte eine Eigenschaft implementieren , die auch bei direkter Zuweisung nicht null zurückgibt. Und es ist wirklich großartig, wenn Sie sicher sein können, dass null unter keinen Umständen geschrieben wird. Schade, dass dies überhaupt nicht der Fall ist:
internal string RawOuterXml
{
get => _rawOuterXml;
set
{
if (string.IsNullOrEmpty(value))
{
_rawOuterXml = string.Empty;
}
_rawOuterXml = value;
}
}
Parser-Warnung : V3008 Der Variablen '_rawOuterXml' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 164, 161. OpenXmlElement.cs 164
Es stellt sich heraus, dass in _rawOuterXml ein Wert aufgezeichnet wird, unabhängig davon, ob null oder nicht. Wenn Sie diesen Code unaufmerksam betrachten, denken Sie möglicherweise, dass null niemals in diese Eigenschaft geschrieben wird - schließlich gibt es eine Überprüfung! Wenn ja, dann finden Sie unter dem Baum keine Geschenke, sondern eine unerwartete NullReferenceException :(
5. Platz - Aufnahme aus einem Array mit einem Array im Inneren
Die Top 5 Triggerung von 2020 wurde vom Analysator für das von mir persönlich getestete TensorFlow.NET-Projekt veröffentlicht. Wenn Sie auf den Link klicken , können Sie den Artikel über die Überprüfung dieses Projekts lesen (oh, und ich habe alle dort oft gesehen).
Übrigens, wenn Sie Beispiele für interessante Fehler aus echten C # -Projekten sehen möchten, empfehle ich Ihnen, meinen Twitter zu abonnieren . Dort habe ich vor, neugierige Trigger und einfach interessante Codefragmente zu veröffentlichen, von denen viele leider nicht in Artikeln enthalten sein werden. Ich werde froh sein dich zu sehen! :)
Nun, jetzt fahren wir mit dem Auslösen fort:
public TensorShape(int[][] dims)
{
if(dims.Length == 1)
{
switch (dims[0].Length)
{
case 0: shape = new Shape(new int[0]); break;
case 1: shape = Shape.Vector((int)dims[0][0]); break;
case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
default: shape = new Shape(dims[0]); break;
}
}
else
{
throw new NotImplementedException("TensorShape int[][] dims");
}
}
Analyzer-Warnung : V3106 Möglicherweise ist der Index nicht gebunden. Der '1'-Index zeigt über die' dims'-Grenze hinaus. TensorShape.cs 107
Tatsächlich war es sehr schwierig zu entscheiden, wo dieser Auslöser platziert werden soll, da er wirklich interessant ist, aber andere sind in dieser Hinsicht nicht weit dahinter. Also, lasst uns herausfinden, was hier los ist.
Wenn die Anzahl der Arrays in Dims nicht gleich 1 ist, wird eine Ausnahme vom Typ NotImplementedException ausgelöst . Und was passiert, wenn es dunkel ist?genau ein Array? Die Anzahl der Elemente in diesem 'Array innerhalb eines Arrays' wird überprüft. Beachten Sie, was passiert, wenn es 2 ist. Als eines der Argumente für den Shape.Matrix- Konstruktor wird unerwartet dims [1] [2] übergeben . Erinnern wir uns jetzt - wie viele Elemente gab es im Dims- Array ?
Genau das ist richtig - wir haben es gerade überprüft! Ein Versuch, das zweite Element aus einem Array abzurufen , das nur ein Element enthält, löst eine IndexOutOfRangeException- Ausnahme aus . Offensichtlich ein Fehler. Aber gibt es einen offensichtlichen Weg, dies zu beheben?
Das erste, was mir in den Sinn kommt, ist Veränderung dimmt [1] [2] auf dimmt [0] [2] . Wird der Fehler verschwinden? Egal wie es ist! Es wird wieder dieselbe Ausnahme geben. Diesmal hängt es jedoch damit zusammen, dass in diesem Fallzweig die Anzahl der Elemente in dims [0] gleich 2 ist. Hat der Entwickler zwei Fehler im Index hintereinander gemacht? Oder sollte dort eine andere Variable verwendet werden? Wer weiß ... Der Analysator hat die Aufgabe, einen Fehler zu finden, und die Person, die ihn gemacht hat, oder seine Kollegen müssen ihn korrigieren.
4. Platz - Eigenschaft eines Objekts, das nicht existiert
Ein weiterer Auslöser traf dieses Top aus dem Artikel über den OpenRA-Check . Vielleicht verdient es mehr, aber durch den Willen des Schicksals war dieser Auslöser auf dem 4. Platz. Das ist auch ziemlich gut! Schauen wir uns an, welchen Fehler PVS-Studio diesmal feststellen konnte:
public ConnectionSwitchModLogic(....)
{
....
var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
if (logo != null)
{
logo.GetSprite = () =>
{
....
};
}
if (logo != null && mod.Icon == null) // <=
{
// Hide the logo and center just the text
if (title != null)
title.Bounds.X = logo.Bounds.Left;
if (version != null)
version.Bounds.X = logo.Bounds.X;
width -= logo.Bounds.Width;
}
else
{
// Add an equal logo margin on the right of the text
width += logo.Bounds.Width; // <=
}
....
}
Analyzer-Warnung : V3125 Das ' Logo' -Objekt wurde verwendet, nachdem es gegen Null verifiziert wurde. Überprüfen Sie die Zeilen: 236, 222. ConnectionLogic.cs 236
Worauf sollten Sie hier achten? Zunächst einmal kann das Logo wahrscheinlich null enthalten . Dies wird sowohl durch die konstanten Überprüfungen als auch durch den Namen der GetOrNull- Methode angedeutet , die den in das Logo geschriebenen Wert zurückgibt . Wenn ja, lassen Sie uns darüber nachdenken, was passiert, wenn GetOrNull wirklich null zurückgibt . Zuerst ist alles in Ordnung, dann wird der Zustand überprüft logo! = null && mod.Icon == null . Wie Sie sich vorstellen können, ist das Ergebnis ein Übergang zur else- Branche und ... Ein Versuch, auf die Bounds- Eigenschaft der Variablen zuzugreifen , in die null geschrieben ist , und dann - bdysh! NullReferenceException klopft an die Tür.
3. Platz - Schrödingers Element
Schließlich kommen wir zu den drei besten Finalisten. Die drei häufigsten Fehler für 2020 wurden im Nethermind-Projekt gefunden, über dessen Überprüfung ein Artikel mit dem interessanten Titel " Code in einer Zeile oder Überprüfung von Nethermind mit PVS-Studio C # für Linux " geschrieben wurde. Der Fehler ist unglaublich einfach, aber für das menschliche Auge unsichtbar, insbesondere wenn man die Größe des Projekts berücksichtigt. Denken Sie, dass dieser Auslöser seinen Platz verdient?
public ReceiptsMessage Deserialize(byte[] bytes)
{
if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
return new ReceiptsMessage(null);
....
}
Warnung des Analysators : V3106 Möglicherweise ist der Index nicht gebunden. Der '0'-Index zeigt über die gebundenen' Bytes 'hinaus. Nethermind.Network ReceiptsMessageSerializer.cs 50
Wahrscheinlich wäre es cool, das Erste in einer leeren Box zu nehmen, aber hier führt ein solcher Wunsch nur dazu, dass eine IndexOutOfRangeException ausgelöst wird . Nur eine Kleinigkeit - ein kleiner Fehler im Bediener, und die Anwendung funktioniert bereits falsch oder stürzt sogar ab.
Natürlich sollten Sie '||' anstelle von '&&' verwenden. Solche logischen Fehler sind insbesondere bei komplexen Konstruktionen keine Seltenheit. Daher ist es sehr praktisch, solche Momente im automatischen Modus zu überprüfen.
2. Platz - Weniger als 2, aber mehr als 3
An zweiter Stelle habe ich den Code aus dem RavenDB-Projekt noch einmal ausgelöst. Ich möchte Sie daran erinnern, dass Sie die Ergebnisse der Überprüfung (und nicht nur) im entsprechenden Artikel nachlesen können .
Nun, jetzt treffen - der Top 2 Fehler von 2020:
private OrderByField ExtractOrderByFromMethod(....)
{
....
if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
throw new InvalidQueryException(....);
....
}
Analyzer-Warnung : V3022 Ausdruck 'me.Arguments.Count <2 && me.Arguments.Count> 3' ist immer falsch. Wahrscheinlich das '||' Operator sollte hier verwendet werden. QueryMetadata.cs (861) Raven.Server
Früher haben wir die Momente betrachtet, in denen eine unerwartete Ausnahme ausgelöst wurde, und jetzt wird im Gegenteil die erwartete Ausnahme niemals ausgelöst. Nun, oder es wird immer noch weggeworfen, wenn jemand eine Zahl findet, die kleiner als 2, aber größer als 3 ist.
Ich wäre nicht überrascht, wenn Sie nicht einverstanden wären, aber ich mag diesen Auslöser wirklich mehr als alle vorherigen. Ja, der Fehler ist unglaublich einfach. Um ihn zu beheben, müssen Sie nur den Operator ändern. Dies wird übrigens auch durch die an den Konstruktor übergebene Nachricht angedeutet InvalidQueryException : Der Aufruf "Ungültiger ORDER BY 'Spatial.distance (from, to, roundFactor)', erwartete 2-3 Argumente, hat" + me.Arguments.Count .
Ich stimme zu, dies ist ein elementares Versehen, aber niemand hat es bemerkt oder korrigiert, zumindest bis es mit PVS-Studio gefunden wurde. Das erinnert mich daran, dass Programmierer, egal wie erfahren sie sind, (leider?) Immer noch Menschen sind. Und Menschen, unabhängig von ihrer Qualifikation, können aus einer Vielzahl von Gründen selbst solche dummen Fehler übersehen. Manchmal wird der Fehler sofort ausgelöst, und manchmal können Sie sich lange fragen, warum der Benutzer die Meldung über den falschen ORDER BY-Aufruf nicht sieht.
1. Platz - Angebote für + 100% Codesicherheit
Hurra, Hurra, Hurra! Wir haben endlich den Abzug erreicht, den ich am interessantesten, witzigsten, coolsten und so weiter fand. Es wurde für Code aus dem ONLYOFFICE-Projekt herausgegeben, dessen Analyse mit einem der neuesten Artikel dieses Jahres verknüpft ist - " ONLYOFFICE Community Server: Wie Fehler zu Sicherheitsproblemen beitragen ".
Nun, ich präsentiere Ihnen die traurigste Geschichte über ArgumentException , die niemals geworfen wird:
public void SetCredentials(string userName, string password, string domain)
{
if (string.IsNullOrEmpty(userName))
{
throw new ArgumentException("Empty user name.", "userName");
}
if (string.IsNullOrEmpty("password"))
{
throw new ArgumentException("Empty password.", "password");
}
CredentialsUserName = userName;
CredentialsUserPassword = password;
CredentialsDomain = domain;
}
Analyzer-Warnung : V3022 Der Ausdruck 'string.IsNullOrEmpty ("password")' ist immer falsch. SmtpSettings.cs 104
Es war sehr schwierig zu entscheiden, welcher Fehler an welcher Stelle platziert werden sollte, aber dieser Auslöser war für mich von Anfang an der Anführer unter allen. Der einfachste kleine Tippfehler - und der Code funktioniert bereits falsch. Weder die Hervorhebung in der IDE noch die Überprüfung oder der gute alte gesunde Menschenverstand halfen. Es ist eine kleine, einfache, schön geschriebene Funktion. Und auch hier konnte PVS-Studio herausfinden, was die Profis vermisst haben.
Wie immer steckt der Teufel im Detail. Wäre es nicht großartig, wenn alle diese Details automatisch durchsucht würden? Natürlich würde es! Und lassen Sie den Entwickler tun, was ein statischer Analysator nicht kann - er erstellt neue schöne und sichere Anwendungen. Erstellt, ohne darüber nachzudenken, ob er beim Überprüfen einer Variablen zusätzliche Anführungszeichen setzt oder nicht.
Fazit
Es war einfach, in 2020 Artikeln 10 interessante Fehler zu finden. Aber es stellte sich als eine ziemliche Aufgabe heraus, sie an Orten zu verteilen. Einerseits spiegeln einige Trigger die Arbeit fortschrittlicher Analysemechanismen besser wider. Auf der anderen Seite scheinen einige der Fehler nur bis zu einem gewissen Grad lustig zu sein. Viele der vorgestellten Positionen könnten umgekehrt werden - zum Beispiel Top-2 und Top-3.
Oder denken Sie vielleicht, dass diese Liste noch einige andere positive Aspekte enthalten sollte? Sie können sogar Ihr eigenes Top erstellen, indem Sie dem Link folgen zur Liste der Artikel und finden Sie die köstlichsten Auslöser Ihrer Meinung nach. In diesem Fall sollten Sie Ihre 2020-Tops in den Kommentaren abwerfen. Ich werde es mit großer Freude lesen. Können Sie eine Liste interessanter machen als meine?
Natürlich ist die Frage nach der „Interessantheit“ von Warnungen ohnehin subjektiv. Meiner Meinung nach ist das Hauptkriterium für die Bewertung der Antwort, ob ein Programmierer, der eine Warnung von PVS-Studio sieht, etwas im entsprechenden Code ändert. Diese Liste wurde nur aus Triggern für Fragmente zusammengestellt, die meiner Meinung nach besser aussehen würden, wenn die Entwickler statische Analysen verwenden würden. Darüber hinaus gibt es keine Probleme beim Ausprobieren von PVS-Studio, indem Sie Ihre eigenen oder einige andere Projekte überprüfen. Sie müssen nur dem Link folgen Laden Sie dort die erforderliche Version des Analysators herunter und fordern Sie einen Testschlüssel an, indem Sie ein kleines Formular ausfüllen.
Nun, das ist alles für mich, vielen Dank für Ihre Aufmerksamkeit und bis bald!
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Nikita Lipilin. Top-10-Fehler in C # -Projekten im Jahr 2020 gefunden .