- 🇦🇹Austria klausi 🇦🇹 Vienna
Alphabetical sorting has now been implemented in Coder and will be released soon. Escalating priority here to finalize the written coding standard.
- 🇦🇺Australia darvanen Sydney, Australia
If there's a cs rule now does that mean it's upgraded to MUST?
use
statements MUST be in alphabetical order - 🇦🇹Austria klausi 🇦🇹 Vienna
Yes, there is now a Coder rule that enforces it as a MUST, since Coder can only throw an error or not.
- 🇦🇺Australia darvanen Sydney, Australia
Since that's passing now I've updated the IS.
- Status changed to Needs review
almost 2 years ago 6:05am 13 June 2023 - 🇦🇺Australia darvanen Sydney, Australia
Updated code standards documentation at https://www.drupal.org/docs/develop/coding-standards/namespaces#order →
- Status changed to RTBC
almost 2 years ago 5:10pm 17 June 2023 - 🇬🇧United Kingdom jonathan1055
Can this now be marked as Fixed? Coder has the sniff, Coder 8.3.19 → was release 9 June, and docs page is updated.
- 🇳🇿New Zealand jweowu
A case-insensitive sort is unstable in principle (even if that is unlikely to be a factor in lists of
Use
statements in practice), and so it's an inferior option. This really should be case-sensitive, as that is the sane way to sort lines of case-sensitive code!The case-insensitive approach appears to have been chosen because "that's what PHP Storm does", which isn't a great reason to pick a worse option (but I understand that it was an easy way to obtain a mostly-good result, so I get it).
However... iff there's a way to tell PHP Storm to do this sensibly via a config file (ala
.editorconfig
), I'd be very much in favour of doing that and switching the standard to be case-sensitive. - Status changed to Needs work
over 1 year ago 9:23am 26 October 2023 - 🇬🇧United Kingdom catch
This should be a case sensitive sort so that it's deterministic.
The coding standards change was never officially approved via the coding standards process, which is understandable since that was dormant for about four years, but now that it's up and running again we should try to do it properly.
Marking needs work for:
1. Needs an updated issue summary following the template.
2. We should probably revert and then re-add the documentation changes.
3. If the coder rule is doing case insensitive sort, then we might need an issue to make it case sensititive
4. We should have a core issue to apply the new rule. - 🇫🇷France andypost
Would be great to mention in standard usage of PHP functions
- 🇬🇧United Kingdom jonathan1055
Before we go any further, and responsding to #38, do we need to find out if there is a way to tell PHPStorm to order with a case-sensitive sort? If there is not, then is that a breaker? Do we still push ahead with requiring a case-sensitive sort, even if popular IDEs will re-order and mess it up?
Replying to #39.3
if the coder rule is doing case insensitive sort, then we might need an issue to make it case sensititive
In ✨ Add sniff to check and fix the order of Use statements Fixed the sniff implemented is the external
SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses
which currently only does a case-insensitive order. So we could raise a SlevomatCodingStandard issue to add a config option to allow case-sensitive or insensitive sorting. - Status changed to Needs review
over 1 year ago 8:58pm 30 October 2023 - 🇬🇧United Kingdom catch
Actually I kind of wonder whether we're ever going to notice the difference in practice between case sensitive and case insensitive sort, I can't think of a single case with core or contrib (or vendor) code where this would actually come up.
Given that, would it actually matter? Moving back to needs review.
- 🇳🇿New Zealand jweowu
My thought in #38 was that as the "case insensitive" decision appears to have been based on PHPStorm's default (but fairly bizarre) behaviour, we should change it if there's a way to tell PHPStorm via some config file in the Drupal repository to use case-sensitive sorting.
Without such a config file to automate the change for PHPStorm users they would presumably need to make a config change manually, in which case users will still encounter inconsistencies in this area -- just not the same users as before (I imagine it would be an improvement for pretty much everything other than PHPStorm).
Overall, case-sensitive is clearly correct, so in principle we should be using that, but I agree that the case-insensitive sorting of 'use' statements is highly unlikely to cause instability in practice, so it might just come down to "which alternative annoys the fewest users?"
(And on that note, I'm only guessing that PHPStorm is pretty much on its own in defaulting to case-insensitive sorting behaviour. I'm honestly just surprised that even one editor is doing that, and so I'm assuming it's probably only one, but I don't know that for sure.)
- 🇳🇿New Zealand quietone
When I sort lines in PHPStorm they are sorted case sensitive. If this is a setting somewhere I do not recall setting it nor have I found it.
This sniff is enabled for commerce_migrate and I could not use PHPStorm to sort the lines to pass phpcs.
Am I the only one using PHPStorm that has this behavior?
- 🇳🇿New Zealand quietone
The Coding Standards Committee has developed a custom issue template → to assist in managing issues efficiently. We have already found that it's use is helping. For issues created before the template was in use I am adding the new template to the Issue Summary and asking everyone here to help convert to it..
Thank you for your help!
- Status changed to Needs work
over 1 year ago 6:25am 8 November 2023 - 🇳🇿New Zealand jweowu
> When I sort lines in PHPStorm they are sorted case sensitive. If this is a setting somewhere I do not recall setting it nor have I found it. This sniff is enabled for commerce_migrate and I could not use PHPStorm to sort the lines to pass phpcs. Am I the only one using PHPStorm that has this behavior?
Looking back at ✨ Add sniff to check and fix the order of Use statements Fixed I think it was https://www.drupal.org/project/coder/issues/3310013#comment-15101090 ✨ Add sniff to check and fix the order of Use statements Fixed which had informed my comments here. I presume that will explain what you're seeing.
- 🇬🇧United Kingdom jonathan1055
Excellent. So that answers my first question in #41. PHPStorm will sort using proper case-sensitive when you use as per #3310013-30: Add sniff to check and fix the order of Use statements →
So we can proceed by requiring the Drupal standard to also use a case-sensitive sort.
We will need to check if the existing sniff has any config option for case sensitive/insensitive, and if not then raise a ticket on SlevomatCodingStandard. But that should not hold up progress on getting the standard agreed.
Updated the issue summary with first draft, please add your support and suggest improvements to the text.
- Status changed to Needs review
12 months ago 1:15am 8 April 2024 - 🇦🇺Australia acbramley
Create a CR here https://www.drupal.org/node/3439320 → (not sure what the versions should be)
The IS has been updated since #46
This should be ready for review.
- 🇦🇺Australia kim.pepper 🏄♂️🇦🇺Sydney, Australia
SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses
defaults to case-insensitive but looks like we can configure that to be case-sensitive.https://github.com/slevomat/coding-standard/blob/master/SlevomatCodingSt...
- 🇺🇸United States dww
Case sensitive sorting sounds good to me, too. Adding myself as another supporter. Other minor edits to the summary since we don't SHOUT in our standards anymore.
I don't use an IDE (emacs FTW), so I can't comment on how to get PHPStorm to do this properly. That does seem like a valid concern, and I think it'd be important to sort that out as part of this issue. That's the only reason I'm not moving this to RTBC, since otherwise, it's ready.
I was confused when this was brought up in Slack since I remember having to get the ordering right to make phpcs happy, but now I see it's that Coder already implemented this, and we're retroactively approving the policy now. 😂
- 🇦🇺Australia kim.pepper 🏄♂️🇦🇺Sydney, Australia
We do have a rule in coder_sniffer but it's not case-sensitive https://git.drupalcode.org/project/coder/-/blob/8.3.x/coder_sniffer/Drup...
- 🇦🇺Australia acbramley
When I sort lines in PHPStorm they are sorted case sensitive
So that answers my first question in #41. PHPStorm will sort using proper case-sensitive when you use "Code > Optimize Imports"
For me, Optimize Imports actually sorts insensitively. Using the example in the IS I get
use Drupal\car\CarInterface; use Drupal\Core\Controller\ControllerBase; use Drupal\door\DoorInterface;
After running Optimize Imports. This is very common to have on as a Save action so this would be very disruptive if we enforced case sensitivity.
If I turn off the auto optimize setting, and just have the "Sort 'use' statements" option set to Alphabetically in Editor > Code Style > PHP > Code Conversion it actually does nothing... so not sure how @quietone is getting the opposite results.
- Status changed to RTBC
12 months ago 3:18am 8 April 2024 - 🇺🇸United States dww
Apologies, I missed #48 which includes the PHPStorm specifics on how to make this work. Added a blurb to the summary about that (needs wordsmithing, but doesn't need to hold this up). Moving to RTBC.
- Status changed to Needs review
12 months ago 3:20am 8 April 2024 - 🇺🇸United States dww
Bah, x-post. Looks like there's still confusion on PHPStorm. I think y'all IDE users should sort that out before this is RTBC. 😅
- 🇦🇺Australia mstrelan
I suspect "Optimize imports" depends on other configuration such as inspections and code style. For example there is an option to sort use statements alphabetically or by length (who would use that?). Possibly it even looks at phpcs configuration.
- Status changed to Needs work
12 months ago 4:53am 8 April 2024 - 🇬🇧United Kingdom joachim
> use statements must be located after the initial @file phpDoc block (and after the namespace declaration, if any).
The wording of this looks like it dates back to when most of our files were procedural and had a @file docblock and only rarely had a namespace.
That's not the case now - we mostly have class files.
It should say something like:
> use statements must be located after the initial @file phpDoc block (if any) and after the namespace declaration (if any).
- Status changed to Needs review
12 months ago 7:19am 8 April 2024 - 🇺🇸United States dww
@joachim: Feel free to make minor edits like that yourself. You don't need to set to NW for something like that, just fix it.
- 🇺🇸United States dww
Merged the 2nd and 3rd bullet points, since they were both about case sensitive sorting.
- 🇳🇱Netherlands kingdutch
The relevant PHPStorm issue to get this fixed for IDE users is https://youtrack.jetbrains.com/issue/WI-26078/Automatically-added-use-st...
- 🇳🇿New Zealand quietone
This section → already has information about the placement of use statements in the file. For classes the placement is shown by example and for files without a namespace there is a item for it. Therefore, I don't think the first item of the proposed text changes is needed.
And I disagree with adding how to setup an IDE in the Coding Standards. Perhaps, that is not the intention.
Also, I expect that any software developer knows what case-sensitive ordering is so we don't need the last line. Correct me if I am wrong.
So, altogether I think this proposed text could be like this.
use
statements should be in case-sensitive order.
Example:use Drupal\Core\Controller\ControllerBase; use Drupal\car\CarInterface; use Drupal\door\DoorInterface;
Opinions?
- 🇬🇧United Kingdom joachim
> Also, I expect that any software developer knows what case-sensitive ordering is so we don't need the last line. Correct me if I am wrong.
I personally have no idea in what order it places upper and lower case.
Also, some IDEs handle ordering differently -- I can't find the issue, but I recently discovered that. IIRC it was something like the ordering of 'Foo\Bar' and 'FooBar' was different in PHP and VSCode, or PHP and one of the PHPCS-type tools.
- 🇺🇸United States dww
For the record, I’m not proposing the “standards” include PHPStorm instructions. I only added it as a related documentation change that we could / should do while fixing this.
- 🇳🇿New Zealand quietone
I personally have no idea in what order it places upper and lower case.
Ah, good to know. I guess that is my bias. I did a lot of work in Octal and used to know the number for many characters. And, of course, using
man ascii
when I forget.I did some reading on hyphens and I think one is not needed after upper and lower. And since this last sentence is explaining the first maybe they should be together like this.
use
statements must be sorted alphabetically, in case-sensitive order. Case-sensitive ordering places upper case before lower case, soCore
precedescar
.
Example:use Drupal\Core\Controller\ControllerBase; use Drupal\car\CarInterface; use Drupal\door\DoorInterface;
I use PHPStorm and it has always used case sensitive sorting and I have not changed any setting for this. Is this different for others? Here is the relevant screenshot, which does not have an option for changing the case sensitivity of the sort. Maybe the behavior was different on older versions?
- 🇬🇧United Kingdom joachim
I found the issue! https://github.com/slevomat/coding-standard/issues/1667
There is disagreement on the relative ordering of \ and _.
Quoting from there:
The coding standard wants the lines to be like this:
```
use Drupal\action_link\Ajax\ActionLinkMessageCommand;
use Drupal\action_link\Entity\ActionLinkInterface;
use Drupal\action_link\Plugin\ActionLinkStyle\Ajax;
use Drupal\action_link_formatter_links\DisplayBuildAlter;
```But if I select them in VSCode and do 'Sort lines ascending' I get this:
```
use Drupal\action_link_formatter_links\DisplayBuildAlter;
use Drupal\action_link\Ajax\ActionLinkMessageCommand;
use Drupal\action_link\Entity\ActionLinkInterface;
use Drupal\action_link\Plugin\ActionLinkStyle\Ajax;
```So basically, I was ordering using the command in VSCode, then getting the PHPCS error, fixing it to satisfy that, then next time I import a class, VSCode arranges the order again in a way that makes PHPCS unhappy.
- 🇦🇹Austria klausi 🇦🇹 Vienna
Merged 📌 Configure use-statement sniff to be case sensitive Active in Coder to switch on case-sensitive sorting.
Escalating this again to major, as we still have no coding standard although this rule is running in Coder for a year now.
I like the proposed text of @quiteone in #67.
- 🇬🇧United Kingdom jonathan1055
I have updated the issue summary with the proposed changes by @quietone in #67
I think we are now at step 4 - Review by the Coding Standards Committee. But according to the full detailed steps → the issues needs to marked RTBC for that. Do we need to address the problem in #68 about
\
vs_
? - 🇳🇿New Zealand jweowu
There's really no cause for confusion. Backslash is ascii 92. Underscore is ascii 95. Hence backslash sorts before underscore.
I can't think of a locale we might be interested in where that order would flip (if any even exist). Certainly `C` and `UTF-8` locales sort these characters in ascii sequence, and I don't think there's any reason why we'd deviate.
printf '\\\n_\n' | env LC_COLLATE=C sort printf '\\\n_\n' | env LC_COLLATE=UTF-8 sort
If VSCode is defaulting to the reverse when sorting lines of text, that just sounds like a VSCode bug/feature, either to be fixed upstream or configured to be more sensible (but I couldn't guess which is more likely).
- 🇦🇺Australia mstrelan
Agree with #71, VSCode is wrong and there is an existing issue for it.
- Status changed to RTBC
7 months ago 12:44pm 11 September 2024 - 🇬🇧United Kingdom jonathan1055
Thank you @jweowu and @mstrelan, that is excellent. I have update the issue summary and the change record.
So this issue is now RTBC and we can move to Step 4 - review by Coding Standards Committee
- 🇦🇺Australia acbramley
#48 doesn't seem to be correct, I've got a bunch of files failed on Diff module that have been sorted via PHPStorm. E.g
use Drupal\Core\Url; use Drupal\node\Entity\Node; use Drupal\node\Entity\NodeType; use Drupal\Tests\views\Functional\ViewTestBase;
If I reorder these with case sensitivity, and then run PHPStorm's action, it will put them back like this. This is going to cause major headaches for people using storm.
- 🇳🇿New Zealand quietone
When I use
Edit->Sort lines
in PHPStorm it is case sensitive. Is the difference a plugin? The only plugin I have enabled for strings is 'Wrap to Column' - 🇦🇺Australia acbramley
@quietone yes that is different, PHPStorm has a built in "Optimise imports" function which, on save, will sort use statements. This is not case sensitive from my testing.
- 🇦🇺Australia acbramley
This was also mentioned back in #62 - PHPStorm users are essentially now forced to disable Optimise imports which is quite a shame...
Developing contrib is going to become more cumbersome as well since phpcbf is not always runnable on contrib (e.g if it doesn't have a committed phpcs.xml file).
- 🇦🇹Austria klausi 🇦🇹 Vienna
A workaround is to install the Drupal coding Standard in phpstorm with newest Coder, then the use statements will be ordered correctly.
I think we cannot revert the Coder change now as people are already adopting this new standard.
Please push phpstorm to fix this issue: https://youtrack.jetbrains.com/issue/WI-26078/Automatically-added-use-st...
- 🇦🇺Australia acbramley
A workaround is to install the Drupal coding Standard in phpstorm with newest Coder, then the use statements will be ordered correctly.
I tried this and it did not work. The only thing I can think of is people have phpcbf running on save which of course would fix it but as per #78 is not always going to work.
- 🇦🇺Australia acbramley
I've got something that works! Thank you @el7cosmos for showing me the Quality Tools > External Formatters setting.
Steps to setup code formatting on save:
1. Go to PHP > Quality Tools. Select "PHP Code Beautifier and Fixer" (this seems to run phpcbf)
2. Go to Tools > Actions on Save. Tick "Reformat code" and choose PHP files (or others if you want). Make sure Whole file is selected, changed lines doesn't seem to work properly).
Now when you save, it will effectively run phpcbf which seems quite fast and will fix up incorrectly ordered use statements, as well as other stuff for you.
This will still have issues with contrib development as per above, but is a nice stopgap.
- 🇮🇹Italy plach Venezia
If you are using PHPStorm, please upvote the related issue. This should increase the likelihood we get a fix in timely fashion.
- 🇺🇸United States mpotter
Now that this update has broken all of our builds and I'm finding this issue for the first time, I feel like the initial "example" that people are referencing was misleading. The example of case-sensitive order was:
use Drupal\Core\Controller\ControllerBase; use Drupal\car\CarInterface; use Drupal\door\DoorInterface; use Drupal\door_handle\HandleInterface;
But this is another example of what case-sensitive order would be:
use Drupal\Core\Controller\ControllerBase; use Drupal\DoorHandle\Something use Drupal\car\CarInterface; use Drupal\door\DoorInterface; use Drupal\door_handle\HandleInterface;
because "D" is < "c". This is actually more difficult for me personally to parse and read. I prefer what PHPStorm is doing. But I guess this decision has already been made and we are stuck with it because the new version was already released. But it's causing a major amount of work for us to update all of our projects to pass our linting/coder validation.
- 🇬🇧United Kingdom jonathan1055
Just for info, if you add the following to your project's phpcs.xml(.dist) file to put the case-sensitive setting back to false:
<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses"> <properties> <property name="caseSensitive" value="false"/> </properties> </rule>
the order that phpcbf fixer gives for mpotter's example in #83 is:
use Drupal\car\CarInterface; use Drupal\Core\Controller\ControllerBase; use Drupal\door\DoorInterface; use Drupal\door_handle\HandleInterface; use Drupal\DoorHandle\Something;
- 🇳🇿New Zealand jweowu
it's causing a major amount of work for us to update all of our projects to pass our linting/coder validation.
It sounds like a relatively easy Perl script to update them all in bulk, FWIW.
- 🇦🇹Austria klausi 🇦🇹 Vienna
No perl command needed, you can run
phpcbf
and it will bulk fix all use statement ordering for you. - 🇨🇦Canada minoroffense Ottawa, Canada
I feel as though big coding standards changes like this should follow a core release or major versioning release of coder or something. We have versioning for core APIs why not versions of the coding spec.
We have dozens of individual modules to go and update now and every CI build is broken (including client releases where this spec has nothing to do with what they're trying to ship). Especially for what is arguably a cosmetic change to the spec.
Sure running the beautifier to autofix is great and all but it takes time to do that, time to explain to clients why everything is broken all of a sudden and more time to create follow up releases etc...
- 🇩🇪Germany donquixote
Also, I expect that any software developer knows what case-sensitive ordering is so we don't need the last line. Correct me if I am wrong.
I would disagree.
A case-sensitive comparison is clear.
But a case-sensitive order, not so much. It could be derived from a character set (e.g. ASCII) where letters have numeric representations, but this is by no means clear. It could also be based on whatever specific php sort functions do.
E.g. for regular letters it could be "A, B, C, ..., a, b, c, ..." or it could be "A, a, B, b, C, c, ...".
For special letters like 'ä' it is less clear. Although I think we don't allow them in namespaces or module names, or people just avoid them and stick to a...z.slevomat uses strcasecmp() and strcmp() depending on the setting.
This will result in "A, B, C, ..., a, b, c..." for regular chars.Here are some experiments, https://3v4l.org/iUKZh
With my current version of PhpStorm, the auto sort with Ctrl+Alt+O (Optimize imports) puts the imports in the "wrong" order according to the new standard.
But I also see cases of the wrong order in Drupal core (maybe because I am not in latest version). - 🇬🇧United Kingdom jonathan1055
Following the comments in #83 and #88 I have updated the proposed text and example, to show that all uppercase letters come before the lowercase letters.
As per comment #73 we are still at step 4 'Review by Coding Standards Committee'
- 🇬🇧United Kingdom longwave UK
After upgrading Coder on a customer project and then running into the issues identified here where PHPStorm sorts differently by default, I am -1 on the specific proposal here. To me it's never been a problem the way things were, and I would prefer just to let PHPStorm handle things where possible instead of making my workflow more complicated.
- 🇳🇿New Zealand quietone
@longwave, what is the 'default' sorting for you? When I use PHPStorm to sort lines it is definitely alphabetical and case-sensitive.
- 🇦🇺Australia mstrelan
The "sort lines" function in phpstorm works differently to automatic imports (e.g. with autosuggest, copy/pasting, etc) as well as the "optimize imports" function, so you only get the correct order if you manually choose to sort lines.
- 🇬🇧United Kingdom longwave UK
A simple example borrowed from my project. If I have:
use Drupal\Component\Utility\Html;
and let PHPStorm autocomplete the
BlockContent
entity class it will order them like this:use Drupal\block_content\Entity\BlockContent; use Drupal\Component\Utility\Html;
This fails the sniff:
9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\Html.
Manually selecting the statements and invoking "Sort lines" will re-sort them as per the sniff:
use Drupal\Component\Utility\Html; use Drupal\block_content\Entity\BlockContent;
But pressing Ctrl+Alt+O "optimize imports" will re-sort them the other way again:
use Drupal\block_content\Entity\BlockContent; use Drupal\Component\Utility\Html;
- 🇬🇧United Kingdom catch
Let's move this back to needs review. The idea of coding standards is so that stylistic things don't get in the way when writing code, but if this is incompatible across this many IDEs, then it's going to get in the way (and already is given that coder apparently enabled this by default already, although probably good overall since it's flagged it earlier).
We could adopt the standard, but postpone/revert coder implementation and core implementing the phpcs rule until at least the most popular IDEs are compatible, but not sure who that helps.
I don't use an IDE so it doesn't affect me but fiddling with stuff like this is one of the reasons I've continued to not use one.
- 🇺🇸United States DamienMcKenna NH, USA
Taking a leaf out of the PSR coding standard approach here.. if there's an existing tool that sorts them in a case-insensitive way, and if most text editors (*cough* Textmate *cough*) & IDEs want to sort them in a case-insensitive way, why are we not following the existing defacto standard of case-insensitive sorting? This feels like trying to shoehorn a specific approach on the community and on IDE developers rather than adopting what's already in use.
- 🇬🇧United Kingdom joachim
There's also the problem of disagreements between the order of \ and _.
e.g. in WizardPluginBase:
use Drupal\views\Views; use Drupal\views_ui\ViewUI;
- 🇦🇹Austria klausi 🇦🇹 Vienna
Hm, sorry that this caused problems for many people :-|
I think it is probably best to open a new Coder issue to carefully list the pros and cons of a revert, and also workarounds like configuring phpcbf in Phpstorm or disabling this sniff so that you don't have a problem. Some projects have adopted the case-sensitive standard already, so a revert creates work again for them.
Vscode: line sorting is almost correct in our case-sensitive standard and wrong in our case-insensitive standard.
Phpstorm: line sorting is correct in the case-sensititve standard, but "optimize import" is wrong in the case sensitive standard. Reversed for the case-insensitive standard.
Textmate: line sorting is wrong in the case-sensititve standard.Difficult choice how and where we want to be wrong with our standard. I would say Vscode is more important than Textmate for example, but a hard choice.
Having no sorting standard for use statements is even worse, makes it harder to find use statements in longer lists.
- 🇦🇺Australia acbramley
Some projects have adopted the case-sensitive standard already, so a revert creates work again for them.
This was the stance just days after the change was committed. Now we've seen how many more people it's affecting. I think it's safe to say it would have been easier for majority of people to revert this change then. Even now it may still be. Commits are easier to revert than having to phpcbf all of your contrib modules for reasons already explained above.
- 🇳🇿New Zealand jweowu
Just to be clear, it was also affecting people prior to the recent change. It's simply affecting different people since the change.
Reverting the change means that the original set of people get the problem again. Which is an option, for sure, but it's not making the problem go away.
- 🇩🇪Germany donquixote
In the issue summary I don't see any argument why the proposed order (case sensitive) would be preferable to something else.
I also don't see a statistical analysis for existing code in and outside of Drupal.
(might be in the comments)
I only see the argument that having a convention is better than having no convention.As for existing code, the following two regex searches can be used to search in a vendor directory:
(I did the search in PhpStorm with case sensitivity and multiline enabled)use ((\w+\\)+\w+)[A-Z]\w+.*; use \1[a-z]\w+.*;
use ((\w+\\)+\w+)[a-z]\w+.*; use \1[A-Z]\w+.*;
The "\1" references a previously captured substring.
For all the code I found, it seemed that case insensitive sorting was used.
But even in a typical Drupal project these were just a few matches overall.Drupal is an outlier because module names in namespaces are lowercase.
(I remember back in the days I argued for this, so that we see the literal module name in the namespace, not a captitalized form. I still think this was the right choice.) - 🇩🇪Germany donquixote
Reverting the change means that the original set of people get the problem again. Which is an option, for sure, but it's not making the problem go away.
I would propose a more flexible solution:
- We choose one default sort order that will also be used for Drupal core. This should follow what existing tools already support, and what is common in existing 3rd party code.
- Contrib modules or website projects are allowed to use their own order. We can reference documentation how to configure this with Coder and phpcs.xml. - 🇵🇪Peru krystalcode
So, what is the purpose of sorting use statements? Is it to please PHPStorm or X, Y or Z text editor so that when you use its sorting feature it is sorted the way the editor wants? And then run PHPCS and get it to give you green lights so that the pipeline passes? Or is it to randomly choose an algorithm i.e. case-sensitive or case-insensitive and agree that everybody follows it?
To me, no, it's about human readability. Even that, using find/search functionality in most text editors you can easily find a use statement directly without having to look for it with the eye. So, the most important circumstance where it matters is when you want to review all use statements in a file, or to find a statement that you don't quite remember what exactly it is and can't use search e.g. I don't know which PSR-18 HTTP client is being used so I can't search specifically for "Guzzle" to find the import statement - I need to look for it manually.
That is, we should be taking an approach based on how the human eye/brain would more comfortably and quickly read the use statements, not how a software or algorithm would do. The software does not care, give it a rule and it will follow it.
What I have concluded to work best is to follow a more intelligent approach and use logical groups of statements separated by an empty or comment line. Do that only if the number of statements justify it - otherwise it's not necessary e.g. if it's more than 5. And if the number of statements in a group grows too much, break it further into subgroups.
Note that PSR-12 might not define the sorting algorithm to use, but it does define as a MUST to group use statements for classes, functions and constants.
I find it really easy for the eye to find what it's looking for when I follow this method.
Here is an example from
group.module
:use Drupal\Component\Utility\Html; use Drupal\Core\Access\AccessResult; use Drupal\Core\Config\Entity\ConfigEntityInterface; use Drupal\Core\Database\Query\AlterableInterface; use Drupal\Core\Database\Query\SelectInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; use Drupal\group\Entity\GroupRelationshipInterface; use Drupal\group\Entity\GroupInterface; use Drupal\group\Entity\Storage\ConfigWrapperStorageInterface; use Drupal\group\Entity\Storage\GroupRelationshipStorageInterface; use Drupal\group\Entity\Storage\GroupRoleStorageInterface; use Drupal\group\QueryAccess\EntityQueryAlter; use Drupal\group\QueryAccess\GroupRelationshipQueryAlter; use Drupal\group\QueryAccess\GroupQueryAlter; use Drupal\views\Plugin\views\query\QueryPluginBase; use Drupal\views\Plugin\views\query\Sql; use Drupal\views\ViewExecutable;
I would write this as follows:
// Drupal modules. use Drupal\group\Entity\GroupRelationshipInterface; use Drupal\group\Entity\GroupInterface; use Drupal\group\Entity\Storage\ConfigWrapperStorageInterface; use Drupal\group\Entity\Storage\GroupRelationshipStorageInterface; use Drupal\group\Entity\Storage\GroupRoleStorageInterface; use Drupal\group\QueryAccess\EntityQueryAlter; use Drupal\group\QueryAccess\GroupRelationshipQueryAlter; use Drupal\group\QueryAccess\GroupQueryAlter; use Drupal\views\Plugin\views\query\QueryPluginBase; use Drupal\views\Plugin\views\query\Sql; use Drupal\views\ViewExecutable; // Drupal core. use Drupal\Component\Utility\Html; use Drupal\Core\Access\AccessResult; use Drupal\Core\Config\Entity\ConfigEntityInterface; use Drupal\Core\Database\Query\AlterableInterface; use Drupal\Core\Database\Query\SelectInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Field\FieldItemListInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface;
Here is an example from
Drupal\graphql_core_schema\Plugin\GraphQL\Schema\CoreComposableSchema
:use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element\Checkboxes; use Drupal\Core\TypedData\TypedDataTrait; use Drupal\graphql\GraphQL\ResolverBuilder; use Drupal\graphql\GraphQL\ResolverRegistry; use Drupal\graphql\GraphQL\ResolverRegistryInterface; use Drupal\graphql\Plugin\GraphQL\Schema\ComposableSchema; use Drupal\graphql\Plugin\SchemaExtensionPluginInterface; use Drupal\graphql\Plugin\SchemaExtensionPluginManager; use Drupal\graphql_core_schema\CoreComposableConfig; use Drupal\graphql_core_schema\CoreComposableResolver; use Drupal\graphql_core_schema\CoreSchemaExtensionInterface; use Drupal\graphql_core_schema\CoreSchemaInterfaceExtensionInterface; use Drupal\graphql_core_schema\EntitySchemaBuilder; use Drupal\graphql_core_schema\Form\CoreComposableSchemaFormHelper; use Drupal\graphql_core_schema\GraphQL\Enums\DrupalDateFormatEnum; use Drupal\graphql_core_schema\GraphQL\Enums\EntityTypeEnum; use Drupal\graphql_core_schema\GraphQL\Enums\LangcodeEnum; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderGenerator; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderRegistry; use Drupal\graphql_core_schema\TypeAwareSchemaExtensionInterface; use Drupal\typed_data\DataFetcherTrait; use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\InterfaceTypeDefinitionNode; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\Parser; use GraphQL\Type\Schema; use GraphQL\Utils\BuildSchema; use GraphQL\Utils\SchemaExtender; use GraphQL\Utils\SchemaPrinter; use Symfony\Component\DependencyInjection\ContainerInterface;
// Drupal modules. use Drupal\graphql\GraphQL\ResolverBuilder; use Drupal\graphql\GraphQL\ResolverRegistry; use Drupal\graphql\GraphQL\ResolverRegistryInterface; use Drupal\graphql\Plugin\GraphQL\Schema\ComposableSchema; use Drupal\graphql\Plugin\SchemaExtensionPluginInterface; use Drupal\graphql\Plugin\SchemaExtensionPluginManager; use Drupal\graphql_core_schema\CoreComposableConfig; use Drupal\graphql_core_schema\CoreComposableResolver; use Drupal\graphql_core_schema\CoreSchemaExtensionInterface; use Drupal\graphql_core_schema\CoreSchemaInterfaceExtensionInterface; use Drupal\graphql_core_schema\EntitySchemaBuilder; use Drupal\graphql_core_schema\Form\CoreComposableSchemaFormHelper; use Drupal\graphql_core_schema\GraphQL\Enums\DrupalDateFormatEnum; use Drupal\graphql_core_schema\GraphQL\Enums\EntityTypeEnum; use Drupal\graphql_core_schema\GraphQL\Enums\LangcodeEnum; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderGenerator; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderRegistry; use Drupal\graphql_core_schema\TypeAwareSchemaExtensionInterface; use Drupal\typed_data\DataFetcherTrait; // Drupal core. use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element\Checkboxes; use Drupal\Core\TypedData\TypedDataTrait; // External libraries. use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\InterfaceTypeDefinitionNode; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\Parser; use GraphQL\Type\Schema; use GraphQL\Utils\BuildSchema; use GraphQL\Utils\SchemaExtender; use GraphQL\Utils\SchemaPrinter; use Symfony\Component\DependencyInjection\ContainerInterface;
And since the first group in this example is quite big, I'd break it further down to:
// This module. use Drupal\graphql\GraphQL\ResolverBuilder; use Drupal\graphql\GraphQL\ResolverRegistry; use Drupal\graphql\GraphQL\ResolverRegistryInterface; use Drupal\graphql\Plugin\GraphQL\Schema\ComposableSchema; use Drupal\graphql\Plugin\SchemaExtensionPluginInterface; use Drupal\graphql\Plugin\SchemaExtensionPluginManager; // Contrib modules. use Drupal\graphql_core_schema\CoreComposableConfig; use Drupal\graphql_core_schema\CoreComposableResolver; use Drupal\graphql_core_schema\CoreSchemaExtensionInterface; use Drupal\graphql_core_schema\CoreSchemaInterfaceExtensionInterface; use Drupal\graphql_core_schema\EntitySchemaBuilder; use Drupal\graphql_core_schema\Form\CoreComposableSchemaFormHelper; use Drupal\graphql_core_schema\GraphQL\Enums\DrupalDateFormatEnum; use Drupal\graphql_core_schema\GraphQL\Enums\EntityTypeEnum; use Drupal\graphql_core_schema\GraphQL\Enums\LangcodeEnum; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderGenerator; use Drupal\graphql_core_schema\SchemaBuilder\SchemaBuilderRegistry; use Drupal\graphql_core_schema\TypeAwareSchemaExtensionInterface; // Core modules. use Drupal\typed_data\DataFetcherTrait; // Core libraries. use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Render\Element\Checkboxes; use Drupal\Core\TypedData\TypedDataTrait; // External libraries. use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\InterfaceTypeDefinitionNode; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\Parser; use GraphQL\Type\Schema; use GraphQL\Utils\BuildSchema; use GraphQL\Utils\SchemaExtender; use GraphQL\Utils\SchemaPrinter; use Symfony\Component\DependencyInjection\ContainerInterface;
Now, to me that's very readable when I need to review or find a use statements - compared to trying to find it alphabetically. I may not even remember the exact fully qualified name, but I know that what I'm looking for is a module or an external library so my eye immediately goes where it needs to go.
Within each group, I think case-insensitive alphabetical sorting makes more sense. For a human,
c
is the same letter asC
. The example in the description of this issue is not particularly useful for a human reader. Fortunately though, this mostly becomes a non-issue with the approach described above because in 99% of the cases modules are grouped separately than core and external libraries, and it's pretty much only Drupal modules that use lowercase.I have documented this approach here: https://github.com/krystalcode/drupal8-coding-standards
When it comes to PHPCS, I'll come back to my initial point. I frequently get developers updating a file where I have used this method, only to change the import statements back to an unreadable blob with the only objective to make PHPCS go green. We're missing the point here.
That said, I think it wouldn't be difficult to write a Sniff that:
- Allows single empty lines between use statements.
- Allows for comment lines between use statements.
- Recognizes groups of import statements using the empty lines as separators.
- Requires alphabetical sorting - but only within each group.
- Raises a warning if a group contains more than 10 import statements - configurable.
- 🇳🇿New Zealand quietone
As asked for by @klausi, here is the Coder issue to discuss reverting, 📌 Revert case sensitive use-statement sniff Active
- 🇩🇪Germany donquixote
@krystalcode (#103):
So, what is the purpose of sorting use statements?
For me, the main purpose of ordering anything, including imports, is to avoid noise and conflicts in git, and duplicated entries resulting from faulty merges. These problems can be avoided if there is one canonical order that is followed by all developers and tools.
- 🇵🇪Peru krystalcode
The way Git merges changes does not have much to do - if anything - with the sorting of the contents of the code. Conflicts most commonly happen when you edit the same lines.
5. use Drupal\address\AddressInterface; 6. use Drupal\node\NodeInterface; 7. use Drupal\user\UserInterface;
Branch A adds:
5. use Drupal\address\AddressInterface; + 6. use Drupal\group\GroupMembershipLoaderInterface; 7. use Drupal\node\NodeInterface; 8. use Drupal\user\UserInterface;
Branch B adds:
5. use Drupal\address\AddressInterface; + 6. use Drupal\commerce\ConfigurableFieldManagerInterface; 7. use Drupal\node\NodeInterface; 8. use Drupal\user\UserInterface;
The result:
use Drupal\address\AddressInterface; <<<<<<< Branch A use Drupal\group\GroupMembershipLoaderInterface; ======= use Drupal\commerce\ConfigurableFieldManagerInterface; >>>>>>> Branch B use Drupal\node\NodeInterface; use Drupal\user\UserInterface;
Both changes placed the import statements where they should and followed the correct alphabetical sorting. Note that I actually tested this to make sure I'm not saying anything inaccurate and got the behavior above.
- 🇳🇿New Zealand jweowu
The git commit noise and conflicts are a result of multiple different users/IDEs applying different sorting rules, and re-arranging the order of lines which were previously sorted by a different program. That sort of noise can ping-pong back and forth if multiple developers are working on the same file (not just the same part of the file), because the list of Use statements at the start of the file is for the whole file. Enforcing a standard, well-defined, stable sort order ensures that this kind of noise won't happen (or at least won't ever be merged).
- 🇵🇪Peru krystalcode
Ok got it, thanks for the explanation. I'm not sure really how common this problem is, but I guess I'm old school and prefer to manually craft code. I also understand the "lazy developer" syndrome where people want to focus on the actual logic that matters most and have their text editor do the boring stuff for them - not saying this in a demeaning way, I appreciate the practicality of such attitude. I write scripts to automate things for me all the time.
In that case I think it should be a suggestion and not phrased with MUST in the standards, there could be a default Sniff in Coder but also allowing for alternatives letting the maintainer of the code make a different choice.
- Status changed to Active
2 months ago 8:07am 23 January 2025 - 🇳🇿New Zealand quietone
It isn't really feasible to implement this because of the way PHPStorm does sorting. If one used the 'Sort lines' menu item then the lines are sorted with case sensitivity whereas when use statements are automatically inserted they are without case sensitivity.
The issue for this is Automatically added use statements should use PHP natural sort order which is 10 years old.