@jrfnl Here is the sniff we discussed earlier today. Happy to chat for feedback, since this is my first proper sniff contribution.

Same comment about documentation for these kinds of sniffs: #1975 (comment)

@jrfnl Onto which branch should I rebase my PRs for errors in unrelated files to go away?

Edit: I sifted through the conversation on another PR and saw that it was develop. I did the same here.

@jrfnl Onto which branch should I rebase my PRs for errors in unrelated files to go away?

develop is where development happens, so feel free to rebase on that.

@jrfnl It looks like the CI is giving me mutually-exclusive requirements. One job wants me to add visibility to class constants, but if I do that, other jobs will complain about invalid syntax on older PHP versions. Unless I'm not seeing something obvious, there's no way to resolve from my end other than not using class constants in my sniff.

@jrfnl I untangled this! I can't seem to find proof in Actions, but I definitely saw a PHPCS 3 in there and a whole bunch of failures (something about PHP 5.5 even). This all went away when I rebased. Then I could add constant visibility.

So it was just bad luck due to the timing of my PR relative to the PHPCS 3 drop. I'll rebase any remaining open PRs right away to avoid this.

@jrfnl Ready for review. To recap my changes:

• Matched test structure (files, method names, comments, whether we test the whole file or per-line, etc.) to what we did in the previous PR you merged.
• Fixed target versions for various tests, as here it's a deprecated feature instead of a new feature.
• Rebased and fixed any CS issues.

@afilina Thank you for your work on this. I'm nearly done reviewing the PR, but before I post my review, I want to have a discussion about whether this sniff should exist (at this time).

As a rule of thumb, PHPCompatibility flags changes in behaviour in PHP and a soft deprecation does not yield any change in the behaviour in PHP.

While PHPCompatibility does contain a few exceptions to that rule (for example: reserved namespaces), this soft deprecation of _sleep() and __wakeup() doesn't feel like one which should qualify for such an exception.

Background of the deprecation in PHP:

• Originally it was proposed and accepted to hard deprecate _sleep() and __wakeup() in PHP 8.5.
• Once some of the bigger projects started looking into this, they found that the information provided in the original RFC was misleading and that the costs of mitigating the deprecation would likely be much higher than anticipated.
• Additionally, the complexity of mitigating the deprecation also means that the mitigation would carry a high risk of bugs, especially in projects with less experienced developers.
• As a result of this real-world experience, a late PHP 8.5 RFC was proposed and accepted to change the hard deprecation (with notice at runtime) to a soft deprecation (no notice at runtime, documentation only deprecation).

Now, if PHP itself has made the choice not to hard drive this deprecation via a notice due to the cost and complexity, why should PHPCompatibility now start driving this change by flagging the methods anyway ?

/cc @MarkMaldaba

/cc @MarkMaldaba

I agree with this assessment. If the code running on PHP 8.5 gives functionally identical behaviour and outputs as earlier versions then there is not a compatibility issue to be reported.

@jrfnl @MarkMaldaba The long-term plan is to still deprecate and remove these magic methods. As someone who creates long-term modernization roadmaps for customers, it's always of huge help to have early heads-up on practices that should be discontinued. It gives more time to address issues and prevents new code from introducing future incompatibilities. The effort needed to fix an incompatibility is irrelevant. And if someone isn't ready to address an issue in their code, they can always suppress a sniff via config.