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

Fix deprecation warnings #1746

Merged
merged 18 commits into from
Oct 29, 2024
Merged

Fix deprecation warnings #1746

merged 18 commits into from
Oct 29, 2024

Conversation

antoscarface
Copy link
Contributor

The PR aims to solve some deprecation warnings complained by some users. Here the most important ones:

Deprecated: Wordlift\Modules\Common\Symfony\Component\Config\Resource\ComposerResource implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /var/www/html/wp-content/plugins/wordlift/modules/common/third-party/vendor/symfony/config/Resource/ComposerResource.php on line 18
  |  
  | Deprecated: Wordlift\Modules\Common\Symfony\Component\Config\Resource\FileResource implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /var/www/html/wp-content/plugins/wordlift/modules/common/third-party/vendor/symfony/config/Resource/FileResource.php on line 20
  |  
  | Deprecated: Return type of Wordlift\Content\Wordpress\Wordpress_Content_Id::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /var/www/html/wp-content/plugins/wordlift/classes/content/wordpress/class-wordpress-content-id.php on line 63

The first twos come from an oldest Symfony libraries which needs to be updated, inside modules/common, which requires to have a min PHP req to at least 7.4+ in order to easily upgrade the library without putting checks about PHP version because of it needs different code to load base on server PHP version.

Also, I had to fix the bootstrap of PHP scoper autoload in order to include also the "autoload files" which are not supported by PHP scoper. This to solve some missing "native php functions" included by the polyfill libraries of symfony, which are required from the upgraded version of symfony dependencies.

In the PR are also fixes some other minor warnings.

The large edits in the PR comes from the dependencies update.

I tried to keep the commits speaking in order to go though them and undestand the flow of changes.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope sorry I didn't noticed that, I gonna check for upgrading it instead of editing it. Do I just need to upgrade the package and replace the entire library manually correct? I'm not seeing a composer here to manage the dependency, if I'm not wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Correct, if I remember correctly, it is just a drop-in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved into a new commit which upgrade the library: 282e985

@@ -10,12 +10,12 @@
* file that was distributed with this source code.
*/

namespace Wordlift_Modules_Common_Composer\Autoload;
namespace Composer\Autoload;
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.


/**
* ClassLoader implements a PSR-0, PSR-4 and classmap class loader.
*
* $loader = new \Wordlift_Modules_Common_Composer\Autoload\ClassLoader();
* $loader = new \Composer\Autoload\ClassLoader();
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

@@ -8,13 +8,13 @@ class ComposerAutoloaderInit8bd2b6d4ccd6b50e0809e5ba62169420

public static function loadClassLoader($class)
{
if ('Wordlift_Modules_Common_Composer\Autoload\ClassLoader' === $class) {
if ('Composer\Autoload\ClassLoader' === $class) {
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

require __DIR__ . '/ClassLoader.php';
}
}

/**
* @return \Wordlift_Modules_Common_Composer\Autoload\ClassLoader
* @return \Composer\Autoload\ClassLoader
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

@@ -23,11 +23,11 @@ public static function getLoader()
}

spl_autoload_register(array('ComposerAutoloaderInit8bd2b6d4ccd6b50e0809e5ba62169420', 'loadClassLoader'), true, true);
self::$loader = $loader = new \Wordlift_Modules_Common_Composer\Autoload\ClassLoader(\dirname(__DIR__));
self::$loader = $loader = new \Composer\Autoload\ClassLoader(\dirname(__DIR__));
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

spl_autoload_unregister(array('ComposerAutoloaderInit8bd2b6d4ccd6b50e0809e5ba62169420', 'loadClassLoader'));

require __DIR__ . '/autoload_static.php';
call_user_func(\Wordlift_Modules_Common_Composer\Autoload\ComposerStaticInit8bd2b6d4ccd6b50e0809e5ba62169420::getInitializer($loader));
call_user_func(\Composer\Autoload\ComposerStaticInit8bd2b6d4ccd6b50e0809e5ba62169420::getInitializer($loader));
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

@@ -2,7 +2,7 @@

// autoload_static.php @generated by Composer

namespace Wordlift_Modules_Common_Composer\Autoload;
namespace Composer\Autoload;
Copy link
Member

Choose a reason for hiding this comment

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

We need a custom scoped namespace or it can clash with other libraries.

@antoscarface
Copy link
Contributor Author

@ziodave I fixed your comments (replied in one and for the others I made a fixup )

I've added also other fixes for other deprecation warnings I'm noticing it.
I don't see any other warnings at the moment.

= 3.8.2 - 2024-09-12 =
* Add missing parameter to the `pre_as_enqueue_async_action` hook.
* Bump minimum PHP version to 7.0.
* Bump minimum WordPress version to 6.4.
Copy link
Member

Choose a reason for hiding this comment

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

I think we try to be compatible with WP 5.8+ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, used the version which fixed the warnings (just one minor up, 3.5.4 -> 3.5.3), so no changes to wp min req

Copy link

sonarcloud bot commented Oct 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@ziodave ziodave merged commit 66f7084 into insideout10:main Oct 29, 2024
30 of 55 checks passed
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