Skip to content

Added deprecated function PHP 8.1 to DiscouragedFunctionSniff #326

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

Conversation

anzin
Copy link
Contributor

@anzin anzin commented Nov 5, 2021

Description (*)

Added deprecated function PHP 8.1 to DiscouragedFunctionSniff

Addresses backward incompatible change in PHP 8.1: https://wiki.php.net/rfc/deprecations_php_8_1

Fixed Issues (if relevant)

  1. Add functions to sniff Magento2.Functions.DiscouragedFunction of the magento-coding-standard magento2#34548

Contribution checklist (*)

  • Author has signed the Adobe CLA
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

andrewbess
andrewbess previously approved these changes Nov 5, 2021
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @anzin ! Please see my comment

'^date_sunrise$' => 'date_sun_info',
'^date_sunset$' => 'date_sun_info',
'^strptime$' => 'date_parse_from_format',
'^strftime$' => null,
Copy link
Member

@sivaschenko sivaschenko Nov 6, 2021

Choose a reason for hiding this comment

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

It would be great to add replacement recommendations for each of the deprecated functions. For example for this function:

Suggested change
'^strftime$' => null,
'^strftime$' => 'IntlDateFormatter::format()',

Copy link
Member

@sivaschenko sivaschenko Nov 6, 2021

Choose a reason for hiding this comment

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

Even though the replacement can be more complex than just a single function call, like in this case

setlocale(LC_ALL, 'nl_NL');
$formatter = new IntlDateFormatter(\Locale::getDefault(), IntlDateFormatter::FULL, IntlDateFormatter::FULL);
$formatter->setPattern('EEEE, LLLL d');
echo $formatter->format(new DateTime());

I think the replacement suggestion should at least help the developer with the research direction.

@anzin anzin force-pushed the improvements/add-php-deprecated-function branch from 89e33f2 to ef8bd3f Compare November 8, 2021 11:35
@anzin anzin requested a review from sivaschenko November 8, 2021 11:36
@anzin
Copy link
Contributor Author

anzin commented Nov 9, 2021

Hello @sivaschenko, I added some replacement recommendations, please check if every.

@sivaschenko
Copy link
Member

@magento import pr to magento-commerce/magento-coding-standard

@m2-github-services
Copy link
Contributor

@sivaschenko an error occurred during the Pull Request import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants