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

Support for group use declaration (PHP7) #878

Closed
PierreLvx opened this issue Jan 26, 2016 · 25 comments
Closed

Support for group use declaration (PHP7) #878

PierreLvx opened this issue Jan 26, 2016 · 25 comments

Comments

@PierreLvx
Copy link

With PHP 7:

Classes, functions and constants being imported from the same namespace can now be grouped together in a single use statement.

Source + example code.

Using the PSR2 Standard, the following happens (sample output):

----------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------
1 | WARNING | [ ] A file should declare new symbols (classes,
|         |     functions, constants, etc.) and cause no other
|         |     side effects, or it should execute logic with
|         |     side effects, but should not do both. The first
|         |     symbol is defined on line 15 and the first side
|         |     effect is on line 13.
13 | ERROR   | [x] There must be one USE keyword per declaration
13 | ERROR   | [x] Closing brace must be on a line by itself
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

I can get rid of the one USE keyword error excluding PSR2.Namespaces.UseDeclaration. I can also work around the side effects warning excluding PSR1.Files.SideEffects, though not ideal.

However, I have no idea how work around the closing brace error, and I don't want to exclude that rule since I still want it to apply on the rest of the code.

Is there a clean way to work around this issue ?

@gsherwood
Copy link
Member

The side effects and brace errors are caused by PHPCS assigning the curly braces to the function keyword in use function ..., which is incorrect. Fixing that should fix both of those.

Edit: There is already code for that. It's something else.

For the use declaration error, I don't think I can fix that. PSR2 is pretty clear about having one declaration per line, so they would need some errata to change that rule and allow for PHP7 syntax, if they want that to be allowed. But as you've said, you can ignore it in your ruleset.

@gsherwood
Copy link
Member

The issues are actually caused by the curly braces being assigned to the use keyword (or a namespace keyword if you have one up top). Fixing that issue will fix the side effects and brace placement errors.

@gsherwood
Copy link
Member

I've assigned the curly braces new tokens so there is no confusion with sniffs and the tokenizer. It also lets sniffs detect this syntax and apply coding standards to it more easily.

@PierreLvx
Copy link
Author

Excellent, thanks 🙏

@luispabon
Copy link

Did this ever get done and released? I'm on 2.6.1 and php-cs is certainly unhappy with my grouped imports.

@gsherwood
Copy link
Member

Did this ever get done and released? I'm on 2.6.1 and php-cs is certainly unhappy with my grouped imports.

If it is unhappy for reasons like There must be one USE keyword per declaration then that might be because you are using PSR2 and that declares one USE statement per line. I mentioned this in a previous comment as being something I don't think I can change.

If you are getting brace or side effect errors, then please let me know as these should be fixed and the unit tests are currently passing.

@luispabon
Copy link

Apologies, I misunderstood your previous comments on what exactly was fixed. I'm not getting any undue errors due to parsing, PHP-CS is indeed applying PSR-2 correctly in this case.

@gsherwood
Copy link
Member

Apologies, I misunderstood your previous comments on what exactly was fixed. I'm not getting any undue errors due to parsing, PHP-CS is indeed applying PSR-2 correctly in this case.

No problem. Thanks for getting back to me so quickly.

@jasonmccreary
Copy link
Contributor

jasonmccreary commented Jul 31, 2016

With respect to PSR-2, I wonder if there is still more to do here. Specifically around the fixer code. As of 2.6.2, the fixer still seems to corrupt PHP 7 grouped use statements.

I'm considering a patch that determines if the following token is a grouped use curly and, if so, expands the grouped declaration to individual use declarations. Would like to know your thoughts before getting started…

@gsherwood
Copy link
Member

I'm considering a patch that determines if the following token is a grouped use curly and, if so, expands the grouped declaration to individual use declarations. Would like to know your thoughts before getting started…

@jasonmccreary I wasn't aware there was an issue (but there clearly is) so I'd be very happy to look at any PR to fix this.

@jasonmccreary
Copy link
Contributor

Cool. I'll submit something tomorrow as I have a need for it anyway and am happy to give back to this excellent tool.

@gsherwood
Copy link
Member

I'll submit something tomorrow as I have a need for it anyway

Thanks a lot. If you get stuck, just open an issue and we can discuss options.

@gsherwood gsherwood changed the title Support for group use declaration (PHP7) Group use declarations are incorrectly fixed by the PSR2 standard Aug 4, 2016
@gsherwood gsherwood changed the title Group use declarations are incorrectly fixed by the PSR2 standard Support for group use declaration (PHP7) Aug 4, 2016
@gsherwood
Copy link
Member

Changed title of incorrect issue - please ignore

@duggals
Copy link

duggals commented Apr 2, 2017

Has this issue been fully resolved and as part of the update for code sniffer.

Thanks everyone

@gsherwood
Copy link
Member

Has this issue been fully resolved and as part of the update for code sniffer.

This issue was resolved via a PR and released in version 2.6.0. The current stable version is 2.8.1, so it's been around for a few releases.

@nsaumini
Copy link

nsaumini commented Jul 5, 2017

Hi everyone,

Example.php

<?php
namespace App\Demo;

use App\Model\{A, B};

class Example
{

}

I ran command phpcs --standard=PSR2 Example.php and getting following error There must be one USE keyword per declaration.

phpcs version is 3.0.1 (stable)

Please advice on this.

@luispabon
Copy link

@nsaumini the problem is not in phpcs. There's no provision in PSR2 for grouped imports, hence phpcs correctly calling out on their usage.

@nsaumini
Copy link

nsaumini commented Jul 5, 2017

@luispabon Noted thanks.

@luispabon
Copy link

luispabon commented Jul 5, 2017

PSR are working on a new coding style extending PSR2 btw, hopefully it'll be accepted/published soonish:

https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md

That'd include a lot of new stuff that came in on PHP7.

@kgasienica
Copy link

There's only hope left, or we've got any information when it will be published :D ?

@vikkio88
Copy link

any info on this?

@agriboed
Copy link

agriboed commented Apr 19, 2018

   <rule ref="PSR2.Namespaces.UseDeclaration">
        <exclude-pattern>*</exclude-pattern>
    </rule>

in your ruleset.xml resolves a problem while we waiting for update :)

@vikkio88
Copy link

I am using that already, thanks @agriboed

@J7mbo
Copy link

J7mbo commented Jun 12, 2018

Anyone solved this apart from the exclude pattern above? PHP 7 has been out for a little while now...

@albertcito
Copy link

  • I created a new file in my root directory called phpcs.xml.
  • Copy/paste the content from the original file ruleset.xml
  • Add the rule in my local file:
  <rule ref="PSR2.Namespaces.UseDeclaration">
        <exclude-pattern>*</exclude-pattern>
    </rule>
  • Run: phpcs --standard=phpcs.xml --extensions=php app

I'm using it for now, to no write directly the ruleset.xmlfile from global composer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests