Add an option to skip importing root namespace classes (like \DateTime)

[gnutix]

Trying to fix #1911 (comment)

services:
    Rector\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector:
        $shouldImportRootNamespaceClasses: false

I still need to prevent changing DocBlock annotations like @var \DateTime... which seems way harder, looking at the code.

[TomasVotruba]

Great pick!

Definitely start with a test with optional parameter to be on

[gnutix]

Test added.

But some help to get the condition $node is NonRootNamespace right would be more than welcome. Currently my condition broke the tests in packages/CodingStyle/tests/Rector/Namespace_/ImportFullyQualifiedNamesRector/NonNamespacedTest.php.

And I'm not sure I'll manage to get the DocBlock right either, as the code is quite complex in there (DocBlockManipulator, DocBlockNameImporter, PhpDocNodeTraverser, etc); I wouldn't know how/where to pass that new option around and where to use it exactly so I wouldn't break half the project.

(and I don't mean that as a way of saying : "please do it all". I'd love to get to learn some of these internals, but some initial guidance might be welcome to find my way around)

[TomasVotruba]

But some help to get the condition $node is NonRootNamespace right would be more than welcome.

From top of my head, I'd detect \DateTime like:

$name = $this->getName($node);

if (Strings::startsWith($name, '\\')) {
   if (substr_count($name, '\\') === 1) {
        // skip
	    return null;
   }
}

---

As for docblock, I'm sure there will be another trick, let's deal with them later.

[gnutix]

Here's a working implementation. I've not handled DocBlock though, so the test is still failing. Shall you take over from here? :) (I've invited you as a collaborator on my fork, so you can work from there directly if you'd like)

[TomasVotruba]

As for doblocks, this might be place to add very same check:
https://github.com/rectorphp/rector/blob/master/packages/NodeTypeResolver/src/PhpDoc/NodeAnalyzer/DocBlockNameImporter.php#L125

The parameter should be passed via importNames() method as 3rd argument

[gnutix]

I've finished the implementation. 👍 Would there be some documentation to write for this new option ?

[TomasVotruba]

Feature looks ready, good job 👍

Would there be some documentation to write for this new option ?

The documentation is generated from code samples, so adding one ConfiguredCodeSample() in getDefinition() will be enough

[TomasVotruba]

I've finished the implementation. +1 Would there be some documentation to write for this new option ?

Feel free to send that in next PR. I'm merging this, since feature is complete

[gnutix]

Not sure how I can describe the new parameter within new CodeSample(...) as this only provides before/after code examples - but not the constructor parameters involved to achieve such results.

[TomasVotruba]

ConfiguredCodeSample

[gnutix]

Sorry, I should have read more attentively! 🙄

[TomasVotruba]

No troubles 👍

[gnutix]

Note : this option was changed into parameters.import_short_classes: false

[TomasVotruba]

Could you add that to README as well?

[Aerendir]

@TomasVotruba I documented it