-
Notifications
You must be signed in to change notification settings - Fork 26
Try to fix broken tests #349
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
base: master
Are you sure you want to change the base?
Conversation
…h is needed since composer version 2.9.x, especially for older Magento versions since those have dependencies with known security vulnerabilities.
|
I'd be fine removing the checks for older Magento versions. What do you think? |
|
Sure, it's going to be some extra small maintenance to keep them up2date every year when Magento drops support for a certain version, but I guess that's fine. As for the phpstan failures it now complains about, I can fix half of them with these changes: diff --git a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php
index b97845f..859f69f 100644
--- a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php
+++ b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php
@@ -61,6 +61,8 @@ class ExtensionAutoloader implements Autoloader
return;
}
+ assert($class !== '');
+
// fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
// local classes in your project. We need to check first if classes exists locally before generating them!
$pathToLocalClass = $this->classLoaderProvider->findFile($class);
diff --git a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php
index e619323..16dda76 100644
--- a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php
+++ b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php
@@ -60,6 +60,8 @@ class ExtensionInterfaceAutoloader implements Autoloader
return;
}
+ assert($interfaceName !== '');
+
// fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
// local classes in your project. We need to check first if classes exists locally before generating them!
$pathToLocalInterface = $this->classLoaderProvider->findFile($interfaceName);
diff --git a/src/bitExpert/PHPStan/Magento/Autoload/FactoryAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/FactoryAutoloader.php
index 51630c2..0df926f 100644
--- a/src/bitExpert/PHPStan/Magento/Autoload/FactoryAutoloader.php
+++ b/src/bitExpert/PHPStan/Magento/Autoload/FactoryAutoloader.php
@@ -44,6 +44,8 @@ class FactoryAutoloader implements Autoloader
return;
}
+ assert($class !== '');
+
// fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
// local classes in your project. We need to check first if classes exists locally before generating them!
$pathToLocalClass = $this->classLoaderProvider->findFile($class);
diff --git a/src/bitExpert/PHPStan/Magento/Autoload/ProxyAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/ProxyAutoloader.php
index c4f0131..038c7f7 100644
--- a/src/bitExpert/PHPStan/Magento/Autoload/ProxyAutoloader.php
+++ b/src/bitExpert/PHPStan/Magento/Autoload/ProxyAutoloader.php
@@ -44,6 +44,8 @@ class ProxyAutoloader implements Autoloader
return;
}
+ assert($class !== '');
+
// fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
// local classes in your project. We need to check first if classes exists locally before generating them!
$pathToLocalClass = $this->classLoaderProvider->findFile($class);Do I include them in this PR, or rather in a separate one? These are needed after phpstan/phpstan-src@9ab8480 happened. The reason I'm using We could alternatively also just ignore those errors in the |
|
Yeah, add the assert checks in this PR, I am fine with that. Using assert() is fine, at least the why is documented here. Maybe open a bug report for PHPStan, the error feels wrong to me, but maybe Ondřej will disagree :) I am quite busy this and the next week, so it will take me some time to merge and test. |
|
And as always, your help in improving the extension is very much welcome! |
…ecurity patch versions, and also switch to PHP 8.2
|
@shochdoerfer, I've dropped Magento 2.4.4/2.4.5 and added 2.4.7/2.4.8 instead. I still have some failures in phpstan, but I don't know how to solve them, can you take a look when you find some time? |
First attempt was to add
--no-security-blockingflag to the composer update commands. But that may not be the best idea, not sure. It was removed again by now.Rest of decisions here can be found in the later comments in the thread.