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 / PER-CS v1 Support #4502

Open
4 of 10 tasks
julienfalque opened this issue Aug 12, 2019 · 68 comments
Open
4 of 10 tasks

PSR-12 / PER-CS v1 Support #4502

julienfalque opened this issue Aug 12, 2019 · 68 comments
Labels
kind/feature request kind/meta Set of actions to be broken down into single issues/PR's topic/core Core features of Fixer's engine topic/PSR-12

Comments

@julienfalque
Copy link
Member

julienfalque commented Aug 12, 2019

PSR-12 just got approved. PER-CS v1 is the same in terms of rules, so all the changes must be included in both of them (@PSR12, @PER-CS1.0).

I think PHP CS Fixer already supports almost all of its rules. The missing cases I could notice are the following:


3. Declare Statements, Namespace, and Import Statements

The header of a PHP file may consist of a number of different blocks. If present, each of the blocks below MUST be separated by a single blank line, and MUST NOT contain a blank line. Each block MUST be in the order listed below, although blocks that are not relevant may be omitted.

  • Opening <?php tag.
  • File-level docblock.
  • One or more declare statements.
  • The namespace declaration of the file.
  • One or more class-based use import statements.
  • One or more function-based use import statements.
  • One or more constant-based use import statements.
  • The remainder of the code in the file.

ordered_imports currently sorts imports without considering their type. We should add an option to sort by type first. We should also add an option (or dedicated rule) to add a separation line between each import group.


[In control structures, ]expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

We need to create a fixer that could force boolean operators to always be at the same position with two options like e.g.:

  • position (beginning or end): choose whether the operators should be at the beginning or end of the lines;
  • when (always or mixed): whether the rule should always be applied or only when a mix of positions is found.

We also need to support the multiple lines part of the rule:

  • each subsequent line is indented at least once;
  • the first condition must be on the next line.

Anonymous Classes MUST follow the same guidelines and principles as closures in the above section.

You need to update class_definition to support adding a spacing before the parentheses in new class().

The opening brace [of an anonymous class] MAY be on the same line as the class keyword so long as the list of implements interfaces does not wrap. If the list of interfaces wraps, the brace MUST be placed on the line immediately following the last interface.

I think this is currently covered by the rule braces. Not sure if we should add a new option to it or create a new fixer.


TODO

  • support for ordering imports by type first
  • support for adding separation lines between import group
  • support for removing blank lines between imports of the same group
  • support for boolean operator position in control structure expression
  • support for indenting multiline control structure expressions
  • support for forcing first condition of multiline control structure expressions to be on next line
  • support for anonymous class opening brace in case implements list wraps
  • support for forcing blank line between import statement types ([OrderedImportsFixer] Ability to add blank line between groups #3582)
  • support for adding a space before parentheses in new class() (ClassDefinitionFixer - PSR12 for anonymous class #5877)
  • @PSR12 ruleset (Add PSR12 ruleset #4943)
@julienfalque julienfalque added kind/feature request kind/meta Set of actions to be broken down into single issues/PR's labels Aug 12, 2019
@kbond
Copy link

kbond commented Aug 12, 2019

What about this - specifically the part about spaces between class/function/const imports. It this possible currently?

@dmvdbrugge
Copy link
Contributor

dmvdbrugge commented Aug 13, 2019

What about this - specifically the part about spaces between class/function/const imports. It this possible currently?

No, the newlines between different use types is not yet possible, however there is a longstanding issue (without PR) addressing it: #3582

edit: I would like to see that being resolved as well as part of this meta-issue

@dmvdbrugge
Copy link
Contributor

dmvdbrugge commented Aug 13, 2019

When doing so, the first condition MUST be on the next line.

Is this part already covered somewhere?

As to your boolean operator fixer: I'd like the fixer to be able to do both, depending on config (always_front, always_back, when_mixed_front, when_mixed_back). It was not clear from your suggestion if you meant this, or just let it do only one of the bullet-points.

@julienfalque
Copy link
Member Author

@kbond Indeed, I updated my post with PR ref given by @dmvdbrugge.

@dmvdbrugge Actually even the "each subsequent line is indented at least once" isn't supported currently. I updated my post to add the rules. As they both deal with whitespace, maybe we could implement them in a single fixer?

I also updated my suggestion for the boolean operator position fixer, hope it's better now :) Thanks!

@jdufresne
Copy link

From #4568, perhaps PSR-12 should be the default configuration for v3.

@ernst77
Copy link

ernst77 commented Dec 10, 2019

Any update on psr-12 support?

@julienfalque
Copy link
Member Author

Nope, nobody gave it a try yet.

@localheinz
Copy link
Member

Perhaps it makes sense to create a branch that aggregates a @PSR12 rule set, so we can see where we are and what's missing?

@julienfalque
Copy link
Member Author

We can create a @PSR12 ruleset indeed, even if it doesn't contain any specific rule yet. Though tracking progress regarding PSR-12 support is the exact purpose of the original post of this issue :)

@Wirone
Copy link
Member

Wirone commented Mar 28, 2023

@a-menshchikov there's #6861 created recently after discussion #6849.

@ostrolucky
Copy link
Contributor

ostrolucky commented Apr 15, 2023

PSR-PER standard has been published recently (2 weeks ago). It is an evolving standard which builds onto PSR-12.

Ref:
https://github.com/php-fig/per-coding-style/blob/master/spec.md
https://github.com/php-fig/per-coding-style/releases/tag/2.0.0

@Wirone Wirone unpinned this issue Jun 14, 2023
@github-actions
Copy link

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

I will close it if no further activity occurs within the next 30 days.

@mvorisek
Copy link
Contributor

mvorisek commented Jul 19, 2023

@Wirone what about splitting this issue into the separate ones? I want to comment on 5.x, but the discussion here on multitopic is quite too long to be helpful.

@Wirone
Copy link
Member

Wirone commented Jul 19, 2023

@mvorisek I agree, that's why I unpinned this issue some time ago. First of all I believe we should focus on PER-CS now, since #6707 was merged it's now possible to work on v2. I am not sure if investing time in PSR-12 is good at this point.

Anyway, if we want to continue working on this, we should create milestone with dedicated issues for each remaining task from this issue's checklist. @keradus what do you think about this idea? I could split this issue in the meantime.

@Jean85
Copy link
Contributor

Jean85 commented Jul 19, 2023

@Wirone keep in mind that PER-CS 1.0 is (intentionally) basically the same as PSR-12: https://groups.google.com/g/php-fig/c/bBh4Y-R5gxE

So you could split this up in sub-issues and keep them as valid, and just review them against the PER-CS 2.0 change list: https://github.com/php-fig/per-coding-style/releases/tag/2.0.0

@Wirone Wirone changed the title PSR-12 Support PSR-12 / PER-CS v1 Support Jul 19, 2023
This was referenced Sep 8, 2023
@Wirone Wirone pinned this issue Sep 11, 2023
@github-actions

This comment was marked as outdated.

@Wirone Wirone added this to the long-term-ideas milestone Oct 23, 2023
@Wirone Wirone added the topic/core Core features of Fixer's engine label Jan 31, 2024
@Wirone Wirone removed this from the long-term-ideas milestone Jan 31, 2024
@VincentLanglet
Copy link
Contributor

[In control structures, ]expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

We need to create a fixer that could force boolean operators to always be at the same position with two options like e.g.:

  • position (beginning or end): choose whether the operators should be at the beginning or end of the lines;
  • when (always or mixed): whether the rule should always be applied or only when a mix of positions is found.

TODO

  • support for boolean operator position in control structure expression

Hi @Wirone, what about adding
https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/v3.49.0/doc/rules/operator/operator_linebreak.rst
to the PSR12 ruleset with the option only_booleans => true ?

The rule seems pretty similar to what is asked in PSR12.

Currently it's used by Symfony Ruleset and PSR12 ruleset.

@Wirone
Copy link
Member

Wirone commented Jun 18, 2024

@VincentLanglet yeah, it looks like operator_linebreak does half of the job (does not have an second mentioned config option). But for PSR-12 it should be enough, as it requires boolean operator to always be in the same position (beginning or end, not a mix of both - we can enforce the default (beginning), and it always can be overridden when needed.

@VincentLanglet
Copy link
Contributor

yeah, it looks like operator_linebreak does half of the job (does not have an second mentioned config option)

I agree.

But for PSR-12 it should be enough, as it requires boolean operator to always be in the same position (beginning or end, not a mix of both - we can enforce the default (beginning), and it always can be overridden when needed.

I have made the PR #7868 (comment) but this seemed refused at this time. I can reopen the PR but this might require some discussion with keradus.

@kenguest
Copy link

kenguest commented Jul 10, 2024

there doesn't seem to be a check re line length? ( cf https://www.php-fig.org/per/coding-style/#23-lines )

@Wirone
Copy link
Member

Wirone commented Jul 10, 2024

there doesn't seem to be a check re line length? ( cf php-fig.org/per/coding-style )

No, because:

There MUST NOT be a hard limit on line length.

This is not something that can be automated in sensible way.

@kenguest
Copy link

Would it not then be possible to just emit a warning in that case? (Which is what PHPCodeSniffer does)

@ostrolucky
Copy link
Contributor

Unlike PHPCodesniffer, php-cs-fixer doesn't have a concept of warning or error violations (non automatically fixable error), all the violations it has must be fixable. What it could do is to have some fixer which splits the code to multiple lines according some rules, like https://github.com/symplify/coding-standard?tab=readme-ov-file#linelengthfixer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature request kind/meta Set of actions to be broken down into single issues/PR's topic/core Core features of Fixer's engine topic/PSR-12
Projects
None yet
Development

No branches or pull requests