Do not assert a namespace separator when reporting a missing mapping file

[dchaudhari7177]

Fixes #520. Targets 3.4.x, since the assertion is on every maintained branch (3.4.x, 4.2.x, 4.3.x, 5.0.x) and merges up.

@derrabus's reading in the issue is right: $pos exists only to render the exception message, so there is nothing there to assert.

$pos = strrpos($className, '\');
assert(is_int($pos));

throw MappingException::mappingFileNotFound(
    $className,
    substr($className, $pos + 1) . $this->fileExtension
);

A class in the root namespace has no separator, strrpos() returns false, and with zend.assertions on the caller gets an AssertionError from the line that was trying to build a MappingException. So the failure replaces the error you were meant to see with a different one, from one line above it.

The special case is that such a class is its own short name — DateTime is DateTime — which is a perfectly good thing to name in the message:

($pos === false ? $className : substr($className, $pos + 1)) . $this->fileExtension

use function assert; and use function is_int; go with it; they were only there for that line, and phpcs flags them otherwise.

Verified

Red before green, with the production file stashed and the new test left in place:

$ php -d zend.assertions=1 vendor/bin/phpunit --filter FindMappingFileNotFoundForClassInRootNamespace
1) …SymfonyFileLocatorTest::testFindMappingFileNotFoundForClassInRootNamespace
Failed asserting that exception of type "AssertionError" matches expected exception
"Doctrine\Persistence\Mapping\MappingException".
Message was: "assert(is_int($pos))" at src/Mapping/Driver/SymfonyFileLocator.php:247

That is the reported bug reproduced exactly. With the fix:

$ php -d zend.assertions=1 vendor/bin/phpunit
OK (138 tests, 262 assertions)

$ vendor/bin/phpcs
$ vendor/bin/phpstan analyse --memory-limit=1G
 [OK] No errors

The new test asserts the message the caller should have been getting all along:

No mapping file found named 'DateTime.yml' for class 'DateTime'.

Note

The reproduction in the issue reaches findMappingFile() for DateTime in the first place, which is arguably its own question — but that path throws MappingException for every other class name, and it should for this one too rather than dying one line earlier with a different error class.

[greg0ire]

Hang on… this is a duplicate of #521

[GromNaN]

Closing as duplicate.