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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants