phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[oxpus]

Nun, man hat in jeder Testsuite Kinderkrankheiten.
Daher baust Du an Deinen Tools ja auch ständig weiter
.
Ich habe durch den Fehler das Paket schrumpfen können.
Aber andere Extensions dürfte der Fehler weniger gefallen...
Bleibt abzuwarten, ob es bald neue Coding Vorgaben gibt

[LukeWCS]

Moin

Nun, man hat in jeder Testsuite Kinderkrankheiten.
Daher baust Du an Deinen Tools ja auch ständig weiter
...
Aber andere Extensions dürfte der Fehler weniger gefallen...

Jupp, eine Software ist nie fertig geschweige denn perfekt. GA hat so ein paar Unzulänglichkeiten, aber zumindest was die Analysetools angeht, sind wir ja auch nicht zwingend auf GA angewiesen.

Seit einer Woche bin ich mit einem neuen Minor Release (1.5.0) beschäftigt, bei dem gleich eine ganze Reihe kleinerer Änderungen und Neuerungen enthalten sein werden.

Dabei stand für mich auch die Frage im Raum, ob man den doppelten Test auf Kreuz-Kompatibilität - also sowohl mit PHPC 9.3.5 als auch 10.0.0 - nur noch mit PHPC 10.0.0 ausführt. Diesbezüglich habe ich dann an die Entwicklerin von PHPC eine Anfrage gestellt und inzwischen auch beantwortet bekommen. Juliette's Antwort war wie immer präzise und ausführlich. Damit wird das Prüfmodul "PHPCompatibility X" ab EC 1.5.0 wegfallen, es wird nur noch "PHPCompatibility X (dev)" ausgeführt, also mit PHPC 10.0.0. Somit reduziert sich die Anzahl Prüfmodule von 16 auf 15, ergo schnellere Ausführung und weniger zu berücksichtigen. Dieser Schritt war eigentlich erst mit dem Release von PHPC 10 geplant und kann jetzt schon erfolgen.

[LukeWCS]

Version 1.5 ist jetzt online.

• Globale Navigationsleiste um auf jeder Seite jede andere Seite erreichen zu können.
• Prüfung auf Kreuz-Kompatibilität erfolgt nicht mehr doppelt mit PHPC 9.3 und PHPC 10.0, sondern nur noch mit 10.0.
• composer.json wird ab sofort auf gültige JSON Struktur und Extension-Merkmal geprüft. Wenn ungültig bzw. nicht vorhanden, wird das Archiv abgelehnt.

Alle Details und weitere Änderungen/Neuerungen sind im Changelog nachzulesen. Dazu wird eine Tasse Kaffee/Tee empfohlen. (Hinweis: Nicht im Lieferumfang von EC enthalten).

[oxpus]

Jo, löppt wie gewohnt.

[LukeWCS]

Jo, löppt wie gewohnt.

Speziell für dich hätte ich noch einen Zusatz. In Kombination mit einem CS Befehl und einer VA Variable kann man ganz gezielt für bestimmte Variablen eine bestimmte Prüfung deaktivieren. Gefunden hatte ich die Möglichkeit bei einer grösseren "Schnitzeljagd" auf Github und etlichen Versuchen. Ich nutze das selbst seit einiger Zeit im EC Source, weil mir VA sonst zuviele false-positives meldet bezüglich "unbenutzter Variablen". Das kann man aber auch für "nicht deklarierte Variablen" nutzen und das könnte bei dir wie folgt aussehen:

Als Beispiel habe ich mir controller\search.php rausgepickt, weil es da gleich 18 Meldungen bezüglich "undeklariert" gibt.

Code: Alles auswählen

<?php

/**
 *
 * @package   phpBB Extension - Oxpus Downloads
 * @copyright 2002-2021 OXPUS - www.oxpus.net
 * @license   http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
 *
 */

// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUndefinedVariableNames sql_array matching_userids

{code}

// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUndefinedVariableNames

Die Variable muss also ganz am Anfang einer Datei gesetzt und ganz am Ende wieder gelöscht werden. Somit kann präzise eine bestimmte Prüfung für bestimmte Variablen ausgeschaltet werden. Das muss halt einmalig gemacht werden, aber so kriegt man die false-positives in den Griff.

Liste der möglichen VA Variablen: https://github.com/sirbrillig/phpcs-var ... tomization

[oxpus]

Dass kann ich gerne mal versuchen.
Vielleicht bekomme ich ja den gesamten Test damit grün.

Und ein Fehler wäre es ja auch nicht, auch nicht für die Prüfungen auf phpbb.com...

Im Voraus schon mal ein dickes Danke schön für das Suchen.
Die Geduld hätte ich jetzt nicht gehabt

[LukeWCS]

Dass kann ich gerne mal versuchen.
Vielleicht bekomme ich ja den gesamten Test damit grün.

Sollte klappen. Ebenso könnte man in dieser Datei nach dem gleichen Muster auch eine zweite Variable setzen, um z.B. auch die Event Rückgabe-Variable $block von der Suche nach "unbenutzten" Variablen auszuschliessen.

Und ein Fehler wäre es ja auch nicht, auch nicht für die Prüfungen auf phpbb.com...

Da es ein Kommentar ist, ist das nichts was PHP oder phpBB stören könnte.

Im Voraus schon mal ein dickes Danke schön für das Suchen.
Die Geduld hätte ich jetzt nicht gehabt

Ich hatte das Problem früher ja auch, eben mit dem EC Source. Bei mir waren es nur "unbenutzte", bei dir zusätzlich noch "undefinierte".

Was die Geduld angeht: Richtlinien auf der einen Seite, praktikable Lösungen auf der anderen. Beides muss sich nicht ausschliessen. Die Schwierigkeit besteht nur darin, einen gemeinsamen Nenner zu finden. ^^ Das Problem bei den false-positives bei EC sind nicht die false-positives an sich, die könnte man ja ignorieren. Das Problem ist eher, dass man dann die tatsächlichen Fehler zwischen den false-positives herausfischen muss. Da betrachte ich zusätzliche Kommentare in den betreffenden Dateien als deutlich angenehmer.

Klar, ohne EC bzw. VA hätte man das Problem gar nicht erst. Aber wir haben mittels VA schon so manches Artefakt und auch Fehler aufgespürt. Von daher ist VA die Mühe wert aus meiner Sicht.

[oxpus]

@LukeWCS
So, dank Deiner Hilfe habe ich jetzt den gesamten Test auf grün.

Dazu sein anzumerken, dass die Definitionen für CS innerhalb der jeweiligen Klasse angeführt werden müssen, da ansonsten EPV die fehlende Definition von IN_PHPBB anmeckert.
Also als erste Kommentarzeile nach dem Klassennamen und als letzte Kommentarzeile vor dem Ende der Klasse.
Beispiel:

Code: Alles auswählen

class search
{
	// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUndefinedVariableNames sql_array matching_userids
	// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUnusedVariableNames block

und am Ende

Code: Alles auswählen

	// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUndefinedVariableNames
	// phpcs:set VariableAnalysis.CodeAnalysis.VariableAnalysis validUnusedVariableNames
}

Jetzt bin ich zu 100% zufrieden und kann mich nur herzlichst bei Dir bedanken.

[LukeWCS]

So, dank Deiner Hilfe habe ich jetzt den gesamten Test auf grün.

Hab die Berichte vorhin schon gesichtet, jupp, schaut gut aus. Und so fallen echte Fehler auch schneller auf.

Dazu sein anzumerken, dass die Definitionen für CS innerhalb der jeweiligen Klasse angeführt werden müssen, da ansonsten EPV die fehlende Definition von IN_PHPBB anmeckert.

LOL da stopft man ein Loch und reisst damit das nächste auf. Das wäre mir beim testen natürlich nicht aufgefallen, weil ich für den EC Source die Prüfung der Mindestvoraussetzungen sowie EPV deaktiviere per speziellem Eintrag in composer.json.

Also als erste Kommentarzeile nach dem Klassennamen und als letzte Kommentarzeile vor dem Ende der Klasse.

Jupp, wir wissen schon aus Erfahrung, das EPV eine Funktion hat um Kommentare prüfen zu können, wegen Event Definitionen zum Beispiel. Ich vermute also, dass genau diese Funktion für diese Falschmeldung verantwortlich ist.

Hab das jetzt mal eben mit der brandneuen DL Ext 8.2.6 mit der besagten Datei search.php getestet. Das Problem scheint nicht das Setzen, sondern das Löschen der CS Variablen zu sein. Setzen kann man ausserhalb der Klasse, aber löschen muss innerhalb passieren. Verschiebe ich deine beiden CS Kommentare zum setzen über namespace, funktioniert das problemlos. Erst wenn ich die beiden CS Kommentare zum löschen unterhalb der letzten Klammer verschiebe, zickt EPV rum und sieht ein Problem wo keines ist. Okay, dann lieber deine Variante, dass ist dann zumindest konsequent, sprich CS Kommentare innerhalb der Klasse und gut ist.

Jetzt bin ich zu 100% zufrieden und kann mich nur herzlichst bei Dir bedanken.

Danke dir ebenfalls das du es durchgezogen und getestet hast. Das sind auch schon wieder Erfahrungswerte und wir wissen jetzt auch, wie wir die false-positives von VA im Griff behalten können.

[oxpus]

Danke dir ebenfalls das du es durchgezogen und getestet hast. Das sind auch schon wieder Erfahrungswerte und wir wissen jetzt auch, wie wir die false-positives von VA im Griff behalten können.

Keine Ursache.
Jetzt wo ich eine solch tolle Testsuite nutzen kann, macht es deutlich mehr Freude, die Extension weiterzuentwickeln.
Wer weiß, was mir auch dazu noch alles einfällt...