php-cs-fixer issue

[badams]

Running php-cs-fixer fix . --level=psr2 generates a massive list of changes, and breaks unit tests (it changes the namespaces for tests from Yasumi\Tests\NewZealand to Yasumi\tests\NewZealand)

Are there some specific settings to use when running php-cs-fixer?

[stelgenhof]

Oh really? I have never seen this on my own machine (using PHPStorm with php-cs-fixer integration). I just tried on a separate environment and noticed the same. Let me investigate what the difference is.

[badams]

The difference isn't too bad, if you just run php-cs-fixer fix src/ --level=psr2 it's mainly spacing/new line fixes along with a few function definitions that don't specify visibility.

if you run it on the tests directory also (as I did originally) it will update the namespaces on all the tests.. I think all you need to fix that issue is update following in composer.json (I've not tested this)

"autoload-dev": {
    "psr-4": {"Yasumi\\Tests\\": "tests/"}
  }

[stelgenhof]

Yeah, I noticed that mostly it is comments and formatting. Oops! Not following PSR-2 as I stated :)
Let me check the "tests" change in composer.json

I am only wondering why it changes "Test" to "test". Couldn't find anything about that in the PSR-2 specs...

[badams]

Yes, and oddly enough it doesn't do the same for other namespaces eg. Yasumi\Provider\NewZealand

[stelgenhof]

I changed composer.json to

"autoload-dev": {
    "psr-4": {"Yasumi\\tests\\": "tests/"}
  }

and HolidaysFilterTest.php also (reference to trait use Yasumi\tests\YasumiBase), and now phpunit runs without error.

Hmm... still wondering why this directory name was changed.

[badams]

I guess if it doesn't hurt BC its not a bad thing to be more compliant - I trust php-cs-fixer knows what it's doing :)

[stelgenhof]

Actually I did some tests, and I get the feeling it applies fixes that are not only PSR2 :(. For example it removes the '@Package....' line from the comments and I checked that is part of the 'Symfony' level.

Looks like the option --level=psr2 doesn't mean exclusively 'PSR2'.

For good measure I checked another package (Dingo API) and their composer.json contents for the unit tests is the same as mine.

I want to be PSR2 compliant, but then php-cs-fixer should not do anything beyond that.

[badams]

Ah, I see.. that makes sense.

Just looking at the docs for php-cs-fixer you can manually specify which fixers you want to include/exclude, using fixers flag eg php-cs-fixer fix . --level=psr2 --fixers="-symfony" but that still gets rid of @Package notations

Even better you can define a .php_cs config file in the root of the project, which is what Dingo API does

I'm just running it with that config now to see how it turns out.

[stelgenhof]

Ah yes. Then maybe it's better to create such a custom config file everybody can use. This will avoid any confusion as well :) Thanks for checking!

[badams]

Looks good so far, I've just had to modify the config from Dingo slightly. Do you want me to create a PR?

[stelgenhof]

Sure! Let me check it out later

[stelgenhof]

Issue resolved with PR #16