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

PHP 8.1 #70

Closed
mmenavas opened this issue Feb 21, 2023 · 7 comments · May be fixed by #71
Closed

PHP 8.1 #70

mmenavas opened this issue Feb 21, 2023 · 7 comments · May be fixed by #71

Comments

@mmenavas
Copy link

mmenavas commented Feb 21, 2023

Overview

Given that PHP 8.0 EOL is on November 2023, I'd like to know if there's any ongoing effort to make this library compatible with php 8.1.

Below are the results of running PHPStan agains the src directory. Since I'm working on a project that uses this library, I'd be more than happy to help out with this effort.

 ------ ----------------------------------------------------------------------------------- 
  Line   Fields/BaseField.php                                                               
 ------ ----------------------------------------------------------------------------------- 
  :101   Unsafe usage of new static().                                                      
         💡 See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static  
  :118   Unsafe usage of new static().                                                      
         💡 See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static  
 ------ ----------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------- 
  Line   Schema.php                                                                         
 ------ ----------------------------------------------------------------------------------- 
  :76    Unsafe usage of new static().                                                      
         💡 See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static  
 ------ ----------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------- 
  Line   SchemaValidator.php                                                                         
 ------ -------------------------------------------------------------------------------------------- 
  :28    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$descriptor.  
  :29    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$errors.      
  :37    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$errors.      
  :52    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$errors.      
  :59    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$descriptor.  
  :78    Access to an undefined property frictionlessdata\tableschema\SchemaValidator::$descriptor.  
 ------ -------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------ 
  Line   Table.php                                                                                                                                                   
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------ 
  :57    Unsafe usage of new static().                                                                                                                               
         💡 See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static                                                                           
  :163   Return type mixed of method frictionlessdata\tableschema\Table::current() is not covariant with tentative return type mixed of method Iterator::current().  
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.                                                      
  :216   Return type mixed of method frictionlessdata\tableschema\Table::rewind() is not covariant with tentative return type void of method Iterator::rewind().     
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.                                                      
  :226   Return type mixed of method frictionlessdata\tableschema\Table::key() is not covariant with tentative return type mixed of method Iterator::key().          
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.                                                      
  :231   Return type mixed of method frictionlessdata\tableschema\Table::next() is not covariant with tentative return type void of method Iterator::next().         
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.                                                      
  :238   Return type mixed of method frictionlessdata\tableschema\Table::valid() is not covariant with tentative return type bool of method Iterator::valid().       
         💡 Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.                                                      
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------ 

EDIT: Adding deprecated warnings issued by PHP 8.1:

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 163

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 231

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::key() should either be compatible with Iterator::key(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 226

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::valid() should either be compatible with Iterator::valid(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 238

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::rewind() should either be compatible with Iterator::rewind(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 216


Please preserve this line to notify @courtney-miles (lead of this repository)

@OriHoch
Copy link
Collaborator

OriHoch commented Feb 21, 2023

Hi, sounds good, I think a PR would be very welcome, let me know if you need any help

@DiegoPino
Copy link

DiegoPino commented Feb 21, 2023 via email

@courtney-miles
Copy link
Collaborator

We do use the Library on PHP 8.1 without errors

Admittedly, the known compatibility with PHP gets a little hairy because its version constraint is >=7.1.0.

We run the tests for all versions up to 8.0, but after that compatibility is an unknown.

As a part of ensuring compatibility with PHP 8.1, would we be happy to peg the upper version of PHP in composer.json so it can't be used with versions before we have verified compatibility?

@mmenavas
Copy link
Author

mmenavas commented Feb 22, 2023

@DiegoPino: I agree with you that the library works on PHP 8.1 without errors; however, I'm seeing PHP Deprecated messages like the ones below. I know I can suppress those messages, but I think this is an opportunity to contribute code to future proof the library.

PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 163
PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 231
PHP Deprecated:  Return type of frictionlessdata\tableschema\Table::key() should either be compatible with Iterator::key(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/.../vendor/frictionlessdata/tableschema/src/Table.php on line 226

@mmenavas
Copy link
Author

@courtney-miles: I created PR #71 to address the deprecated messages.

As a side note, it looks like we can ignore most of the PHPStan messages as PHP8.1 (and 8.2) are only yelling at us when the return types of overriding methods are omitted or incompatible with those of the parent methods.

@courtney-miles
Copy link
Collaborator

I'm sorry for coming back to this sooo late.

I have raised #72 because there was a number of changes to address deprecation notices.

The hairiest part with PHP 8.1 is that strptime is deprecated -- which means PHP has no native way to interpret the format pattern for Date (and datetime, time.) So those patterns have had to be transformed to the format that date_parse_from_format understands -- this is done with best effort, but then my understanding is that strptime was dropped because it itself cannot consistently interpret patterns between system.

@courtney-miles
Copy link
Collaborator

Changes have been released in version 1.2.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 a pull request may close this issue.

4 participants