Skip to content
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.

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

PSR-12 is this properly supported yet? #2060

Closed
snake-py opened this issue Sep 8, 2022 · 18 comments · Fixed by #2070
Closed

PSR-12 is this properly supported yet? #2060

snake-py opened this issue Sep 8, 2022 · 18 comments · Fixed by #2070

Comments

@snake-py
Copy link

snake-py commented Sep 8, 2022

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
}
@czosel
Copy link
Collaborator

czosel commented Sep 13, 2022

@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".

@snake-py
Copy link
Author

snake-py commented Sep 13, 2022

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.

@czosel
Copy link
Collaborator

czosel commented Sep 14, 2022

@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/

@snake-py
Copy link
Author

@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
----------------------------------------------------------------------

@cseufert
Copy link
Collaborator

@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.

@cseufert
Copy link
Collaborator

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?

Great idea, I have opened a PR for this already.

@KorvinSzanto
Copy link

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/

@czosel
Copy link
Collaborator

czosel commented Sep 26, 2022

@KorvinSzanto Thanks for the heads-up!
@cseufert Should we call it PER then? 🙈

@cseufert
Copy link
Collaborator

@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

@snake-py
Copy link
Author

snake-py commented Sep 27, 2022

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 php-fig/per-coding-style#49 (comment)

Is there a way to prevent line breaks for certain expressions?

@czosel
Copy link
Collaborator

czosel commented Sep 27, 2022

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?

@cseufert
Copy link
Collaborator

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.

@snake-py
Copy link
Author

snake-py commented Sep 27, 2022

@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"]
}

@snake-py
Copy link
Author

snake-py commented Oct 6, 2022

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?

@czosel
Copy link
Collaborator

czosel commented Oct 6, 2022

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 ?

@snake-py
Copy link
Author

snake-py commented Oct 7, 2022

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.

@snake-py snake-py closed this as completed Oct 7, 2022
@czosel
Copy link
Collaborator

czosel commented Oct 7, 2022

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

@cseufert
Copy link
Collaborator

cseufert commented Oct 9, 2022

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.

