TASK: Optimise php imports in Neos 9 code

[mhsdesign]

Related: #4580

Phpstorm supports to optimise imports via Optimise Imports [ctrl+alt+O]
This will not only remove outdated imports but also sort them.

In the past when removing a usage i deleted the line manually without sorting to prevent merge conflicts,
but i would like to use the key stroke generally and know that others do as well.

So with care that we do want to upmerge things from 8.3 i ran the import optimisation across the whole repository but only committed newer neos 9 code.

New neos code lies in Neos.ContentRepository*, Neos.ContentGraph*, Neos.TimeableNodeVisibility and partially in selected subdirectories and files of Neos.Neos (my strategy was if the file was already touched for a neos 9 refactoring (e.g. we import the contentRepository) its fine to cleanup here).

After this change optimising imports can be carried out safely (at least in new code) as the imports are now tidied.
This will prevent needless mergeconflicts across prs in the future and should improve our workflow.

Upgrade instructions

Review instructions

This change should not make upmerging harder than before as the classes in Neos.ContentRepository* didnt exist in 8.3 and the few classes modified in Neos.Neos have been adjusted in general for Neos 9.

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[kdambekalns]

👍 by 👀

[mhsdesign]

Hmm I just had a change of mind. To be kind to our existing Mammut prs ill delay this cosmetic cleanup as it can be run at any point automatically ;)

[mhsdesign]

Thanks for your reviews though ;)

[mhsdesign]

With all the elephants merged a change like this can now be more safely carried out ;) (we wont hurt any other big pr)

[dlubitz]

Works for me, but ordering imports by name is useless if we don't have any rule to keep this also in future.

[mhsdesign]

Yeah sure without automatic linting we cannot guarantee that this will be kept. But for those of us using phpstorm they can enforce this pattern. And having this as a headstart is quite good i think (the escr code was created over years by multiple people, its only natural that we have more than 160 outdated imports in the files). Also for linting we would have exclude all the packages that were deliberately not cleaned up as they would cause unnecessary conflicts in up-merges like Neos.Fusion or Neos.Media and friends.

Fyi im using the default phpstorm import sort settings (alphabetically):

[bwaidelich]

Shall we add https://cs.symfony.com/doc/rules/import/ordered_imports.html ?

[mhsdesign]

Hmm seeing that we have already php cs (im not using it locally really ...) thats not a bad idea.
And regarding import ordering in Neos.Fusion or Neos.Media and friends, i can very well imagine creating a pr against 8.3 and upmerge these changes ;)

[bwaidelich]

Hmm seeing that we have already php cs (im not using it locally really ...) thats not a bad idea.

Especially because it is enforced by our CI (composer lint)

Related: #4580

[mhsdesign]

@bwaidelich i stand completely on the schlauch i dont know how to set this up and it might be that i confused php cs and the fixer 😅

in the end we want a linter and not a fixer, or at least first a linter and then a fixer on top. But i couldnt find a sniff in php code sniffer that does this. I tried extending your new xml somehow but there seems to sniff?
There is only src/Standards/PSR12/Sniffs/Files/ImportStatementSniff.php which doesnt check usages nor sort.

[bwaidelich]

PHP_CodeSniffer is pretty confusing.. In our case:
composer lint:phpcs runs phpcs, that complaints about linting errors (and we run that in our CI).
composer lint:phpcs:fix was added for convenience. It runs phpcbf, that automatically fixes most of the linter errors.

And then there is PHP-CS-Fixer that is a different, unrelated, tool that also provides a linter & fixer. But we don't use that (yet).

Unfortunately, detecting unused import statements is not yet available for PHPCS and seems to be a very complex task.
For ordering imports there are 3rd party "sniffs", e.g. : https://github.com/slevomat/coding-standard/blob/master/doc/namespaces.md#slevomatcodingstandardnamespacesalphabeticallysorteduses-

[bwaidelich]

A little PHP-CS-Fixer experiment: #4949

[bwaidelich]

@mhsdesign I ran my experiment with your PR and it reduces the violating files from 237 to 29. But after applying the fixer, I still get a diff:
changes.patch

edit: ..which is to be expected, those are just the remaining use orderings AFAIS. I created a PR with the composer lint:fix change applied: #4950

[mhsdesign]

Wow thanks @bwaidelich for explaining. it really seems that they choose unwise namings ^^

I would propose to merge this now, and create a separate pr to fix violations in 8.3

Also linting in ci seems more complex and not a one liner so let’s merge this first and keep #4949 as a follow-up

That way we can continue with other mamut prs right now (e.g. node identity)