Skip to content

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 20, 2021

Follow up on #3480

I realized after the merge that I had not published a few follow up comments. Addressing those here.

  • The new is_readonly key still needed to be added to the documentation for the File::getMemberProperties() method.
  • For consistency in how the method is tested, adding the is_readonly property to all other test expectations, which makes the testPHP81NotReadonly test case redundant.
  • Adding two additional test cases. In particular I wanted to verify (and safeguard) that the retokenization of the ? to T_NULLABLE after a readonly keyword would not be broken (which it partially was, but that has now been fixed in PHP 8.1 | Tokenizer/PHP: readonly vs union types bug fix #3513).
@jrfnl jrfnl force-pushed the feature/file-getmemberprops-minor-tweaks-for-readonly branch from ee8ea84 to 5c8429d Compare April 20, 2022 10:17
@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 20, 2022

Rebased to get past imaginary merge conflicts.

@gsherwood As this is a follow-up on a PR which has gone into PHPCS 3.7.0 and updates the documentation for that change, can this still be merged in 3.7.0 ?

Follow up on 3480 I realized after the merge that I had not published a few follow up comments. Addressing those here. * The new `is_readonly` key still needed to be added to the documentation for the `File::getMemberProperties()` method. * For consistency in how the method is tested, adding the `is_readonly` property to all other test expectations, which makes the `testPHP81NotReadonly` test case redundant. * Adding two additional test cases. In particular I wanted to verify (and safeguard) that the retokenization of the `?` to `T_NULLABLE` after a `readonly` keyword would not be broken (which it partially was, but that has now been fixed in 3513).
@jrfnl jrfnl force-pushed the feature/file-getmemberprops-minor-tweaks-for-readonly branch from 5c8429d to 9d4c6d1 Compare April 23, 2022 01:55
@gsherwood gsherwood added this to the 3.7.0 milestone May 16, 2022
@gsherwood gsherwood merged commit 502e62a into squizlabs:master May 16, 2022
@gsherwood
Copy link
Member

Thank you

@jrfnl jrfnl deleted the feature/file-getmemberprops-minor-tweaks-for-readonly branch May 16, 2022 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants