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 PHP 8.1 auto_detect_line_endings deprecation notice #3394

Closed
wants to merge 1 commit into from

Conversation

alexpott
Copy link

auto_detect_line_endings is deprecated in PHP 8.1 - allegedly it has not been relevant for a while - https://www.mail-archive.com/internals@lists.php.net/msg107423.html

Copy link
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

PHP 8.1 deprecates this setting, but doesn't remove support for it (yet).

For the purposes of PHPCS, it may still be relevant for this setting to be enabled as legacy projects being scanned, may contain files saved in the old Mac format.

With that in mind, I'd like to suggest to silence the deprecation notice instead of removing the ini change.

@ini_set('auto_detect_line_endings', true);

Silencing the deprecation notice should work fine until PHP 9.0, by which time it can be re-evaluated what should be done with this.

@alexpott
Copy link
Author

I guess we could do that but from the https://wiki.php.net/rfc/deprecations_php_8_1 ...

The auto_detect_line_endings ini setting modifies the behavior of file() and fgets() to support an isolated \r (as opposed to \n or \r\n) as a newline character. These newlines were used by “Classic” Mac OS, a system which has been discontinued in 2001, nearly two decades ago. Interoperability with such systems is no longer relevant.

I guess we can't be sure that a legacy project is not depending on this ini setting but the chance of such a project using the latest version of PHPCS feels very slim.

I think that not removing this now queues the work up for later.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 19, 2021

  • Per the link in your original description, the "two decades" in the RFC is incorrect and should have been one decade.
  • PHPCS 3.x still supports PHP 5.4, so it is perfectly viable for a legacy project to use the latest version of PHPCS.
  • Line ending detection being incorrect, can throw quite some sniffs off in PHPCS, so it's not about a legacy project depending on the setting, but about PHPCS itself depending on this setting if the legacy project would happen to include the classic Mac OS line endings.

As an alternative, it could be considered to set the ini variable conditionally based on the PHP version detected.

Personally, I think it would be a good compromise to:

  • Silence the deprecation notice in PHPCS 3.x.
  • Remove the statement in PHPCS 4.x.

(PHPCS 4.x will have a minimum PHP version of 7.2 and is likely to be released before PHP 9.0).

Does that sound reasonable ?

@alexpott
Copy link
Author

Doing some more reading - https://superuser.com/questions/439440/did-mac-os-lion-switch-to-using-line-feeds-lf-n-for-line-breaks-instead-of - it appears that some versions of excel on os x did save files with OS 9 line endings about a decade ago.

I think one decent argument for removing this is because out-of-the-box git does not a recognise such line endings.

OTOH suppressing with an @ is the path of least change.

FWIW this code does originate from 2011 :)

commit 258a26cadac81384a0df404443cbfecfb258c7ea
Author: Greg Sherwood <gsherwood@squiz.net>
Date:   Fri Jan 14 00:40:56 2011 +0000

    Fixed bug #18193 : CodeSniffer doesn't reconize CR (\r) line endings

    git-svn-id: http://svn.php.net/repository/pear/packages/PHP_CodeSniffer/trunk@307459 c90b9560-bf6c-de11-be94-00142212c4b1

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Aug 17, 2021
@gsherwood gsherwood added this to the 3.6.1 milestone Aug 17, 2021
@gsherwood gsherwood changed the title Fix PHP 8.1 deprecation Fix PHP 8.1 auto_detect_line_endings deprecation notice Aug 26, 2021
@gsherwood
Copy link
Member

I like the approach outlined by @jrfnl so I silenced the error in PHPCS 3 and removed the use of the ini setting in PHPCS 4. Thanks for raising this issue.

@gsherwood gsherwood closed this Aug 26, 2021
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Aug 26, 2021
villfa added a commit to villfa/Codor that referenced this pull request Nov 26, 2021
villfa added a commit to villfa/Codor that referenced this pull request Nov 26, 2021
villfa added a commit to bmitch/Codor that referenced this pull request Nov 26, 2021
jonnyeom added a commit to jonnyeom/php-coding-standard that referenced this pull request Feb 5, 2023
jonnyeom added a commit to jonnyeom/php-coding-standard that referenced this pull request Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PHPCS v3 Development
Ready for Release
Development

Successfully merging this pull request may close these issues.

None yet

3 participants