Skip to content

Conversation

@greg0ire
Copy link
Member

@greg0ire greg0ire commented May 21, 2024

It was changed in 3e2fad5, I am not sure why, I think it might be to avoid the extra conditions / complexity I'm adding here. What I know is that it makes a guard condition in FileDriver::initialize() useless, most likely resulting in persistence looking for a file with a name consisting only in an extension (.php, .yml…).

protected function initialize()
{
$this->classCache = [];
if ($this->globalBasename === null) {
return;
}
foreach ($this->locator->getPaths() as $path) {
$file = $path . '/' . $this->globalBasename . $this->locator->getFileExtension();
if (! is_file($file)) {
continue;
}
$this->classCache = array_merge(
$this->classCache,
$this->loadMappingFile($file)
);
}
}

@greg0ire greg0ire requested review from SenseException, derrabus and stof and removed request for SenseException, derrabus and stof May 21, 2024 12:54
@greg0ire greg0ire marked this pull request as draft May 21, 2024 16:07
@greg0ire

This comment was marked as resolved.

Since 3e2fad5, the zero-value for
globalBasename is the empty string. It is no longer possible to have
null in here.
@greg0ire greg0ire force-pushed the restore-initial-default branch from e696041 to 1c50c3d Compare May 21, 2024 20:55
@greg0ire greg0ire changed the title Restore null default for globalBasename Handle the correct zero-value May 21, 2024
@greg0ire greg0ire marked this pull request as ready for review May 21, 2024 20:56
@greg0ire greg0ire added the Bug Something isn't working label May 21, 2024
@derrabus
Copy link
Member

Can we produce a test that fails without your change?

@greg0ire
Copy link
Member Author

greg0ire commented May 21, 2024

Can we produce a test that fails without your change?

Not a very meaningful one I'm afraid. It would assert that a file named literally .php doesn't get loaded even if it exists. I don't think such a scenario should be tested, and I think this guard condition is here for performance only (it spares a call to is_file()).

@derrabus derrabus added this to the 3.3.3 milestone May 22, 2024
@derrabus derrabus merged commit bf7aab0 into doctrine:3.3.x May 22, 2024
@greg0ire greg0ire deleted the restore-initial-default branch May 22, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants