🥅 😱 🏳️ Pylint: Ein detaillierter Test des Code-Analysators 👨🏼‍🔧 🌅 🌸

Als Luke mit Flake8 zusammenarbeitete und Pylint im Auge behielt, hatte er den Eindruck, dass 95% der von Pylint erzeugten Fehler falsch waren. Andere Entwickler hatten andere Erfahrungen mit der Interaktion mit diesen Analysatoren, daher beschloss Luke, die Situation im Detail zu untersuchen und seine Arbeit an 11.000 Zeilen seines Codes zu studieren. Darüber hinaus lobte er die Vorteile von Pylint und betrachtete es als Ergänzung zu Flake8.

Luke ( Luke Plant ) - einer der britischen Entwickler, auf dessen Artikel mit der Analyse beliebter Codeanalysatoren wir kürzlich gestoßen sind. Linters studieren den Code, helfen Ihnen beim Auffinden von Fehlern und machen ihn stilistisch konsistent mit den Standards und dem Code, die die Entwickler in Ihrem Team schreiben. Die häufigsten sind Pylint und Flake8. Wir sind in Leader-IDWir benutzen sie auch, deshalb haben wir seinen Artikel gerne übersetzt. Wir hoffen, dass es Ihre Erfahrung mit diesen Tools bereichern wird.

Grundeinstellungen und Testbasis

Für dieses Experiment habe ich einen Teil des Codes aus einem meiner Projekte übernommen und Pylint mit den Grundeinstellungen gestartet. Dann versuchte er das Ergebnis zu analysieren: Welche Warnungen waren nützlich und welche falsch.

Eine kleine Hilfe zu dem Projekt, aus dem der Code stammt:

Eine reguläre Anwendung, die in Django geschrieben ist (d. H. In demselben Python). Django hat seine eigenen Besonderheiten und als Framework seine Grenzen, aber Sie können normalen Python-Code schreiben. Andere Bibliotheken, die Vorlagen verwenden (Rückruffunktionen oder Entwurfsvorlagen für Vorlagenmethoden), haben auch einige ihrer Nachteile als Framework.
Besteht aus 22.000 Codezeilen. Ungefähr 11.000 Linien gingen durch Pylint (9.000, wenn Lücken weggelassen wurden). Dieser Teil des Projekts bestand hauptsächlich aus Ansichten und Testcode.
Um den Code für dieses Projekt zu analysieren, habe ich bereits Flake8 verwendet, nachdem ich alle empfangenen Fehler verarbeitet habe. Ziel dieses Experiments war es, die Vorteile von Pylint als Zusatz zu Flake8 zu bewerten.
Das Projekt hat eine gute Testabdeckung des Codes, aber da ich sein einziger Autor bin, hatte ich keine Gelegenheit, Peer Review zu verwenden.

Ich hoffe, dass diese Analyse für andere Entwickler nützlich sein wird, um zu entscheiden, ob Pylint im Stapel verwendet werden soll, um die Qualität des Codes zu überprüfen. Ich gehe davon aus, dass Sie, wenn Sie bereits so etwas wie Pylint verwenden, es systematisch für die erforderliche Qualitätskontrolle des Codes verwenden, um Probleme zu minimieren.

Also hat Pylint 1650 Ansprüche auf meinen Code ausgestellt.

Unten habe ich alle Kommentare des Analysators nach Typ gruppiert und ihnen meine Kommentare gegeben. Wenn Sie eine detaillierte Beschreibung dieser Nachrichten benötigen, lesen Sie den entsprechenden Abschnitt der Pylint-Funktionsliste.

Bugs

Pylint hat genau einen Fehler in meinem Code gefunden. Ich betrachte einen Fehler als einen Fehler, der während des Betriebs des Programms auftritt oder möglicherweise auftritt. In diesem Fall habe ich Ausnahmen verwendet - broad-except.das heißt except Exceptionnicht nur, was Flake8 fängt. Dies würde mit einigen Ausnahmen zu einem Laufzeitverhalten führen. Sollte dieser Fehler jemals zur Laufzeit auftreten (nicht die Tatsache, dass er auftritt), hätte ein falsches Codeverhalten jedoch keine schwerwiegenden Konsequenzen ...

Gesamt: 1

Nützlich