tisnamuliarta added a commit to tisnamuliarta/laravel-shadcn that referenced this issue Mar 11, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@prettier/plugin-php](https://togithub.com/prettier/prettier-php) |
[`^0.18.9` ->
`^0.22.0`](https://renovatebot.com/diffs/npm/@prettier%2fplugin-php/0.18.9/0.22.2)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@prettier%2fplugin-php/0.22.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@prettier%2fplugin-php/0.22.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@prettier%2fplugin-php/0.18.9/0.22.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@prettier%2fplugin-php/0.18.9/0.22.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>prettier/prettier-php (@&#8203;prettier/plugin-php)</summary>

###
[`v0.22.2`](https://togithub.com/prettier/plugin-php/releases/tag/v0.22.2)

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.22.1...v0.22.2)

#### What's Changed

- Make chain breaking as similar to original prettier (js) as much as
possible by [@&#8203;eldair](https://togithub.com/eldair) in
[prettier/plugin-php#2320
- fix: 🐛 singleQuote option does not work on format API by
[@&#8203;shufo](https://togithub.com/shufo) in
[prettier/plugin-php#2303
- chore: upgrade dependencies by
[@&#8203;czosel](https://togithub.com/czosel) in
[prettier/plugin-php#2321

#### New Contributors

- [@&#8203;eldair](https://togithub.com/eldair) made their first
contribution in
[prettier/plugin-php#2320

**Full Changelog**:
prettier/plugin-php@v0.22.1...v0.22.2

###
[`v0.22.1`](https://togithub.com/prettier/plugin-php/releases/tag/v0.22.1)

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.22.0...v0.22.1)

- fix the entry
([#&#8203;2290](https://togithub.com/prettier/prettier-php/issues/2290),
thanks [@&#8203;fisker](https://togithub.com/fisker))

###
[`v0.22.0`](https://togithub.com/prettier/plugin-php/releases/tag/v0.22.0)

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.21.0...v0.22.0)

- feat: make format compatible with "PER Coding Style 2.0"
([#&#8203;2269](https://togithub.com/prettier/prettier-php/issues/2269),
thanks [@&#8203;ferchoz](https://togithub.com/ferchoz))
- fix: make `/standalone` entry work with `require()`
([#&#8203;2247](https://togithub.com/prettier/prettier-php/issues/2247),
thanks [@&#8203;shufo](https://togithub.com/shufo))
- chore: upgrade deps
([#&#8203;2289](https://togithub.com/prettier/prettier-php/issues/2289))

###
[`v0.21.0`](https://togithub.com/prettier/plugin-php/releases/tag/v0.21.0):
0.21.0

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.20.1...v0.21.0)

- fix(match): Indent match conditions
([#&#8203;2258](https://togithub.com/prettier/prettier-php/issues/2258),
thanks [@&#8203;claytonrcarter](https://togithub.com/claytonrcarter)!)
- chore!: upgrade dependencies
([#&#8203;2262](https://togithub.com/prettier/prettier-php/issues/2262))

#### Breaking changes

This release drops support for node v14.

###
[`v0.20.1`](https://togithub.com/prettier/plugin-php/releases/tag/v0.20.1)

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.20.0...v0.20.1)

**This package is now pure ESM.**

- vscode documentation update
([#&#8203;2194](https://togithub.com/prettier/prettier-php/issues/2194),
thanks [@&#8203;WazzaJB](https://togithub.com/WazzaJB)!)
- Use `AstPath` getters
([#&#8203;2214](https://togithub.com/prettier/prettier-php/issues/2214),
thanks [@&#8203;fisker](https://togithub.com/fisker)!)
- Migrate to ESM
([#&#8203;2213](https://togithub.com/prettier/prettier-php/issues/2213),
thanks [@&#8203;fisker](https://togithub.com/fisker)!)

###
[`v0.20.0`](https://togithub.com/prettier/plugin-php/releases/tag/v0.20.0)

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.7...v0.20.0)

Many thanks to [@&#8203;fisker](https://togithub.com/fisker) for many
contributions!

- feat: upgrade to prettier 3
([#&#8203;2200](https://togithub.com/prettier/prettier-php/issues/2200))
- Simplify `print()`
([#&#8203;2212](https://togithub.com/prettier/prettier-php/issues/2212))
- Convert tests to ES Modules
([#&#8203;2210](https://togithub.com/prettier/prettier-php/issues/2210))
- Fix standalone test
([#&#8203;2211](https://togithub.com/prettier/prettier-php/issues/2211))
- Update .gitattributes
([#&#8203;2209](https://togithub.com/prettier/prettier-php/issues/2209))

### Breaking Change

This adds support for Prettier v3. It also drops compatibility with
Prettier v1 and v2.

###
[`v0.19.7`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.7):
0.19.7

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.6...v0.19.7)

- fix(match): Support empty lines and comments in match expressions
([#&#8203;1956](https://togithub.com/prettier/prettier-php/issues/1956),
[#&#8203;2201](https://togithub.com/prettier/prettier-php/issues/2201) -
thanks [@&#8203;claytonrcarter](https://togithub.com/claytonrcarter)!)

###
[`v0.19.6`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.6):
0.19.6

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.5...v0.19.6)

- Add support for final class const
([#&#8203;2186](https://togithub.com/prettier/prettier-php/issues/2186),
thanks [@&#8203;cseufert](https://togithub.com/cseufert)!)
- chore: upgrade dependencies
([#&#8203;2193](https://togithub.com/prettier/prettier-php/issues/2193))

###
[`v0.19.5`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.5):
0.19.5

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.4...v0.19.5)

- fix: add option for PHP 8.2
([#&#8203;2179](https://togithub.com/prettier/prettier-php/issues/2179))
*Note*: This allows setting `phpVersion` to `8.2` - Support for PHP 8.2
is still incomplete, though!

###
[`v0.19.4`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.4):
0.19.4

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.3...v0.19.4)

This release includes several internal cleanups and updates, thanks to
contributions by [@&#8203;fisker](https://togithub.com/fisker)!

- improve doc print and update prettier dependency
([#&#8203;1922](https://togithub.com/prettier/prettier-php/issues/1922))
- use `clean.ignoredProperties`
([#&#8203;2158](https://togithub.com/prettier/prettier-php/issues/2158))
- Remove handling `\r`
([#&#8203;2157](https://togithub.com/prettier/prettier-php/issues/2157))
-   dependency updates

###
[`v0.19.3`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.3):
0.19.3

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.2...v0.19.3)

- feat: add support for readonly classes \[PHP 8.2]
([#&#8203;2131](https://togithub.com/prettier/prettier-php/issues/2131),
thanks [@&#8203;genintho](https://togithub.com/genintho)!)
- chore: upgrade dependencies
([#&#8203;2134](https://togithub.com/prettier/prettier-php/issues/2134))

###
[`v0.19.2`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.2):
0.19.2

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.1...v0.19.2)

#### Deprecations

The `psr2` setting for the `braceStyle` option has been renamed to
`per-cs` to be more in line with future PER releases (see
[prettier/plugin-php#2060
for details). `psr2` will continue to work for now, but will be removed
in a future release.

#### Bugfixes

- Support expressions in new statement
([#&#8203;2086](https://togithub.com/prettier/prettier-php/issues/2086),
thanks [@&#8203;cseufert](https://togithub.com/cseufert)!)
- Fix for function curly brace with attrs
([#&#8203;2089](https://togithub.com/prettier/prettier-php/issues/2089),
thanks [@&#8203;cseufert](https://togithub.com/cseufert)!)
- Deprecated PSR-2 braceStyle and use PER-CS instead
([#&#8203;2070](https://togithub.com/prettier/prettier-php/issues/2070),
thanks [@&#8203;cseufert](https://togithub.com/cseufert)!)

###
[`v0.19.1`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.1):
0.19.1

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.19.0...v0.19.1)

- fix for inline closure
([#&#8203;2062](https://togithub.com/prettier/prettier-php/issues/2062),
thanks [@&#8203;cseufert](https://togithub.com/cseufert)!)
- chore: upgrade dependencies
([#&#8203;2068](https://togithub.com/prettier/prettier-php/issues/2068))

###
[`v0.19.0`](https://togithub.com/prettier/plugin-php/releases/tag/v0.19.0):
0.19.0

[Compare
Source](https://togithub.com/prettier/prettier-php/compare/v0.18.9...v0.19.0)

- fix: formatting of long cases in match statement
([#&#8203;2054](https://togithub.com/prettier/prettier-php/issues/2054))
- chore: upgrade dependencies
([#&#8203;2059](https://togithub.com/prettier/prettier-php/issues/2059))

Breaking Change:

-   Dropped support for Node v12.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/tisnamuliarta/laravel-shadcn).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIzMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants