plugin-php
plugin-php copied to clipboard
PSR-12 is this properly supported yet?
Hello, I am trying to use this with psr-12 standard, but I think I am doing something wrong. I want to use this plugin before PHP_CodeSniffer, which uses the PSR-12 standard. I found several threads already speaking about it, but the I did not find anything in the documentation.
Can you help me to set this up?
# my script
#! /usr/bin/env bash
run_prettier(){
files=$1
if [ "$files" != "" ]
then
echo "Running Prettier... on $files"
prettier --write $files
if [ $? != 0 ]
then
echo "Fix the error before commit."
exit 1
else
# Add back the modified/prettier files to staging
echo "$files" | xargs git add
fi
fi
}
# output
Running Prettier... on src/LoggingServicePackageProvider.php
[error] Invalid braceStyle value. Expected "1tbs" or "psr-2", but received "psr-12".
Fix the error before commit.
.prettierrc:
{
"phpVersion": "8.1",
"printWidth": 80,
"tabWidth": 4,
"useTabs": false,
"singleQuote": false,
"trailingCommaPHP": true,
"braceStyle": "psr-12",
"requirePragma": false,
"insertPragma": false
}
@snake-py thanks for bringing this up! In my mind PSR-12 was just an extension on top of PSR-2, but it turns out that PSR-2 has been replaced by PSR-12:
As of 2019-08-10 PSR-2 has been marked as deprecated. PSR-12 is now recommended as an alternative.
https://www.php-fig.org/psr/psr-2/
I think we should rename the braceStyle
option to from psr-2
to psr-12
, since 12
defines exactly the same brace style convention (as far as I can tell). To avoid the breaking change, we can keep the psr-2
as a synonym, but remove it from the docs. What do you think @cseufert?
@snake-py For your case, you'll probably get what you expect by specifying "braceStyle": "psr-2"
.
Well the issue I am facing is if I run prettier on my files and afterward I run PHP_CodeSniffer the changes prettier made will collide with the PSR-12 standard. This makes the package for me unusable. I thought if I used the same standard everything should be formatted the same.
I can see if I can make an example where I run into issues, but my lead is already annoyed because I spend a lot of time on this :D
@czosel I am unsure if this will be out of scope, but is there a way how to format the code? Can I define the amount of character in one line (when to break etc. ?) In all honesty, this is the first time that I use prettier from the command line :D and even have to set it up.
@snake-py Thanks for clarifying, I misunderstood your original post. Could you share some examples how the output of our plugin is in conflict with PSR-12?
I'm not using PHP_CodeSniffer myself, but if it's comparable to eslint this might call for a package like https://github.com/prettier/eslint-config-prettier which disables all the linting rules that are in conflict with Prettier?
Concerning line width: The Prettier core option --print-width
should have you covered. You can play with the effect in our playground: https://loilo.github.io/prettier-php-playground/
@czosel for instance:
foreach ($client->getContractsByCustomerId($customer["id"]) as $contract) {
prettier will do this:
foreach (
$client->getContractsByCustomerId($customer["id"])
as $contract
) {
I will now get the following error from code sniffer with PSR12
FILE: /app/app/Services/AuthService.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
388 | ERROR | [x] Expected 1 space before "as"; 20 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
@snake-py I would see this as more of a bug on the PHPCBF ruleset, as it is not really violating any specific PSR-12 rule that I can see, however it does not appear to know how to deal with a foreach expression split over multiple lines.
I think we should rename the
braceStyle
option to frompsr-2
topsr-12
, since12
defines exactly the same brace style convention (as far as I can tell). To avoid the breaking change, we can keep thepsr-2
as a synonym, but remove it from the docs. What do you think @cseufert?
Great idea, I have opened a PR for this already.
PSR-12 editor here, just want to give you a heads up that PSR-12 is now stale and the Coding Style PER v1.0.0 should be used in its place moving forward. The value of relying on the CS-PER instead of PSR-12 is the CS-PER will be releasing new versions that add support for new language features as they are added and PSR-12 will likely be deprecated soon once we get out the next version of CS-PER. You can see the latest version of CS-PER (Which is just PSR-12 with names changed) here: https://www.php-fig.org/per/coding-style/
@KorvinSzanto Thanks for the heads-up! @cseufert Should we call it PER then? 🙈
@czosel Yes, I think aliasing PSR-2
, to PER
or PER-CS
would make sense, I'm leaning more to PER-CS
as I assume there will be additional PER's in the future
Hey, I opened a clarification ticket on the PSR12 guide about this and they told me that it is according to their standard "not okay" to break foreach up into multiple lines https://github.com/php-fig/per-coding-style/issues/49#issuecomment-1258509807
Is there a way to prevent line breaks for certain expressions?
Yes, in principle we can control the line breaks. But in this case I'm not sure if it's the correct way to go. We have always used PSR / PER as guidance when making formatting decisions, but we never aimed to be fully PSR / PER compliant. Probably there are many more examples for this. To me, the idea behind this plugin is to stay reasonably close to how Prettier for JS works. What do you think @cseufert?
I feel that trying to control the behaviour in these edge cases goes against the spirit of prettier itself My reasons against:
- The PER-CS group suggest refactoring the loop to use a temp variable, which sounds reasonable.
- Prettier tries hard not to exceed the max line length, which I feel is one of the most important aspects, to keep code readable, especially diff/patches.
@czosel I mean no offense, but this will make the plugin effectively useless to me, and probably other people as well.
@cseufert
The PER-CS group suggest refactoring the loop to use a temp variable, which sounds reasonable.
sure this will solve this particular issue, however it will not solve the overall issue..
Prettier tries hard not to exceed the max line length, which I feel is one of the most important aspects, to keep code readable, especially diff/patches.
I am unsure how complex this would be and I also would be willing to code it myself, with some guidance. But can we not have a simple setting that accepts an array, which will disable line breaks for this particular line?
Something like:
{
"disableLineBreaksOnKeywords": ["foreach"]
}
Any further thoughts regarding this issue or do you just want to stick to the statement that this package is not written with the purpose to fulfill any coding standard?
I agree with @cseufert‘s statement about line length. Also we’re very conservative about adding new options as explained in Prettiers Option Philosophy.
Personally, very specific style guides have become irrelevant to me since I started using prettier. Prettier aims to format code in a way that is readable, as if it had been manually formatted by the developer, but in an automatic and reproducible way. To me, style guides are useful to define how Code in a specific project should be formatted, if no automatic formatter is available. With prettier this is implicitly defined, making a sort of „automatic style guide“. The fact that it doesn’t match any other manual style guide is not important anymore.
What’s the reason you feel that you have to be 100% PER-compliant @snake-py ?
What’s the reason you feel that you have to be 100% PER-compliant
Well, because our code quality check in our pipeline uses PSR-12 standards. The code quality check in our pipelines is done by code climate and it checks more than just formatting like it checks how big functions are and if there are strings that should be used as constants etc. But it also checks if everything is formatted to PSR-12. I am not sure if I can disable the formatting check.
But I understand your argument and I will see how I want to solve this. Either I will drop prettier and try PHPCBF again or I will try to disable the formatting checks inside the pipeline.
Right - I think in the long run we'll need a package similar to prettier-config-eslint that disables all the formatting rules that conflict with Prettier. See also: https://prettier.io/docs/en/integrating-with-linters.html
But I understand your argument and I will see how I want to solve this. Either I will drop prettier and try PHPCBF again or I will try to disable the formatting checks inside the pipeline.
If you do figure out what checks conflict with prettier and how to disable them, please consider documenting these for other prettier users.