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

PSR12: Support multiple lines control structure spacing sniffs #2113

Closed
clement-michelet opened this issue Aug 7, 2018 · 8 comments
Closed

Comments

@clement-michelet
Copy link

Following the PSR12 :

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. The closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

<?php

if (
    $expr1
    && $expr2
) {
    // if body
} elseif (
    $expr3
    && $expr4
) {
    // elseif body
}

The current PSR12 ruleset will show the following error code : PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace

The expected behavior is that no error is shown for the example code.

@clement-michelet clement-michelet changed the title PSR12: Support multiple line controle structure spacing sniffs PSR12: Support multiple lines control structure spacing sniffs Aug 7, 2018
@gsherwood
Copy link
Member

Just a note in case you are trying to use the PSR-12 standard and not just test it: the included PSR-12 standard is not complete (I released it as a preview) and I'm unable to complete it due to the inconsistencies in the PSR-12 spec itself.

Since PSR-12 is now back in draft, I'm not going to spend a lot of time on it until a few things start to change.

But this one is a pretty clear change from PSR-2 and is failing because I just used the PSR-2 sniff to cover a bunch of cases. This needs to be fixed up, probably by refactoring and extending the PSR-2 sniff.

@gsherwood gsherwood added this to Backlog in PHPCS v3 Development via automation Aug 12, 2018
@gsherwood gsherwood moved this from Backlog to Selected for Development in PHPCS v3 Development Aug 12, 2018
@gsherwood gsherwood moved this from Selected for Development to Backlog in PHPCS v3 Development Apr 3, 2019
@ADmad
Copy link

ADmad commented Apr 10, 2019

Since 3.4.1 using following code and PSR12 standard phpcs incorrectly reports the error shown below.

<?php
for (
    $i = 0;
    $i < 10;
    $i++
) {
    // for body
}
----------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------
 2 | ERROR | [x] Expected 0 spaces after opening bracket; newline found (PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace)
 3 | ERROR | [x] Expected 1 space after first semicolon of FOR loop; newline found
   |       |     (Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterFirst)
 4 | ERROR | [x] Expected 1 space after second semicolon of FOR loop; newline found
   |       |     (Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterSecond)

@JosephLeedy
Copy link

Now that PSR-12 has been ratified, any timeline on when this issue will be fixed?

@gsherwood
Copy link
Member

any timeline on when this issue will be fixed

No, I can't provide a time.

@scheb
Copy link

scheb commented Aug 20, 2019

What's your opinion about this solution scheb@73a6ccf ? It would allow PSR2 standard to behave as before, but allow newlines within PSR12.

I did not change all the sniffs yet, because I wanted to get some opinions first. The principle would be the same for others such as ForLoopDeclaration.

Besides that, I don't understand how to write a test case for a sniff with certain properties set. Maybe someone be so kind to point me to an example.

@puggan
Copy link

puggan commented Aug 21, 2019

Can't see any other test using properties, but i guess you can write directly to $ruleset->ruleset

$config            = new Config();
$config->standards = ['PSR2'];
$config->sniffs    = ['PSR2.ControlStructures.ControlStructureSpacing'];

$ruleset = new Ruleset($config);
$ruleset->ruleset['PSR2.ControlStructures.ControlStructureSpacing']['properties']['allowNewLineAfterOpeningBrace'] = true;

Or just load the PSR12

$config            = new Config();
$config->standards = ['PSR12'];
$config->sniffs    = ['PSR2.ControlStructures.ControlStructureSpacing'];

$ruleset = new Ruleset($config);

@jrfnl
Copy link
Contributor

jrfnl commented Aug 21, 2019

Besides that, I don't understand how to write a test case for a sniff with certain properties set. Maybe someone be so kind to point me to an example.

@scheb Use // phpcs:set .... in the unit test case file to test with specific property settings. Just search for that in the /Tests/*/*.inc files and you'll see plenty of examples.

@gsherwood
Copy link
Member

I've just noticed this is still open, but the newline issue has already been resolved in the PSR12 standard for 3.5.0 and this should have been closed then.

The solution was to write a new sniff for PSR12.

PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Oct 27, 2019
@gsherwood gsherwood removed this from Ready for Release in PHPCS v3 Development Oct 27, 2019
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.

7 participants