RootNamespaceNormalizer: empty namespace is null, not ''

[dg]

Otherwise it prepends leading \ to class references inside namespace.

[aik099]

Was changed code covered by tests? I wonder why none of tests failed without your change.

[lisachenko]

I'm not sure, that top-level namespace should be null. Actually, I want to make it \, but we can discuss this.

I like strong typing, so if talk about namespace name, it can be either empty string or \, but not null, because it will violate the return type (in future)

[dg]

I think test was wrong, ie. test counts with leading \.

When you parse this by Nikic PHP Parser:

namespace { class X {} }

It will generate Namespace_(null, ...) and leading slash will never appear.

RootNamespaceNormalizer generates Namespace_(new FullyQualified(''), ... and then Name::concat() returns leading slashes.

[dg]

I like strong typing, so if talk about namespace name, it can be either empty string or , but not null, because it will violate the return type (in future)

Me too. It is not about empty string vs null but about object vs null.

[lisachenko]

Could you please add a broken test? Is it a big issue for you that class name is FQDN with leading slash? Because logically FQDN with leading slash is more correct way to specify a class name. But it can be my personal taste :)

[dg]

Leading slash or not is not problem, it can be matter of personal taste, the problem is inconsistency. PhpParser never returns leading slash, but RootNamespaceNormalizer makes inconsistency and when you parse this code

<?php 
class X extends Y
{}

some methods (I think Locator is among them) suddenly receives \Y instead of Y.

[lisachenko]

Could you check #67 (merged into the master branch). Looks like it is related to this issue too.

[lisachenko]

Unfortunately, there are many checks in the code that verifies that instance of name object is FullyQualified. If we replace it with null for the top-level namespace something can change and can result in broken BC for the external clients.

[lisachenko]

New version of php-parser declares top-level namespace with null, so let's proceed with that.

[scrutinizer-notifier]

The inspection completed: No new issues