[5.1] Updating php-cs-fixer and php_codesniffer to latest versions

[Hackwar]

Summary of Changes

This updates php-cs-fixer and php_codesniffer to the latest versions respectively. I'd like to do this now since that removes a deprecated package.

Testing Instructions

Everything should behave like before.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

[richard67]

@Hackwar The composer.lock shows much more being updated than only what the description of this PR says. Is that the result of a plain composer update statement? This means you update all dependencies. Or have you explicitly only made a composer update friendsofphp/php-cs-fixer?

[Hackwar]

I did a composer update friendsofphp/php-cs-fixer --with-all-dependencies, since otherwise it doesn't update. That is also the reason why this is aimed at 5.1-dev and not 5.0-dev.

[Hackwar]

I merged the necessary codestyle changes into this PR as well. No need to keep 2 PRs open for this...

[richard67]

@Hackwar @brianteeman 's comment in the other, closed PR is still valid for this PR here, and I agree with him: #42604 (comment) .

For "elseif" or "case" statements, the comments are before the corresponding "elseif" or case". The CS fixer now intends them by 4 more spaces because it assumes that these comments are referring to the actual "if" or "elseif" or "case" and not the one starting in the line below.

E.g. for "elseif" in file administrator/components/com_categories/src/Model/CategoriesModel.php in line 267 and 268:

            // Case: Using only the by level filter
        } elseif ($level) {

Or for the "case" in file administrator/components/com_finder/src/Indexer/Adapter.php in lines 929 and 930:

                // Archived items should only show up when option is enabled
            case 2:
                if ($this->params->get('search_archived', 1) == 0) {

It would be better readable if we would move these comments to the line below where they belong to, e.g. for "elseif" in file administrator/components/com_categories/src/Model/CategoriesModel.php:

        } elseif ($level) {
            // Case: Using only the by level filter

Or for the "case" in file administrator/components/com_finder/src/Indexer/Adapter.php:

            case 2:
                // Archived items should only show up when option is enabled
                if ($this->params->get('search_archived', 1) == 0) {

That's just a suggestion, I would not insist on it, and I know it requires some manual work.

But I think the better readability would make it be worth the work.

[Quy]

Fix phpcs https://ci.joomla.org/joomla/joomla-cms/72832/1/7

[LadySolveig]

PR phpcs fix

PR php-cs-fixer configuration update - deprecated rule
I can also do a completely separate PR for this, but I actually think it's part of the update.

[Quy]

I have tested this item ✅ successfully on 9760312

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42603.}

[wilsonge]

Code review looks fine. I think if either of the symfony upgrades did anything we'd have bigger problems plus system tests would pick it up as they're much deeper libraries

[laoneo]

Just a not here, I did always do the composer update and then composer bump. Like that are the versions in the composer.json also updated.
As CS Fixer released 3.47.1 yesterday, you can update it already or we can do it then in beta.

[LadySolveig]

Thanks @laoneo for the new version hint for CS Fixer - updated.

[brianteeman]

Not addressing #42603 (comment) ??

[richard67]

Not addressing #42603 (comment) ??

It was partly addressed. Some "case" statements have been fixed, but none of the "elseifs" or "else".

[richard67]

@LadySolveig Since your last commit, this PR has conflicts. And the issue which I had mentioned with my big comment has been addressed only partly, see my previous comment.

[LadySolveig]

@richard it is not very clear to me why you did not address this when @Razzo1987 contacted you directly when he has done the manually clean up here 620d3e0

could you please clean up the the remaining comments for the elseif and else yourself @Hackwar

[richard67]

@richard it is not very clear to me why you did not address this when @Razzo1987 contacted you directly when he has done the manually clean up here 620d3e0

@LadySolveig Well I saw it was dealing only with the “case” statements so I assumed the other things will be done later. My first comment was very clear, I think.

[brianteeman]

Unfortunately doing it directly without a pr (not good practice) has already been partially undone by subsequent pull requests eg #42667

[LadySolveig]

I am not sure if I have understood you correctly, but I resolved the conflicts yesterday and made sure that the fixes for the comments remain as @Razzo1987 has already fixed them for us. @brianteeman have I overlooked something here somewhere?
Sorry, I see what you mean, that was my mistake that I pushed it directly

@richard67 I had the impression that we agreed here in the discussion with the maintainers that it would be easier to fix this directly here in the PR. But if the opinion has changed here I will merge and we will fix the rest later as you suggested in your comment.

[richard67]

@LadySolveig Sorry in case if I’ve caused some confusion. Yes, it would be easier to fix it now, but we can do that also later. I don’t insist in my suggestions and am ok with merging the PR.

[HLeithner]

rest looks ok by code review

[Hackwar]

In the meantime, both codestyle packages have released new versions...

[LadySolveig]

Thank you !