phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

In diesem Forum gibt es Starthilfe zum neuen Extension-System von phpBB 3.1/3.2. Fragen zur Entwicklung von Extensions und zur Konvertierung von phpBB 3.0.x MODs sind ebenfalls willkommen.
Benutzeravatar
LukeWCS
Junior Supporter
Beiträge: 515
Registriert: 15.12.2014 10:19
Kontaktdaten:

phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von LukeWCS »

1. Einleitung

phpBB Ext Check hilft dabei, die offiziellen Vorgaben für phpBB-Erweiterungen umzusetzen und eine Mindestqualität des Codes nach phpBB-Standard zu erreichen. Außerdem um Fehler und potentielle Sicherheitslücken aufzudecken.

Das ist ein Sammelthema für Infos, Fragen und Rückmeldungen zu meinem Ext Check (kurz: EC). Dabei handelt es sich um ein (momentan) internes Tool. Ext Check ist ein Web Dienst bei dem einfach ein ZIP hochgeladen werden kann und der mehrere Tools steuert und die Ergebnisse der Prüfungen in einer Übersicht darstellt. Die einzelnen Ergebnisse können dann geöffnet/heruntergeklappt werden, um die Details der jeweiligen Prüfung einsehen zu können. Dabei handelt es sich es um die Bildschirmausgaben die von den jeweiligen Tools erzeugt wurden.

Nachfolgend ein paar Bilder. Getestet wurde eine speziell präparierte Version meines "LF who was here 2" mit konstruierten Fehlern, um eine möglichst grosse Bandbreite an verschiedenen Rückmeldungen seitens der Tools zu erreichen.

Hochladen - mit Infos zur Prozedur: [ externes Bild ]
Bericht - Übersicht der Ergebnisse: [ externes Bild ]
Bericht - einige Ergebnisse geöffnet: [ externes Bild ]

Der Bericht mit der Übersicht und allen Ergebnissen kann optional auch als HTML-Datei lokal gespeichert werden. Das HTML des Berichts wird so generiert, dass keinerlei zusätzliche Dateien oder externe Dienste benötigt werden, wodurch der gespeicherte Bericht auch offline funktionsfähig bleibt.

Wer Erweiterungen entwickelt und den Ext Check Dienst nutzen möchte, kann mir eine PN schicken.

2. Die Tools
  1. Extension Pre Validator - Analysetool zur Überprüfung der Spezifikationen für Erweiterungen nach phpBB-Standard.
  2. PHP_CodeSniffer - Analysetool zur Überprüfung der Programmierrichtlinien nach phpBB-Standard. Prüft PHP, JS und CSS.
    1. PHPCompatibility - Ein Regelwerk für CodeSniffer zur Überprüfung der Code-Kompatibilität einer bestimmten PHP Version.
    2. VariableAnalysis - Ein Regelwerk für CodeSniffer zur Prüfung von Variablen in PHP Dateien.
  3. phpBB33YAMLcheck - Analysetool zur Überprüfung der YAML Dateien auf Kompatibilität mit phpBB 3.3.
1 und 2 sind die offiziellen Tools, mit denen auch bei Travis CI auf Einhaltung der phpBB Vorgaben geprüft wird. Ein schlechtes Ergebnis bei EPV sorgt zum Beispiel dafür, das eine Erweiterung beim hochladen in die CDB direkt abgelehnt wird. 2a, 2b und 3 sind zusätzliche Tools die bei Ext Check eingesetzt werden.

3. Hintergrund zur Entstehung von Ext Check

phpBB Ext Check ist aus dem Wunsch heraus entstanden, ein einfach zu bedienendes Werkzeug zu haben, mit dem man Erweiterungen schon vor der offiziellen Validierung auf Fehler überprüfen kann. Es gibt zwar bereits mit Travis CI eine solche Möglichkeit (zumindest für EPV und CodeSniffer), dazu ist aber zum einen ein GitHub Account erforderlich und zum anderen ist die Einrichtung des Dienstes alles andere als einfach und die Logs sind unübersichtlich.

4. Vergleich mit dem Travis CI Dienst (nach phpBB Vorgaben)
  1. Generelles:
    1. Beim Travis CI Dienst gibt es nur ein einziges Log in dem die Ergebnisse von allen Tools gesammelt werden, ohne optische Trennung der einzelnen Ergebnisse. Bei Ext Check gibt es für jede Prüfung separate Logs, welche auch nur bei Bedarf geöffnet werden müssen und zusätzlich werden die Logs von CodeSniffer, Compatibility, VariableAnalysis und YAMLcheck nach Dateien eingeteilt, was die Übersichtlichkeit weiter verbessert.
    2. Beim Travis CI Dienst gibt es keine Übersicht der einzelnen Ergebnisse. Bei Ext Check sieht man schon auf den ersten Blick, bei welcher Prüfung die Ergebnisse gesichtet werden müssen und bei welcher nicht, inklusive der Information wie viele Warnungen, Fehler usw. jeweils gefunden wurden.
    3. Beim Travis CI Dienst werden viele und für die Fehlerbehebung unnötige Informationen ausgegeben und man muss sich das, was wirklich relevant ist, erst herausfischen (Beispiel "LF who was here 2" [ externes Bild ]). Zum Beispiel sind die ganzen Travis CI Statusmeldungen bei der Fehlerbehebung unerheblich und irritieren nur. Das hängt natürlich damit zusammen, dass Travis CI eine universelle Plattform für unterschiedliche Test-Systeme ist und nicht auf das phpBB Umfeld begrenzt ist. Ext Check ist dagegen auf phpBB Erweiterungen spezialisiert und so werden nur relevante Informationen diesbezüglich ausgegeben und auch die Bildschirmausgaben der Tools werden von irrelevanten Dingen bereinigt. So kann man sich ganz auf die Fehlerbehebung konzentrieren.
  2. Extension Pre Validator:
    1. Der Aufruf von EPV erfolgt exakt genauso wie beim Travis CI Dienst.
  3. PHP_CodeSniffer
    1. Bei Travis CI werden in den Reports von CodeSniffer auch die Sniff-Codes, also die Namen der Regeln ausgegeben, durch die die jeweiligen Fehlermeldungen ausgelöst wurden. Es hat sich gezeigt, dass diese zusätzlichen Informationen gerade CodeSniffer Neulinge eher irritieren und somit kontraproduktiv bei der Problembehebung sind. Darum sind diese bei Ext Check deaktiviert.
    2. Ebenso werden bei Ext Check sämtliche Hinweise bezüglich PHPCBF CAN FIX THE[...] samt zugehörigen [x] und [ ] herausgefiltert. Auch sonst werden bestimmte Anzeigen gefiltert und auf das Wesentliche reduziert.
    3. Bei PHP_CodeSniffer gilt das Konzept "silent by design", was bei Ext Check jedoch unerwünscht ist, da es hier neben präzisen Prüfungen auch auf unmissverständliche Rückmeldungen ankommt. Dazu gehört zum Beispiel auch, dass selbst dann explizite Meldungen ausgegeben werden sollten, wenn es eigentlich nichts zu melden gibt. Darum gibt es bei Ext Check in den Reports von CodeSniffer zusätzliche Informationen am Anfang des Protokolls welcher Regelsatz (Ruleset) verwendet wird, welche Dateitypen (Extensions) überprüft werden, wie viele zutreffende Dateien insgesamt gefunden wurden (Files found) und wie viele Dateien davon Fehler aufweisen (Files affected). Diese Informationen werden immer angezeigt wenn zutreffende Dateien gefunden wurden, egal ob dabei Fehler gefunden wurden oder nicht.

      Code: Alles auswählen

      Ruleset       : phpBB PHP Strict Standard Extensions
      Extensions    : php,js,css
      Files found   : 5
      Files affected: 3
    4. Der oben genannte Informationsblock wird nur dann nicht angezeigt, wenn ein Tool keinerlei zutreffende Dateien gefunden hat, die es untersuchen könnte. Aber auch dieser Fall wird bei Ext Check explizit mit No matching files found gemeldet und in der Übersicht entsprechend mit einem grauen Haken markiert.
    5. Wenn alle zutreffenden Dateien keine Fehler aufweisen, dann wird auch das explizit mit No issues found gemeldet und in der Übersicht mit einem grünen Haken markiert.
    6. Beim Travis CI Dienst werden von CodeSniffer nur .php Dateien überprüft. CodeSniffer ist aber auch für .js und .css geeignet und bei Ext Check werden diese beiden Dateitypen ebenfalls überprüft.
    7. Beim Travis CI Dienst werden die Ordner tests/ und vendor/ ignoriert, das gleiche gilt bei Ext Check.
  4. PHPCompatibility (wird beim Travis CI Dienst nicht eingesetzt):
    1. Es zeigt im Bericht sofort auf einen Blick, mit welchen PHP Versionen die geprüfte Erweiterung kompatibel ist. Somit kann man gezielt Inkompatibilitäten beheben, sofern notwendig. Wenn z.B. eine Erweiterung die phpBB Version 3.2 voraussetzt, wäre es irrelevant, wenn die Erweiterung mit PHP 5.3 inkompatibel ist.
    2. Ausserdem ist die Übersicht der kompatiblen PHP-Versionen sehr hilfreich wenn es darum geht, in composer.json die Minimum- bzw. Maximum-PHP-Version einzutragen. Denn wohl nur die wenigsten Ext Coder dürften aus dem Stand heraus wissen, mit welchen PHP-Versionen ihre Erweiterung tatsächlich kompatibel ist.
    3. Bei Ext Check werden zuerst alle unterstützten PHP Versionen von 5.3 bis 7.4 einzeln geprüft, damit man in der Übersicht sofort sieht, mit welchen PHP-Versionen die Erweiterung kompatibel ist. Zusätzlich wird noch der vollständige Test ausgeführt, bei der auf "cross compatibility" geprüft wird. Dabei geht es speziell um Versions-übergreifende Kompatibilität verschiedener PHP Versionen. Zum Beispiel wird man darauf hingewiesen, wenn sich Parameter bei PHP Funktionen zwischen verschiedenen PHP Versionen geändert haben.
    4. Bei Ext Check werden die Ordner tests/ und vendor/ ignoriert.
  5. VariableAnalysis (wird beim Travis CI Dienst nicht eingesetzt):
    1. Hilft bei der Suche nach unbenutzten und undefinierten Variablen, das ist auch das primäres Aufgabengebiet von VA.
    2. Prüft auch andere Dinge bezüglich Variablen wie z.B. mehrfache globale Deklarationen der gleichen Variable.
    3. Bei Ext Check werden die Ordner tests/, language/ und vendor/ ignoriert.
  6. phpBB33YAMLcheck (wird beim Travis CI Dienst nicht eingesetzt):
    1. Damit werden alle YAML Dateien einer Erweiterung auf die Vorgaben von phpBB 3.3 geprüft.
5. Hinweise
  1. Es ist nicht notwendig, dass man bei jedem Test einen grünen Haken beim Ergebnis bekommt. Im Detail:
    1. Darauf achten sollte man aber unbedingt bei EPV und YAMLcheck und möglichst auch bei CodeSniffer.
    2. Bei PHPCompatibility entscheidet die phpBB Version. Wenn die Erweiterung z.B. ein phpBB 3.2 voraussetzt, wäre PHP 5.3 irrelevant, da mindestens PHP 5.4 vorhanden sein muss. Wenn die Erweiterung ein phpBB 3.3 voraussetzt, wären die PHP Versionen 5.3, 5.4, 5.5, 5.6 und 7.0 irrelevant, da mindestens PHP 7.1 vorhanden sein muss.
      Als Beispiel dient "phpBB3 F1Webtip Extension" von Dr.Death. Vom Zweig "develop" wurde ein EC Bericht erstellt, der die folgende Übersicht zeigt: [ externes Bild ]. Was auf den ersten Blick nach einem schlechten Ergebnis aussieht, ist tatsächlich ein einwandfreies Ergebnis: die Fehler bei 5.3 sind in diesem Fall irrelevant, da ohnehin mindestens phpBB 3.2 vorhanden sein muss. Und der Fehler bei 5.4 ist irrelevant, weil die Mindest-Version mit 7.1 definiert wurde.
    3. Bei VariableAnalysis entscheidet der jeweilige Fall. Meldungen bezüglich | WARNING | Unused variable $xyz. können im Zweifelsfall auch ignoriert werden. Codetechnisch haben unbenutzte Variablen keine Auswirkung, sie sind in den meisten Fällen einfach nur unnötig. Dagegen sollten Meldungen bezüglich | WARNING | Variable $xyz is undefined. auf jeden Fall untersucht werden, weil undefinierte Variablen eine potentielle Fehlerquelle darstellen.
      Grundsätzlich gilt bei VA: Das codetechnische Ermitteln von unbenutzten und nicht-deklarierten Variablen ist keine triviale Angelegenheit. Darum sind die Ergebnisse mit Vorsicht zu betrachten, also nicht blind darauf vertrauen.
  2. Wer bei der Ausführung von Ext Check einen Ausnahmefehler bekommt, sollte mir das bitte hier im Thema oder per PN melden. Dabei das hochgeladene ZIP bitte aufheben und mir bei Gelegenheit zukommen lassen, damit ich das Problem reproduzieren kann.
6. Unterstützte Browser

Ext Check generiert auf allen Seiten W3C-konformes HTML, sollte also mit den meisten Browsern problemlos funktionieren. Bitte auch beachten, dass für den Ext Check Dienst Javascript aktiviert sein muss, was aber in einem phpBB Umfeld ohnehin vorausgesetzt wird. Ext Check wurde (und wird) mit den folgenden Browsern getestet und vereinzelt an diese angepasst.
  • Opera
  • Vivaldi
  • Firefox
  • Internet Explorer
  • Edge
  • Safari
7. Danke!

An dieser Stelle Danke an Kirk sowie chris1278 die durch Verwendung von Ext Check und Rückmeldungen zum heutigen Stand von Ext Check beigetragen haben. Auch Danke an Dr.Death für die Rückmeldungen und Infos bezüglich dem Mac-Upload-Problem und anderen Dingen bei Safari. Außerdem Danke an Crizzo und gn#36.

Besonderer Dank an:

phpBB - Extension Pre Validator
Squiz Labs - PHP_CodeSniffer
PHPCompatibility - PHPCompatibility
Payton Swick - PHP_CodeSniffer VariableAnalysis
SensioLabs - ANSI to HTML5 Converter
Emanuil Rusev - Parsedown
Lehrling - Favicon für Ext Check

8. Zum Thema passende Links

phpBB Coding Guidelines - Die offiziellen Programmierrichtlinien (Englisch).
phpBB Extensions Docs - Die offizielle Dokumentation zu Erweiterungen (Englisch).
Knowledge Base - Editor-übergreifende Format-Vorgaben mit EditorConfig - phpBB Datei-Vorgaben definieren, um bestimmte Fehler zu vermeiden.

Sonstiges

So sah die erste Version von Ext Check Web aus :wink::

Version lang.lang.her - Hochladen [ externes Bild ]
Version lang.lang.her - Bericht [ externes Bild ]
Zuletzt geändert von LukeWCS am 26.05.2020 23:36, insgesamt 136-mal geändert.
Möge das Backup mit dir sein. Immer.