Zusätzlich zu diesem Fehler fand Pylint einige weitere, die ich als nützlich eingestuft habe. Der Code ist nicht von ihnen abgefallen, aber es können Probleme beim Refactoring und im Prinzip Fehler in der Zukunft auftreten, wenn die Liste der Funktionen erweitert und der Code unterstützt wird.

Sieben von ihnen waren too-many-locals/ too-many-branches/ too-many-local-variables. Sie gehörten zu drei Teilen meines Codes, die schlecht strukturiert waren. Es wäre schön, über die Struktur nachzudenken, und ich bin sicher, ich könnte es besser machen.

Andere Fehler:

unused-argument× 3 - einer von ihnen war wirklich ein Pfosten, und der Code wurde zufällig korrekt ausgeführt. Die anderen beiden unnötigen und nicht verwendeten Argumente würden in Zukunft zu Problemen führen, wenn ich sie verwenden würde.
redefined-builtin × 2
dangerous-default-value × 2 - keine Fehler, da ich nie die Standardwerte verwendet habe, aber es wäre schön, dies nur für den Fall zu beheben.
stop-iteration-return × 1 - hier habe ich etwas Neues für mich gelernt, ich hätte es selbst nie gefunden.
no-self-argument × 1

Gesamt: 16

Kosmetische Änderungen

Ich würde diesen Dingen weniger Aufmerksamkeit schenken. Sie sind entweder unbedeutend oder unwahrscheinlich. Andererseits ist ihre Korrektur nicht überflüssig. Einige von ihnen sind umstritten stilistisch. Ich habe in anderen Abschnitten über einige ähnliche Pfosten gesprochen, aber die hier aufgeführten sind auch für diesen Kontext geeignet. Wenn ich Pylint regelmäßig benutze, würde ich diese „Fehler“ korrigieren, aber in den meisten Fällen würde ich mir darüber keine Sorgen machen.

invalid-name× 192

Dies waren im Grunde Variablen mit einem Buchstaben. Wird in solchen Kontexten verwendet, in denen es nicht beängstigend war, zum Beispiel:

oder

Viele waren im Testcode:

len-as-condition × 20
useless-object-inheritance × 16 (Python 2-Erbe)
no-else-return × 11
no-else-raise × 1
bad-continuation × 6
redefined-builtin × 4
inconsistent-return-statements × 1
consider-using-set-comprehension × 1
chained-comparison × 1

GESAMT: 252

Nutzlos

Zu diesem Zeitpunkt hatte ich Grund, den Code so zu schreiben, wie ich ihn geschrieben habe, auch wenn er an einigen Stellen ungewöhnlich erscheint. Und meiner Meinung nach ist es besser, es in dieser Form zu belassen, obwohl einige nicht zustimmen oder eine andere Art des Codeschreibens bevorzugen, die möglicherweise Probleme vermeiden könnte.

too-many-ancestors × 76

Waren im Testcode, wo ich viele Klassen von Verunreinigungen verwendet habe , um Dienstprogramme oder Stubs zu platzieren.

unused-variable × 43

Es wurde fast die ganze Zeit im Testcode gefunden, wo ich den Rekord gebrochen habe:

... und hat keines der Elemente verwendet. Es gibt verschiedene Möglichkeiten, um zu verhindern, dass Pylint Fehler meldet (z. B. Namen angeben unused). Aber wenn Sie es in der Form belassen, in der ich geschrieben habe, wird es lesbar sein und die Leute (einschließlich ich) werden es verstehen und unterstützen können.

invalid-name × 26

Dies waren Fälle, in denen ich im Kontext geeignete Namen zugewiesen habe, die jedoch nicht den Pylint-Namensstandards entsprachen. Zum Beispiel db (dies ist eine gebräuchliche Abkürzung für Datenbank) und einige andere nicht standardmäßige Namen, die meiner Meinung nach verständlicher waren. Aber Sie können mir nicht zustimmen.

redefined-outer-name × 16

Manchmal wird ein Variablenname sowohl für interne als auch für externe Kontexte richtig geschrieben. Und Sie müssen niemals einen externen Namen aus einem internen Kontext verwenden.

too-few-public-methods × 14

Beispiele hierfür sind Klassen mit Daten, die mit attrs erstellt wurden , für die keine öffentlichen Methoden vorhanden sind, und eine Klasse, die die Wörterbuchschnittstelle implementiert, die jedoch erforderlich ist, um sicherzustellen, dass die Methode ordnungsgemäß funktioniert__getitem__

no-self-use × 12

Sie wurden im Testcode angezeigt, in dem ich der Basisklasse absichtlich Methoden ohne Parameter hinzugefügt habe self, da es auf diese Weise bequemer war, sie zu importieren und für den Testfall verfügbar zu machen. Einige von ihnen sind sogar in separate Funktionen eingebunden.

attribute-defined-outside-init × 10

In diesen Fällen gab es gute Gründe, den Code so zu schreiben, wie er ist. Grundsätzlich sind diese Fehler im Testcode aufgetreten.

too-many-locals× 6, too-many-return-statements× 6, too-many-branches× 2, too-many-statements× 2

Ja, diese Funktionen waren zu lang. Aber als ich sie ansah, sah ich keine guten Möglichkeiten, sie zu reinigen und zu verbessern. Eines der Merkmale war einfach, wenn auch lang. Es hatte eine sehr klare Struktur und war nicht schief geschrieben, und jede Möglichkeit, sie zu reduzieren, die ich mir vorstellen konnte, würde nutzlose Schichten unnötiger oder unbequemer Funktionen beinhalten.

arguments-differ × 6

Dies ist hauptsächlich auf die Verwendung von * args und ** kwargs in der überschriebenen Methode zurückzuführen, mit der Sie sich vor Änderungen in der Signatur von Methoden aus Bibliotheken von Drittanbietern schützen können (in einigen Fällen kann diese Meldung jedoch auf echte Fehler hinweisen).

ungrouped-imports × 4

Ich benutze bereits isort zum importieren

fixme × 4

Ja, es gibt einige Dinge, die behoben werden müssen, aber im Moment möchte ich sie nicht beheben.

duplicate-code × 3

Manchmal verwenden Sie eine kleine Menge Boilerplate-Code, was einfach notwendig ist, und wenn der Funktionskörper nicht viel echte Logik enthält, wird diese Warnung angezeigt.

broad-except × 2
abstract-method × 2
redefined-builtin × 2
too-many-lines × 1

Ich habe versucht herauszufinden, wie ich dieses Modul auf natürliche Weise brechen kann, konnte es aber nicht. Dies ist ein Beispiel, in dem Sie sehen können, dass der Linter das falsche Werkzeug ist. Wenn ich ein Modul mit 980 Codezeilen habe und weitere 30 hinzufüge, überschreite ich die Grenze von 1000 Zeilen, und Benachrichtigungen vom Linter helfen mir hier nicht weiter. Wenn 980 Zeilen in Ordnung sind, warum ist dann 1010 schlecht? Ich möchte dieses Modul nicht umgestalten, aber ich möchte, dass der Linter keine Fehler erzeugt. Die einzige Lösung, die ich in diesem Moment sehe, besteht darin, irgendwie so zu machen, dass der Linter still ist, und dies widerspricht dem endgültigen Ziel.

pointless-statement × 1
expression-not-assigned × 1
cyclic-import × 1

Wir haben den Zyklus herausgefunden, indem wir seine Teile in eine der Funktionen verschoben haben. Ich konnte keinen besseren Weg finden, um den Code basierend auf Einschränkungen zu strukturieren.

unused-import × 1

Ich habe bereits # NOQAbei Verwendung von Flake8 hinzugefügt , damit dieser Fehler nicht auftritt.

too-many-public-methods × 1

Wenn es in meiner Testklasse 35 statt 20 regulierte Tests gibt, ist das wirklich ein Problem?

too-many-arguments × 1

Gesamt: 243

Kann nicht reparieren

Diese Kategorie spiegelt die Fehler wider, die ich aufgrund externer Einschränkungen nicht beheben kann, selbst wenn ich dies wollte, z. B. die Notwendigkeit, Klassen an eine Bibliothek oder ein Framework eines Drittanbieters zurückzugeben oder zu übergeben, die bestimmte Anforderungen erfüllen müssen.

unused-argument × 21
invalid-name × 13
protected-access × 3

Eingeschlossener Zugriff auf "dokumentierte interne Objekte" wie sys._getframein stdlib und Django Model._meta.

too-few-public-methods × 3
too-many-arguments × 2
wrong-import-position × 2
attribute-defined-outside-init × 1
too-many-ancestors × 1

Gesamt: 46

Falsche Nachrichten

