Non seulement Microsoft a récemment publié le code open source de ses propres projets, mais d'autres sociétés suivent également cette tendance. Pour nous, les développeurs de PVS-Studio, c'est un excellent moyen de tester à nouveau l'analyseur, de voir les choses intéressantes qu'il peut trouver et d'en informer les auteurs du projet. Aujourd'hui, nous regardons à l'intérieur du projet Fast Reports.
Qu'est-ce qui a été vérifié?
FastReport est un générateur de rapports développé par
Fast Reports . Écrit en C # et compatible avec .NET Standard 2.0+. Le code source du projet a été récemment
publié sur GitHub , où il a été téléchargé pour une analyse plus approfondie.
Les rapports prennent en charge l'utilisation de texte, d'images, de lignes, de formes, de diagrammes, de tableaux, de codes à barres, etc. Ils peuvent être d'une seule page et de plusieurs pages, comprenant notamment, en plus des données, une couverture et une page arrière. Les sources de données peuvent être XML, CSV, Json, MS SQL, MySql, Oracle, Postgres, MongoDB, Couchbase, RavenDB, SQLite.
Il existe différentes façons de créer des modèles de rapport: à partir du code; en tant que fichier xml; en utilisant un concepteur en ligne ou FastReport Designer Community Edition.
Si nécessaire, les bibliothèques peuvent être téléchargées en tant
que packages NuGet .
Vous pouvez en savoir plus sur les fonctionnalités du produit sur
la page GitHub du projet .
Il n'y a eu aucun problème avec l'assemblage du projet - je l'ai assemblé à partir de Visual Studio 2017, d'où il a ensuite été vérifié par le plugin PVS-Studio.
PVS-Studio est un analyseur statique qui recherche les erreurs dans le code C, C ++, C #, Java. Lors de l'analyse du code C #, vous pouvez utiliser l'analyseur Visual Studio IDE à l'aide du plug-in PVS-Studio, ou vous pouvez
vérifier les projets à partir de la ligne de commande , pour laquelle l'utilitaire de ligne de commande PVS-Studio_Cmd.exe est utilisé. Si vous le souhaitez, vous pouvez
configurer l'analyse sur le serveur de génération ou
raccorder les résultats de l'analyse à SonarQube .
Voyons ce qui était intéressant cette fois.
Puisque le projet est petit, vous ne devez pas compter sur beaucoup de fautes de frappe et d'endroits suspects. Regardons ce qui a été trouvé et essayons même de reproduire quelque chose en pratique.
Lunettes et plus
Rencontré la méthode suivante:
public override string ToString() { if (_value == null) return null; return this.String; }
Avertissement PVS-Studio :
V3108 Il n'est pas recommandé de renvoyer 'null' à partir de la méthode 'ToSting ()'. Variant.cs 1519
Oui, renvoyer
null à partir d'une méthode
ToString () substituée n'est pas une erreur en soi, mais c'est quand même un mauvais style. Cela est indiqué, entre autres, dans la
documentation de Microsoft :
Votre remplacement ToString () ne doit pas retourner Empty ou une chaîne nulle . Les développeurs qui n'attendent pas de retour
nul comme valeur de retour de
ToString () peuvent être désagréablement surpris lorsqu'une
ArgumentNullException est levée lors de l'exécution du code ci-dessous (à condition que la méthode d'extension soit appelée pour
IEnumerable <T> ).
Variant varObj = new Variant(); varObj.ToString().Contains(character);
Vous pouvez trouver à redire au fait que l'exemple est synthétique, mais l'essence de cela ne change pas.
De plus, ce code est commenté comme suit:
Oups Au lieu de "" retourne
null .
Continuons.
Il existe une classe
FastString dans la
bibliothèque . Description:
Alternative rapide de StringBuilder . En fait, cette classe contient un champ de type
StringBuilder . Les constructeurs de la classe
FastString appellent la méthode
Init , qui initialise le champ correspondant.
Le code de l'un des constructeurs:
public FastString() { Init(initCapacity); }
Et le code de la méthode
Init :
private void Init(int iniCapacity) { sb = new StringBuilder(iniCapacity);
Si vous le souhaitez, vous pouvez accéder au champ
sb via la propriété
StringBuilder :
public StringBuilder StringBuilder { get { return sb; } }
Au total,
FastString a 3 constructeurs:
public FastString(); public FastString(int iniCapacity); public FastString(string initValue);
J'ai déjà montré au corps du premier designer qu'ils font les deux suppositions restantes, je pense, n'est pas non plus difficile. Et maintenant, attention. Devinez ce que le code suivant produira:
FastString fs = new FastString(256); Console.WriteLine(fs.StringBuilder.Capacity);
Attention, la réponse:
De façon inattendue? Regardons le corps du constructeur correspondant:
public FastString(int iniCapacity) { Init(initCapacity); }
Les lecteurs réguliers de nos articles devraient déjà avoir un œil pour trouver un problème ici. L'analyseur oculaire (parfum, logique, appelez-le comme vous voulez) est définitivement engourdi, et il a trouvé un problème: le paramètre de constructeur V3117 'iniCapacity' n'est pas utilisé. FastString.cs 434
Quelle coïncidence si le code de classe contient un champ constant
initCapacity , qui est passé en argument à la méthode
Init au lieu du paramètre constructeur
iniCapacity ...
private const int initCapacity = 32;
Lorsque vous utilisez des noms similaires, vous devez être très, très prudent. Partout où des erreurs ont été commises concernant l'utilisation de noms similaires - projets en C, C ++, C #, Java - il y avait des fautes de ce type partout ...
Soit dit en passant, sur les fautes de frappe.
Faisons l'exemple simple suivant et voyons comment cela fonctionne:
static void Main(string[] args) { TextObject textObj = new TextObject(); textObj.ParagraphFormat = null; Console.WriteLine("Ok"); }
Comme vous l'avez peut-être deviné, la sortie sera différente de la chaîne «Ok» :)
Lequel? Ainsi, par exemple:
Le problème réside dans la propriété
ParagraphFormat et dans l'utilisation de noms similaires:
public ParagraphFormat ParagraphFormat { get { return paragraphFormat; } set { ParagraphFormat = value; } }
PVS-Studio Warning :
V3110 Possible récursion infinie à l'intérieur de la propriété 'ParagraphFormat'. TextObject.cs 281
La propriété
ParagraphFormat est un wrapper sur le champ
paragraphFormat . De plus, son accesseur de propriété get est orthographié correctement, mais l'accesseur de propriété set contient une faute de frappe: au lieu du champ, l'enregistrement se produit dans la même propriété, ce qui provoque une récursivité. Encore une fois, une erreur liée à des noms similaires.
Considérez l'extrait de code suivant.
public override Run Split(float availableWidth, out Run secondPart) { .... if (r.Width > availableWidth) { List<CharWithIndex> list = new List<CharWithIndex>(); for (int i = point; i < size; i++) list.Add(chars[i]); secondPart = new RunText(renderer, word, style, list, left + r.Width, charIndex); list.Clear(); for (int i = 0; i < point; i++) list.Add(chars[i]); r = new RunText(renderer, word, style, list, left, charIndex); return r; } else { List<CharWithIndex> list = new List<CharWithIndex>(); for (int i = point; i < size; i++) list.Add(chars[i]); secondPart = new RunText(renderer, word, style, list, left + r.Width, charIndex); list.Clear(); for (int i = 0; i < point; i++) list.Add(chars[i]); r = new RunText(renderer, word, style, list, left, charIndex); return r; } .... }
PVS-Studio Warning :
V3004 L'
instruction 'then' est équivalente à l'instruction 'else'. HtmlTextRenderer.cs 2092
Un petit copier-coller, et maintenant, quelle que soit la valeur de l'expression
r.Width> availableWidth , les mêmes actions seront effectuées. Supprimez l'
instruction if ou modifiez la logique dans l'une des branches.
public static string GetExpression(FindTextArgs args, bool skipStrings) { while (args.StartIndex < args.Text.Length) { if (!FindMatchingBrackets(args, skipStrings)) break; return args.FoundText; } return ""; }
Avertissement de l'analyseur :
V3020 Un «retour» inconditionnel dans une boucle. CodeUtils.cs 262
En raison de l'
instruction de retour inconditionnelle
, pas plus d'une itération sera exécutée pour la boucle ci-dessus. Peut-être que ce code est sorti après refactoring, ou peut-être que c'est juste une façon inhabituelle de faire ce qui pourrait être fait sans boucle.
private int FindBarItem(string c) { for (int i = 0; i < tabelle_cb.Length; i++) { if (c == tabelle_cb[i].c) return i; } return -1; } internal override string GetPattern() { string result = tabelle_cb[FindBarItem("A")].data + "0"; foreach (char c in text) { int idx = FindBarItem(c.ToString()); result += tabelle_cb[idx].data + "0"; } result += tabelle_cb[FindBarItem("B")].data; return result; }
Avertissement PVS-Studio :
V3106 Valeur d'index négative possible. La valeur de l'indice 'idx' pourrait atteindre -1. BarcodeCodabar.cs 70
Code potentiellement dangereux. La méthode
FindBarItem peut renvoyer
-1 si elle ne trouve pas l'élément passé en tant que paramètre. Dans le code appelant (méthode
GetPattern ), cette valeur est écrite dans la variable
idx et est utilisée comme index du tableau
tabelle_cb sans vérification préalable. Lors de l'accès à l'index
-1, une exception de type
IndexOutOfRangeException sera
levée .
Continuons.
protected override void Finish() { .... if (saveStreams) { FinishSaveStreams(); } else { if (singlePage) { if (saveStreams) { int fileIndex = GeneratedFiles.IndexOf(singlePageFileName); DoPageEnd(generatedStreams[fileIndex]); } else { .... } .... } .... } .... }
PVS-Studio Warning :
V3022 Expression 'saveStreams' est toujours faux. HTMLExport.cs 849
Le code ci-dessus avec l'obtention de la valeur
fileIndex et l'appel de la méthode
DoPageEnd ne
sera jamais exécuté, car le résultat de la deuxième expression
saveStreams dans le code sera toujours
faux .
Des plus intéressants, c'est peut-être tout (vous ne vous attendiez pas à un article dans l'esprit
de l'analyse Mono ?). Il y avait d'autres avertissements de l'analyseur, mais ils ne me semblaient pas assez intéressants pour les inclure dans l'article (une partie reste toujours en coulisses).
Connaître la conception serait utile pour leur analyse, donc idéalement, les auteurs devraient examiner ces avertissements par eux-mêmes. Ce sont des avertissements tels que
V3083 (un appel potentiellement dangereux aux gestionnaires d'événements),
V3022 (la condition est toujours vraie / fausse (dans ce cas, souvent en raison de méthodes qui renvoient une seule valeur)),
V3072 ,
V3073 (travaillant avec
IDisposable ) et autres.
Si tout cela n'est pas pertinent, vous pouvez:
Conclusion
Malgré le fait que l'article était court, j'ai été ravi de «toucher» les avertissements de l'analyseur avec mes mains - pour voir comment ce que jure l'analyseur se manifeste dans la pratique.
Je souhaite aux auteurs du projet de réussir, de corriger les problèmes découverts, et je veux saluer un pas vers la communauté open-source!
Je conseille aux autres d'essayer l'analyseur sur votre code et de voir ce que vous pouvez trouver intéressant.
Meilleurs vœux!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Sergey Vasiliev.
Les rapports les plus rapides du Far West - et une poignée de bugs ...