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

zend-loader and autoloader overhaul - cont. #116

Merged
merged 3 commits into from
Oct 4, 2022

Conversation

falkenhawk
Copy link
Member

@falkenhawk falkenhawk commented Oct 18, 2021

continued (apparently unfinished) #1 / 76477fb

  • introduce Zend_Loader::tryLoadClass() to attempt loading a class but do not throw an exception when file is not found (but still throw when required class in the existing file is not defined)
  • did not go for suppressing loading classes with @ suppressor by default, since regular warnings/errors coming from a loaded file should be visible, otherwise it might be very confusing for devs
  • introduce Zend_Loader_Exception_FileNotFoundException and Zend_Loader_Exception_ClassNotFoundException
    • instead of throwing Zend_Exception in Zend_Loader::loadClass() with a generic message File \"$file\" does not exist or class \"$class\" was not found in the file
    • Zend_Loader_Exception_FileNotFoundException will be thrown with message File "$file" could not be found within configured include_path. or
    • Zend_Loader_Exception_ClassNotFoundException with message Class "$class" was not found in the file "$file"., in their respective cases.
  • since composer autoloader takes precedence and Zend_Autoloader / Zend_Loader is only an extra helper, when necessary (when still needed and configured on legacy projects) - Zend_Loader should not emit warnings when checking for files, by default. Added isReadable check inside loadFile before include/include_once. There might be a performance hit, but it should be okay when most of the autoloading is handled by composer autoloader anyway.
  • use stream_resolve_include_path() in isReadable() - it should be more performant and the function is available since php 5.3.2
  • Zend_Loader::loadClass() returns boolean now
  • adjust combinations of class_exists + Zend_Loader::loadClass() in remaining affected places (i.a. Translate or File_Transfer or Filter adapters)
  • unregister autoloader from spl_autoload when Zend_Loader_Autoloader::resetInstance() is called
  • added more tests for Zend_Loader and Zend_Autoloader
  • added tests for loading custom Zend_Translate adapter while having Zend_Autoloader enabled
  • adjusted existing tests which expected "file not found" php warning to be triggered by Zend_Loader

should fix #46, replaces #96 and #20
Also possibly introduces a breaking change in how Zend_Loader works, so will probably release as v1.15

@falkenhawk falkenhawk force-pushed the loader-overhauled-again branch 6 times, most recently from eceee15 to c5262a0 Compare October 18, 2021 21:20
- introduce `Zend_Loader::tryLoadClass()` to attempt loading a class but do not throw an exception when file is not found (but still throw when required class in the existing file is not defined)
- did not go for suppressing loading classes with `@` suppressor by default, since regular warnings/errors coming from a loaded file should be visible, otherwise it might be very confusing for devs
- introduce `Zend_Loader_Exception_FileNotFoundException` and `Zend_Loader_Exception_ClassNotFoundException`
  - instead of throwing `Zend_Exception` in `Zend_Loader::loadClass()` with a generic message `File \"$file\" does not exist or class \"$class\" was not found in the file`
  - `Zend_Loader_Exception_FileNotFoundException` will be thrown with message `File "$file" could not be found within configured include_path.` or
  - `Zend_Loader_Exception_ClassNotFoundException` with message `Class "$class" was not found in the file "$file".`, in their respective cases.
- since composer autoloader takes precedence and `Zend_Autoloader` / `Zend_Loader` is only an extra helper, when necessary (when still needed and configured on legacy projects) - `Zend_Loader` should not emit warnings when checking for files, by default. Added `isReadable` check inside `loadFile` before `include`/`include_once`. There might be a performance hit, but it should be okay when most of the autoloading is handled by composer autoloader anyway.
- Zend_Loader::loadClass returns boolean now
- adjust combinations of `class_exists` + `Zend_Loader::loadClass` in remaining affected places (i.a. Translate or File_Transfer or Filter adapters)
- unregister autoloader from spl_autoload when `Zend_Loader_Autoloader::resetInstance()` is called
- added more tests for Zend_Loader and Zend_Autoloader
- added tests for loading custom Zend_Translate adapter while having Zend_Autoloader enabled
- adjusted existing tests which expected "file not found" php warning to be triggered by `Zend_Loader`

ref: 76477fb
fixes the test failure on php 7.4 & 8.0:
- Failed asserting that PHPUnit_Framework_Error_Notice Object (...) is an instance of class "Zend_Gdata_App_Exception".
@falkenhawk
Copy link
Member Author

@glensc Could you take a look, please?

cc @g10-fred @saru2048 @Dringho

in isReadable() - it should be more performant and the function is available since php 5.3.2
@glensc
Copy link
Contributor

glensc commented Oct 19, 2021

@falkenhawk nothing seemingly wrong with the changes, so I'll say godspeed with testing! :)

@falkenhawk falkenhawk self-assigned this Jul 4, 2022
@glensc
Copy link
Contributor

glensc commented Sep 20, 2022

Since I don't use zend-loader myself, I can't comment on this.

@falkenhawk
Copy link
Member Author

falkenhawk commented Oct 4, 2022

Internal tests on existing projects were successful. Unfortunately our usage does not cover all possible scenarios for zend-autoloader/zend-loader/composer autoloader combinations, so we are able to speak only for ourselves.

Unfortunately, we haven't heard anything from other users, who might be interested in this if this PR fixes their cases - for almost a year.

I will merge it now and release 1.15.0 together with other changes.

@glensc
Copy link
Contributor

glensc commented Oct 4, 2022

if this has issues, they can rollback to 1.14.0 and report the issue, there aren't many other changes in 1.15.0.

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.

Zend_Translate issues 'failed to open stream' warning when using a custom adapter
3 participants