search

ampproject / amp-toolbox-php

AMP Optimizer PHP library
Apache License 2.0
73 stars 25 forks source link

Remove the override iterationArray property from the inherited classes #504

Closed ediamin closed 2 years ago

ediamin commented 2 years ago

Fixes #503

Currently in PHP 5.6 we get errors in some validator spec classes due to override the property $iterationArray used in the trait AmpProject\Validator\Spec\Iteration. This PR fixes this issue by removing the override property from the inherited classes.

ediamin commented 2 years ago

In addition to the fix the error related to iterationArray, I've added the missing constants that are generated by the spec generator and also fix some PHP 8.1 compatibility issues.

schlessera commented 2 years ago

@ediamin Do you know why this was not caught by our PHP 5.6 tests? Can you try to add a test that triggers this issue on PHP 5.6 without the fix?

ediamin commented 2 years ago

@schlessera I've found out that the shivammathur/setup-php@v2 action uses error_reporting = E_ALL & ~E_DEPRECATED & ~E_STRICT and more specifically by ignoring the E_STRICT errors we don't see the issue with the $iterationArray. There is a reported issue in this action repo already. Further I've done testing using error_reporting=E_ALL in the test.yml. With this settings the tests for PHP 5.6 do not pass.

Should I update the ini-values: pcov.directory=., error_reporting=E_ALL for shivammathur/setup-php@v2 action in tests.yml?

schlessera commented 2 years ago

Yes, please. We want to catch all possible issues in our automated test suite.

ediamin commented 2 years ago

Updated the tests.yml with ini-values: pcov.directory=., error_reporting=E_ALL