Skip to content

chore: phpcs using config in github actions, restore: phpcs.xml.dist #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 20, 2025
+63 −1
Merged

chore: phpcs using config in github actions, restore: phpcs.xml.dist #26

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b8a3c0
chore: add php-cs-fixer for github workflows (pipelines)
Nov 19, 2025
c243d50
restore: phpcs.xml.dist
RobinTheHood Nov 14, 2025
27ec84d
Revert "chore: add php-cs-fixer for github workflows (pipelines)"
Nov 19, 2025
2b1dfeb
chore: explicit usage configuration file phpcs.xml.dist
Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/php-code-style.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: '8.4' # PHP Version
tools: phpcs # PHP CodeSniffer installieren
tools: php-cs-fixer # PHP-CS-Fixer installieren

# Schritt 3: Code-Style prüfen mit PSR-12 Standard
# phpcs = PHP CodeSniffer prüft den Code auf Style-Fehler
Expand All @@ -30,4 +30,4 @@ jobs:
# admin/*.php = Prüft alle PHP-Dateien im admin-Ordner
# includes/*.php = Prüft alle PHP-Dateien im includes-Ordner
- name: Check PHP Code Style (PSR-12)
run: phpcs --standard=PSR12 *.php admin/*.php includes/*.php
run: php-cs-fixer fix --config=.php-cs-fixer.dist.php --show-progress=none --diff --dry-run
Copy link
Contributor

@RobinTheHood RobinTheHood Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich denke, dass das so noch keine gute Idee ist. Wenn ich das zum Test ausführe erhalte ich folgende Meldung.

PHP CS FIXER

Running analysis on 1 core sequentially.
You can enable parallel runner and speed up the analysis! Please see usage docs for more information.
Loaded config default from ".php-cs-fixer.dist.php".
Using cache file ".php-cs-fixer.cache".
   1) admin/send_email.php
      ---------- begin diff ----------
--- /Users/robin/test/admin/send_email.php
+++ /Users/robin/test/admin/send_email.php
@@ -8,7 +8,7 @@
 // MailSender functions
 require_once __DIR__ . '/../includes/send_email_functions.php';
 
-use App\Service\MailSender; 
+use App\Service\MailSender;
 
 // E-Mail-Konfiguration
 $to = '[email protected]';

      ----------- end diff -----------


Found 1 of 11 files that can be fixed in 0.010 seconds, 16.00 MB memory used

PHPCS

FILE: /Users/robin/test/admin/send_email.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 11 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
  1. Wird bei php cs fixer nicht gesagt, gegen welche Regel hier verstoßen wird und ...
  2. Fehler die nicht automatisch gefixt werden können, werden nicht angezeigt.

Meine aktuelle Meinung ist: php cs fixer ist eine gute Idee und funktioniert auch aber für eine reine Prüfung im Pull Request bietet phpcs den klareren Vorteil dass es exakt benennt welche Regel verletzt wurde und in welcher Zeile. php cs fixer zeigt im dry run zwar die Änderungen die nötig wären doch es liefert nur den Patch ohne die konkrete Regel dahinter. Hier ist z.B. nicht sichtbar, dass das Problem das Leerzeichen am Ende war. Für Reviewer ist das weniger eindeutig weil man zwar sieht dass etwas geändert werden müsste jedoch nicht warum. phpcs liefert dagegen eine saubere Meldung mit Sniff Name Dateiangabe und Zeile wodurch Verstöße sofort nachvollziehbar werden und sich schneller korrigieren lassen. Genau diese klare Zuordnung macht phpcs als reines Prüfwerkzeug in der CI aus meiner Sicht besser.

Ich muss aber auch sagen, dass ich nicht weiß, ob sich das mit php cs fixer nachbilden lässt.

Copy link
Contributor

@Tomcraft1980 Tomcraft1980 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bekommen wir einen lauffähigen Zustand wieder her? 😉
Ich bin da gerade ein wenig überfordert und auf eure Expertise angewiesen.

Copy link
Contributor

@RobinTheHood RobinTheHood Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tomcraft1980 Werden wir bekommen. Ich würde gerne noch abwarten was @dgodglueck sagt. Aktuell finde ich "meine" Variante noch besser, aber ich habe die Weisheit nicht gepachtet. Vielleicht hat er noch einen guten Einwand.

Copy link
Contributor

@Tomcraft1980 Tomcraft1980 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich danke euch beiden sehr! 👍

Copy link
Author

@dgodglueck dgodglueck Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mit dem jetzigen PR wird alles wieder passen 👍
Ich finde tatsächlich die Lösung mit dem phpcs Tool auch übersichtlicher, und macht am Ende das gleiche, wieder was neues gesehen und gelernt

Copy link
Contributor

@RobinTheHood RobinTheHood Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

phpcs --standard=PSR12 *.php admin/*.php includes/*.php

Hatten wir mal zur Demo, aber es bezieht noch nicht die phpcs.xml.dist im Root Verzeichnis mit ein. Da die phpcs.xml.dist erst später hinzugekommen ist als "Beispiel".

Man könnte es also so anpassen, dass man phpcs statt der Option --standard=PSR12 die Datei phpcs.xml.dist mitgibt. Ich glaube man kann --standard=PSR12 als Option einfach weglasse und dann nimmt er automatisch die phpcs.xml.dist aus dem Root. Aber für die CLI wäre es vielleicht leichter oder besser verständlich / leserlicher, wenn man direkt sagt, er soll die phpcs.xml.dist nehmen.

Copy link
Contributor

@RobinTheHood RobinTheHood Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hinweis: Es geht phpcs.xml.dist als auch .phpcs.xml.dist (mit Punkt) wir haben glaube ich OHNE Punkt.

Copy link
Author

@dgodglueck dgodglueck Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobinTheHood soweit ich verstanden habe, werden die Tools im Rootverzeichnis ausgeführt, wo auch die phpcs.xml.dist liegt, aber ich hatte mit dem cs-fixer identisch Problem. Also wenn ich die Konfiguration nicht explizit angebe, dass das Tool (beliebiges) sagt es benötigt eine.

Copy link
Contributor

@RobinTheHood RobinTheHood Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgodglueck Ich glaube du schreibst aktuell den Dateinahmen mit Punkt --standard=.phpcs.xml.dist müsste wohl aber --standard=phpcs.xml.dist ohne Punkt sein.

Copy link
Author

@dgodglueck dgodglueck Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups 🤦🏼‍♂️, korrigiert

62 changes: 62 additions & 0 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?xml version="1.0"?>
<ruleset name="Project PSR-12 Style Only">
<description>
PSR-12 als Basis, aber alle „logiknahen“ Regeln (Namespaces, Side Effects,
Strict Types, Typ-Decls etc.) sind deaktiviert. Es bleiben vor allem
Whitespace- und Formatierungsregeln übrig.
</description>

<!-- Was nie geprüft werden soll -->
<exclude-pattern>includes/external/*</exclude-pattern>

<!-- Basis: kompletter PSR-12 Standard -->
<rule ref="PSR12"/>

<!-- =========
LOGIK-/STRUKTUR-REGELN DEAKTIVIEREN
========= -->

<!-- Side Effects & "1 Klasse pro Datei + Namespace" -->
<rule ref="PSR1.Files.SideEffects">
<severity>0</severity>
</rule>

<rule ref="PSR1.Classes.ClassDeclaration">
<severity>0</severity>
</rule>

<!-- declare(strict_types=1) & Co. -->
<rule ref="PSR12.Files.DeclareStatement">
<severity>0</severity>
</rule>

<!-- Typed Parameter & Return Types -->
<rule ref="PSR12.Functions.NullableTypeDeclaration">
<severity>0</severity>
</rule>

<rule ref="PSR12.Functions.ReturnTypeDeclaration">
<severity>0</severity>
</rule>

<!-- Kurzformen von Typkeywords (bool/int/...) -->
<rule ref="PSR12.Keywords.ShortFormTypeKeywords">
<severity>0</severity>
</rule>

<!-- Namespace-Struktur -->
<rule ref="PSR12.Namespaces.CompoundNamespaceDepth">
<severity>0</severity>
</rule>

<!-- Sichtbarkeit von Konstanten erzwingen (kann Backward Compatibility sein) -->
<rule ref="PSR12.Properties.ConstantVisibility">
<severity>0</severity>
</rule>

<!-- Optional: Wenn du noch mehr "Logik-Nähe" rauswerfen willst,
kannst du hier weitere Sniffs nach Bedarf auf severity=0 setzen. -->

<!-- Nur Infos: Warnungen kannst du komplett ausblenden, wenn du magst -->
<!-- <arg name="warning-severity" value="0"/> -->
</ruleset>