Dinge, in denen Pylint eindeutig falsch liegt. In diesem Fall handelt es sich nicht um Pylint-Fehler: Tatsache ist, dass Python dynamisch ist und Pylint versucht, Dinge zu entdecken, die nicht perfekt oder zuverlässig ausgeführt werden können.

no-member × 395

Verbunden mit mehreren Basisklassen: von Django und denen, die ich selbst erstellt habe. Pylint konnte die Variable aufgrund von Dynamik / Metaprogrammierung nicht erkennen.

Viele Fehler traten aufgrund der Struktur des Testcodes auf (ich habe eine Vorlage von django-functest verwendet, die in einigen Fällen durch Hinzufügen zusätzlicher Basisklassen mithilfe der aufgerufenen "abstrakten" Methoden korrigiert werden konnte NotImplementedError) oder möglicherweise durch Umbenennen vieler Testklassen (I. Ich habe es nicht getan, weil es in einigen Fällen verwirrend wäre.

invalid-name × 52

Das Problem trat hauptsächlich auf, weil Pylint die PEP8-Konstantenregel anwendete , da jeder mit definierte Name der obersten Ebene =eine „Konstante“ ist. Es ist schwieriger als es scheint, genau zu definieren, was wir unter einer Konstanten verstehen, aber dies gilt nicht für einige Dinge, die von Natur aus Konstanten sind, z. B. Funktionen. Die Regel sollte auch nicht auf weniger vertraute Methoden zum Erstellen von Funktionen angewendet werden, z. B.:

Einige Beispiele sind umstritten, da nicht definiert ist, was eine Konstante ist. Sollte beispielsweise eine auf Modulebene definierte Instanz einer Klasse, die einen veränderlichen Status haben kann oder nicht, als konstant betrachtet werden? Zum Beispiel in diesem Ausdruck:

no-self-use × 23

Pylint falsch angegebene Methode könnte eine Funktion für viele Fälle sein, in denen ich Vererbung verwende, um verschiedene Implementierungen durchzuführen, bzw. sie nicht in Funktionen konvertieren kann.

protected-access × 5

Pylint hat fälschlicherweise bewertet, wer der "Eigentümer" war (das aktuelle Codefragment erstellt das geschützte Attribut des Objekts und verwendet es lokal, Pylint sieht dies jedoch nicht).

no-name-in-module × 1
import-error × 1
pointless-statement × 1

Diese Aussage ergab tatsächlich das Ergebnis:

Ich habe dies verwendet, um absichtlich einen ungewöhnlichen Fehler zu verursachen, der bei Tests wahrscheinlich nicht gefunden wird. Ich beschuldige Pylint nicht, es nicht erkannt zu haben ...

Gesamt: 477

Zwischensumme

Wir sind noch nicht am Ziel, aber es ist Zeit, unsere Ergebnisse zu gruppieren:

"Gut" - "Bugs" - und "Utilities" -Blöcke - hier hat Pylint definitiv geholfen: 17.
"Neutral" - "Kosmetische Veränderungen" - ein kleiner Vorteil von Pylint, Fehler verursachen keinen Schaden: 252.
"Schlecht" - "Nutzlos", "Nicht zu beheben", "Ungenauigkeiten" - wo Pylint Änderungen im Code wünscht, wo dies nicht erforderlich ist. Einschließlich, wo Änderungen aufgrund externer Abhängigkeiten nicht vorgenommen werden können oder wo Pylint den Code einfach falsch analysiert hat: 766.

Das Verhältnis von gut zu schlecht ist sehr klein. Wenn Pylint mein Kollege wäre, der bei der Überprüfung des Codes hilft, würde ich ihn bitten, zu gehen.

Um falsche Benachrichtigungen zu entfernen, können Sie die ausgehende Fehlerklasse übertönen (was dann die Nutzlosigkeit der Verwendung von Pylint erhöht) oder dem Code spezielle Kommentare hinzufügen . Letzteres möchte ich nicht tun, weil:

Es braucht Zeit!
Ich mag es nicht, wenn sich Kommentare häufen, um den Linter zum Schweigen zu bringen.

Ich werde diese Pseudokommentare gerne hinzufügen, wenn es ein definitives Plus vom Linter gibt. Darüber hinaus bin ich besorgt über Kommentare, sodass meine Syntaxhervorhebung sie anschaulich anzeigt: wie in diesem Artikel empfohlen . An einigen Stellen habe ich jedoch bereits # Kommentare NOQAzu Muffle Flake8 hinzugefügt, aber mit ihnen können Sie für einen Abschnitt nur fünf Fehlercodes hinzufügen.

Docstrings

Die verbleibenden Probleme, die Pylint entdeckte, waren fehlende Dokumentationszeilen. Ich habe sie in eine separate Kategorie eingeordnet, weil:

Sie sind sehr kontrovers und Sie haben möglicherweise eine völlig andere Politik in Bezug auf solche Dinge.
Ich hatte keine Zeit, sie alle zu analysieren.

Insgesamt entdeckte Pylint 620 fehlende Dockstrings (in Modulen, Funktionen, Klassenmethoden). Aber in vielen Fällen hatte ich recht. Zum Beispiel:

Wenn aus dem Namen alles klar ist. Zum Beispiel:
Wenn bereits ein Docstring definiert ist - zum Beispiel, wenn ich die Datenbank-Router- Schnittstelle des Django implementiere . Das Hinzufügen einer eigenen Zeile kann in diesem Fall gefährlich sein.

In anderen Fällen würden diese Beschreibungszeilen meinen Code nicht verletzen. In etwa 15–30% der von Pylint gefundenen Fälle hätte ich gedacht: „Ja, ich muss hier einen Dokumentstring hinzufügen, danke Pylint für die Erinnerung.“

Im Allgemeinen mag ich die Tools nicht, mit denen Dockstrings überall und überall hinzugefügt werden können, da ich denke, dass sie in diesem Fall schlecht ausfallen werden. Es ist besser, sie überhaupt nicht zu schreiben. Eine ähnliche Situation mit schlechten Kommentaren:

Sie zu lesen ist Zeitverschwendung: Sie haben keine zusätzlichen Informationen oder sie enthalten falsche Informationen.
Sie helfen dabei, Dokumentstrings im Text unbewusst hervorzuheben, da sie nützliche Informationen enthalten (wenn Sie sie in diesem Fall schreiben).

Warnungen vor fehlenden Dokumentationszeilen sind ärgerlich: Um sie zu entfernen, müssen Sie Kommentare separat hinzufügen. Dies dauert ungefähr genauso lange wie das Hinzufügen des Andockvorgangs. Außerdem erzeugt alles visuelle Geräusche. Als Ergebnis erhalten Sie Dokumentationszeilen, die nicht notwendig oder nützlich sind.

Fazit

Ich glaube, dass sich meine anfänglichen Annahmen über die Nutzlosigkeit von Pylint als richtig erwiesen haben (im Kontext der Codebasis, für die ich Flake8 verwende). Damit ich es verwenden kann, sollte der Indikator, der die Anzahl der falsch positiven Ergebnisse widerspiegelt, niedriger sein.

Zusätzlich zur Erzeugung von visuellem Rauschen ist zusätzliche Zeit erforderlich, um falsche Benachrichtigungen auszusortieren oder herauszufiltern, und ich möchte dem Projekt nichts davon hinzufügen. Junior-Entwickler werden bescheidener sein, Änderungen vorzunehmen, um Pylints Kommentare zu entfernen. Dies führt jedoch dazu, dass sie den Arbeitscode brechen und nicht erkennen, dass Pylint falsch ist, oder dass Sie infolgedessen viele Umgestaltungen vornehmen müssen, damit Pylint Ihren Code richtig verstehen kann.

Wenn Sie Pylint von Anfang an in einem Projekt oder in einem Projekt verwenden, in dem es keine externen Abhängigkeiten gibt, werden Sie meiner Meinung nach eine andere Meinung haben und die Anzahl falscher Benachrichtigungen wird geringer sein. Dies kann jedoch zu zusätzlichen Zeitkosten führen.

Ein anderer Ansatz besteht darin, Pylint für eine begrenzte Anzahl von Fehlerarten zu verwenden. Es gab jedoch nur wenige, deren Antworten sich als richtig oder äußerst selten falsch herausstellten (relativ und absolut). Unter ihnen sind: dangerous-default-value, stop-iteration-return, broad-exception, useless-object-inheritance.

Auf jeden Fall hoffe ich, dass dieser Artikel Ihnen bei der Überlegung geholfen hat, ob Sie Pylint verwenden oder in einem Streit mit Kollegen.

Pylint: Ein detaillierter Test des Code-Analysators