Benutzeravatar
Kirk
Supporter
Supporter
Beiträge: 6715
Registriert: 24.05.2010 08:31
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von Kirk »

Da ich schon des längeren das Vergnügen habe diesen Ext Check zu testen, muss ich sagen das dies sehr nützlich ist. Dadurch konnte ich meine Extension Codetechnisch verfeinern.
Eine tolle Arbeit 👍

Benutzeravatar
chris1278
Mitglied
Beiträge: 1839
Registriert: 12.11.2007 06:20
Wohnort: Euskirchen
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von chris1278 »

Dem kann ich mich nur anschliessen.

HJW
Mitglied
Beiträge: 1147
Registriert: 20.04.2007 20:48
Wohnort: 45481 Mülheim an der Ruhr
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von HJW »

Naja, ich kenne die erst seit heute, aber hatte eigentlich an meinem freien Tag etwas anderes vor :lol:

Es gab und gibt da schon eine lange Liste, die ich noch abarbeiten muss. Was mir aufgefallen ist, dass ich html-Dateien mit Windows Zeilenende hatte, die wurden mir aber nicht angezeigt.

Benutzeravatar
LukeWCS
Junior Supporter
Beiträge: 515
Registriert: 15.12.2014 10:19
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von LukeWCS »

@HJW

Na dann zurück zu deinem freien Tag. :lol: EC läuft dir nicht davon.

Wegen HTML: jupp, gut möglich. Ich kann mit Sicherheit sagen, das HTML weder von CS, noch von PHPCompatibility oder YAMLcheck berücksichtigt werden, weil das ja auch nicht in deren Aufgabengebiet liegt. Bliebe also nur EPV und das müsste ich erst prüfen, ob EPV überhaupt etwas mit HTML zu tun hat, weiss ich gerade aus dem Stand heraus nicht.

Was das Zeilenende angeht, gäbe es aber einen ganz anderen Weg um dieses Problem gar nicht erst aufkommen zu lassen. Das wäre auch z.B. für Tastenplayer interessant. Ich werde dazu noch ein HowTo hier in der Bastelstube erstellen was dazu benötigt wird und wie man es einrichtet. Wenn du z.B. NP++ benutzt, müsstest du dich gar nicht mehr darum kümmern, das würde NP++ dann automatisch für dich erledigen. Sogar am Ende einer Datei ein abschliessendes LF einbauen geht automatisch, wenn man das vergessen haben sollte.
Zuletzt geändert von LukeWCS am 21.01.2020 17:50, insgesamt 1-mal geändert.
Möge das Backup mit dir sein. Immer.

HJW
Mitglied
Beiträge: 1147
Registriert: 20.04.2007 20:48
Wohnort: 45481 Mülheim an der Ruhr
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von HJW »

Eigentlich habe ich das auch so eingerichtet, dass neue Dateien mit dem Unix-Ende geöffnet werden.
Aber wahrscheinlich eine alte Datei als Vorlage genommen.
Bei php wird es aber geprüft.

Benutzeravatar
LukeWCS
Junior Supporter
Beiträge: 515
Registriert: 15.12.2014 10:19
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von LukeWCS »

HJW hat geschrieben:
21.01.2020 17:46
Bei php wird es aber geprüft.
Ja, definitiv, denn PHP ist natürlich im Mittelpunkt von EPV, CS und PHPCompatbility.

Ich habe mir jetzt mal den Source von EPV hinsichtlich HTML angeschaut: bei EPV wurde HTML zumindest als Dateityp deklariert. Es sieht mir aber im Moment nicht danach aus, das irgendwas bezüglich HTML geprüft wird. Das passt ja auch zu deiner Beobachtung, das ein falsches Zeilenende nicht bemerkt wird. Denn wenn schon so etwas elementares nicht geprüft wird, dürfte HTML auch sonst keine (grosse) Rolle spielen. Auf der anderen Seite: wenn im HTML irgendwelche Fehler sind, dann wird maximal etwas gar nicht oder falsch angezeigt. Wenn aber im PHP Fehler vorhanden sind, dann kann das eventuell üble Auswirkungen haben. :wink:

Bei Travis CI wird ja auch beim CodeSniffer mit den offiziellen Regeln von phpBB weder JS noch CSS überprüft. Bei meinem EC dagegen schon, zumindest was die offiziellen Programmierrichtlinien von phpBB angeht. Das heisst EC ist hier noch etwas pingeliger.
Möge das Backup mit dir sein. Immer.

Benutzeravatar
LukeWCS
Junior Supporter
Beiträge: 515
Registriert: 15.12.2014 10:19
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von LukeWCS »

@HJW (gilt aber auch für uns alle die EC nutzen)

Ich habe heute in NP++ 2 Funktionen entdeckt, mit dem man zwei der häufig gemeldeten Fehlermeldungen von CS auf einen Schlag (pro Datei) beheben kann.

1. Bei deinem Calendar (als Beispiel) wird in EPV sehr viel von diesem hier gemeldet (Beispiel):

Code: Alles auswählen

452 | ERROR | Whitespace found at end of line
2. Ausserdem auch einige von diesen:

Code: Alles auswählen

   7 | ERROR | Tabs must be used to indent lines; spaces are not allowed
Beheben mit NP++:

Fehler 1: Bearbeiten > Nicht druckbare Zeichen > Leerzeichen und Tabulatoren am Zeilenende löschen.
Fehler 2: Bearbeiten > Nicht druckbare Zeichen > Leerzeichen in Tabulatoren umwandeln (nur am Zeilenanfang)

Das spart eine ganze Menge nervige Fleissarbeit. Gerade Fehler 1 ist völlig "normal". Es reicht ja schon, wenn man bei einer eingerückten Ebene eine Leerzeile mit Enter einfügt, dann hat man ja bereits Fehler 1 erzeugt.

Und ich habe mich ausserdem gestern mit .editorconfig weiter auseinandergesetzt: man kann NP++ dazu bringen, Fehler 1 gar nicht erst zu produzieren. ^^ Ich muss dringend das HowTo dafür erstellen, weil das vieles erleichtert und einige Fehler so gar nicht erst entstehen.
Möge das Backup mit dir sein. Immer.

HJW
Mitglied
Beiträge: 1147
Registriert: 20.04.2007 20:48
Wohnort: 45481 Mülheim an der Ruhr
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von HJW »

Danke für die Info, da wird so einiges angezeigt. Wenn ich diesen Kleinkram so entfernen kann, geht es wesentlich schneller. Funktioniert hervorragend.

Benutzeravatar
LukeWCS
Junior Supporter
Beiträge: 515
Registriert: 15.12.2014 10:19
Kontaktdaten:

Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

Beitrag von LukeWCS »

HJW hat geschrieben:
22.01.2020 19:15
Wenn ich diesen Kleinkram so entfernen kann, geht es wesentlich schneller.
Ich habe das angesprochene HowTo erstellt. Wenn man mit EditorConfig arbeitet, muss man sich um den besagten Fehler 1 schon mal nicht mehr kümmern, weil er gar nicht erst entsteht. Auch ein abschliessendes LF am Dateiende wird dann automatisch gesetzt, falls vergessen.

edit: hab eben mal zum testen Calendar 0.9.5 alleine über EditorConfig bearbeitet. Das heisst alle von CS gemeldeten Dateien einfach in NP++ geöffnet und gleich wieder abgespeichert. Dann habe ich erneut EC laufen lassen. Die 390 Fehler bei CS wurden alleine durch diese Aktion auf 142 reduziert.
Möge das Backup mit dir sein. Immer.

Antworten

Zurück zu „Extension Bastelstube“