Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds typehints compatibility #20

Merged
merged 3 commits into from
Apr 25, 2019
Merged

Adds typehints compatibility #20

merged 3 commits into from
Apr 25, 2019

Conversation

michalbundyra
Copy link
Member

@michalbundyra michalbundyra commented Apr 25, 2019

User code and 3rd party libraries still use legacy typehints. If we try to load new class and corresponding legacy class was not loaded before then we can get error of typehints incompatibility. On loading new class we need then create alias to legacy class so PHP interpreter thinks that it's the same class and we can have seamless migration.

TODO:

  • Add reverse mapping for all namespaces
  • Change autoloader to use above reverse mapping
  • Add tests for all cases (reverse mapping)

User code and 3rd party libraries still use legacy typehints.
If we try to load new class and corresponding legacy class was not
loaded before then we can get error of typehints incompatibility.
On loading new class we need then create alias to legacy class
so PHP interpreter thinks that it's the same class and we can
have seamless migration.
\Composer\Autoload\includeFile($file);
class_alias($class, 'Zend\\' . $class, false);
}
} elseif (strpos($class, 'Laminas\\') === 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine the statements, and return early:

if (strpos($class, 'Expressive\\') === 0
    && ($file = $classLoader->findFile($class))
) {
    \Composer\Autoload\includeFile($file);
    class_alias($class, 'Zend\\' . $class, false);
    return;
}

// next condition here

I'd also import the Composer\Autoload\includeFile function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weierophinney

Combine the statements, and return early

Actually it's not gonna help, because now we have elseif, and if we combine ifs then condition from elseif is going to be checked in case strpos($class, 'Expressive\') is satisfied but the file is not found.

This whole part is going to be changed anyway - as we need to have reverse mapping, then we will do the same as in append preloader - split class name into segments and try to find quicker the legacy name.

I'd also import the Composer\Autoload\includeFile function.

Unfortunately, we cannot. As importing function is available form PHP 5.6. We've decided to support PHP 5.3 here.

@michalbundyra
Copy link
Member Author

@weierophinney This PR is now ready to go!

@weierophinney weierophinney merged commit f329abe into laminas:master Apr 25, 2019
weierophinney added a commit that referenced this pull request Apr 25, 2019
@michalbundyra michalbundyra deleted the feature/legacy-type-hints branch April 25, 2019 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants