feat: rewrite CS rules

Zestaw zasad oparty o WPCS oraz wspólnie ustalone reguły, silnie inspirowany m.in. YoastCS.

Plik .editorconfig jest kopią pliku przygowowanego przez Mateusza.

W tym MR przedstawiam skonfigurowany plik ruleset.xml, który zawiera większość potrzebnych elementów, ale kilka trzeba będzie jeszcze dopisać jako niestandardowe sniffy (np. komentarze dla hooków czy suffix Trait.php dla traitów), ale to planowałem dodać trochę później, już jako 2.x

Podbiłem wersję do 2.0, ale jeśli uważacie, że 1.2 będzie lepsze, to zmienię.

assigned to @grola, @dyszczo, @sebastian.pisula, @mateuszgbiorczyk, @potreb, and @marcinkolanko

By Bartek Jaskulski on 2021-06-04T11:36:10 (imported from GitLab)

Dobra robota @bartjaskulski ;) Mam kilka swoich propozycji:

1. Zamiast wykluczać katalogi proponuję wybrać te, które będziemy obsługiwać i ustawić to w głównym pliku wtyczki:

<file>./src</file>
<file>./templates</file>

2. Jest problem z kompatybilnością z wersją PHP - gdy ustawię w głównym pliku np. PHP 7.4 to i tak testuje dla 7.0. Wyrzuciłbym regułę z paczki reguł i dał tylko w głównym pliku.
3. Dorzuciłbym wykluczenie dla reguł wymagających używania funkcji WordPressowych zamiast tych z PHP:

<exclude name="WordPress.WP.AlternativeFunctions" />

4. Dodatkowo wymagane jest używanie funkcji gmdate zamiast date. Często jednak lepiej użyć date, jeżeli nie chcemy zmienić strefy czasowej. Proponuję to wywalić:

<rule ref="WordPress-Extra">
	<exclude name="WordPress.DateTime.RestrictedFunctions.date_date" />
</rule>

5. Poniższy zapis powoduje u mnie następujący błąd "ERROR: The specified cache file path "./.cache/phpcs.cache" points to a non-existent directory":

<arg name="cache" value="./.cache/phpcs.cache"/>

6. Czy możemy zrobić tak, że jeśli już mamy komentarz dla metody to wymagamy, aby on kompletny? Np. taki zapis nie zwraca mi błędu, że brakuje opisu dla parametru:

/**
 * Returns data to be saved into database as array of records.
 *
 * @param mixed[] $resource_data
 *
 * @return mixed[] Data for SQL insert.
 */

7. Regułę Squiz.Commenting.FileComment.Missing dałbym tylko dla katalogu /src. Dla katalogu np. z templatkami niech komentarz pliku będzie obowiązkowy.
8. Nie wczytują się u mnie zestawy reguł PHPCompatibility oraz WordPress. Aby to rozwiązać, w głównym pliku reguł potrzebna jest taka linijka:

<config name="installed_paths" value="vendor/phpcompatibility/php-compatibility,vendor/wp-coding-standards/wpcs" />

By Mateusz Gbiorczyk on 2021-06-08T07:43:53 (imported from GitLab)

1. Ze względu na to, że ta wartość może zostać nadpisana przez argumenty wejściowe, przeniosłem to do phpcs.xml, a wszystkie zasady wykluczenia zostawił. W ten sposób jeśli puszczamy przelot bez argumentów, to faktycznie mamy ograniczenie do interesujących nas folderów, ale jeśli z jakichś przyczyn zdefiniujesz, że chcesz sprawdzić określony folder, to na wszelki wypadek CS wyklucza niepotrzebne foldery.
2. Zrobiłem dwie roszady: po pierwsze zmieniłem PHPCompatibility na PHPCompatibilityWP. Zostawiłem deklarację w WPDesk, ale definicję wartości zostawiam już po stronie projektu, więc jak nadpiszesz PHP do 7.4 powinno zadziałać.
3. Jeśli jest zgoda wszystkich, to dodaję wykluczenie.
4. Jak wyżej. Ewentualnie - czy jest to na tyle często, że dodawanie phpcs:ignore byłoby uciążliwe? A może lepiej zdefiniować to już per projekt w phpcs.xml?
5. Myślę, że cachowanie wyników ogólnie byłoby dobrym rozwiązaniem. Jeśli się zgadzacie to rozwiązania są dwa: albo przeniosę plik cache do folderu głównego wtyczki, albo po prostu trzeba będzie utworzyć folder .cache. W obu przypadkach trzeba będzie dodać odpowiedni element do .gitignore. Jeśli uważacie, że to jednak nie ma sensu, to usuwam argument z phpcs. Edit: Tymczasowo przeniosłem do pliku .phpcs-cache nie będzie generować błędu, bo będzie w folderze głównym.
6. Wydaje mi się, że tutaj dogadaliśmy się, że nawet jak już masz opisany komentarz, to nie wymagamy do niego krótkiego opisu. To dlatego, że czasem parametrem może być np. $email, do którego już możesz nie potrzebować dodatkowego wyjaśnienia.
7. Trochę to poprawiłem, ale właśnie tak miało być, z tym, że z wykorzystaniem reguły <include-pattern>, który wskazuje, że ma sprawdzać jedynie pliki z folderu templates. Niestety, jest jeszcze jeden problem: PhpStorm nie najlepiej radzi sobie z tą regułą. O ile w konsoli śmiga to poprawnie, to Storm nie wskazuje na brak FileDoc w plikach template (ale i tak wywali się, kiedy będziesz próbował zrobić commit)
8. Już poprawiam. Nie ogarnąłem, bo u siebie instaluję reguły globalnie. Edit: Jeszcze trochę zmieniłem podejście. Kiedy pakiet jest instalowany odpala się funkcja, której celem jest zainstalowanie standardów dla phpcs. W ten sposób wszystko powinno śmigać już bez deklaracji ścieżek.

By Bartek Jaskulski on 2021-06-09T14:25:05 (imported from GitLab)

added 1 commit

• 5fe78632 - fix: refine issues with running cs

Compare with previous version

By Bartek Jaskulski on 2021-06-09T14:21:30 (imported from GitLab)

3. Jeśli jest zgoda wszystkich, to dodaję wykluczenie.

Ja też bym chyba to odpuścił. Rozwiązania które proponuje WP są mocno dyskusyjne.

4. Często jednak lepiej użyć date, jeżeli nie chcemy zmienić strefy czasowej.

Niektóre wtyczki zmieniają domyślą strefę czasową. Niech diabli wezmą ich autorów, ale zdarza się, że takie rzeczy dzieją się nawet gdzieś w hookach normalnie w runtime. Wtedy w jednym miejscu date jest w jednej strefie, a w drugim miejscu w drugiej i spróbuj to zdebugować u klienta wtedy. Dlatego ja po kilku fuckupach w ShopMagic jestem za tym, żeby date nie używać i strefę podawać jawnie.

6. Wydaje mi się, że tutaj dogadaliśmy się, że nawet jak już masz opisany komentarz, to nie wymagamy do niego krótkiego opisu. To dlatego, że czasem parametrem może być np. $email, do którego już możesz nie potrzebować dodatkowego wyjaśnienia.

To prawda. Pomedytowałem trochę i myślę, że najlepiej zrobić tak, że jeśli już jest PHPDoc to najlepiej żeby był kompletny czyli zawierał opisy pól. W sumie dopisanie opisu do $email aż tak nie zaboli, a rwane opisy wydaje mi się, że będą bardzo niespójne do czytania.

By Dyszczo on 2021-06-10T23:16:40 (imported from GitLab)

@dyszczo:

Ze wszystkim się zgodzę, tylko co do ostatniego punktu zaoponuję. To czy zaboli wstawienie opisu do np. $customer_email jest mniej istotne niż to, czy ktokolwiek ten opis wstawi, kiedy łatwiej i szybciej jest zostawić .

Na 100% dojdzie do takich sytuacji, więc moim zdaniem lepiej zostawić tutaj zupełną dowolność i nie wymagać opisu parametru, bo może będzie niespójne, ale przynajmniej nie zagracone znakami przestankowymi. A skoro nie wyciągamy dokumentacji z PHPDoca to ta niespójność raczej nie powinna aż tak bardzo boleć.

By Bartek Jaskulski on 2021-06-11T15:17:11 (imported from GitLab)

added 1 commit

• 71319150 - docs: change version back to 1.2

Compare with previous version

By Bartek Jaskulski on 2021-06-11T15:25:03 (imported from GitLab)

added 1 commit

• 07c05eda - fix: update included rule in example file

Compare with previous version

By Bartek Jaskulski on 2021-06-11T15:30:52 (imported from GitLab)

Ze wszystkim się zgodzę, tylko co do ostatniego punktu zaoponuję.

Przekonałeś mnie :)

By Dyszczo on 2021-06-12T14:46:37 (imported from GitLab)

mentioned in commit aa28f042

By Bartek Jaskulski on 2021-06-14T08:29:53 (imported from GitLab)

merged