Skip to content

Fix GH-11310: __debugInfo() does nothing on classes extending DateTime - #12534

Open
paulfedorow wants to merge 1 commit into
php:masterfrom
paulfedorow:date-debuginfo
Open

Fix GH-11310: __debugInfo() does nothing on classes extending DateTime#12534
paulfedorow wants to merge 1 commit into
php:masterfrom
paulfedorow:date-debuginfo

Conversation

@paulfedorow

@paulfedorowpaulfedorow commented Oct 27, 2023•
edited
Loading

Copy link
Copy Markdown

FixesGH-11310. Makes the __debugInfo method work in classes that extend DateTime and DateTimeImmutable.

  • Implemented __debugInfo for DateTime and DateTimeImmutable.
  • Modified the get_properties_for handlers of DateTime and DateTimeImmutable to invoke __debugInfo.

@paulfedorow
paulfedorow marked this pull request as draft October 27, 2023 08:09
@paulfedorow
paulfedorow marked this pull request as ready for review October 27, 2023 09:10
Girgias
Girgias reviewed Oct 27, 2023
View reviewed changes
Comment threadext/date/php_date.c Outdated
Comment threadext/date/tests/date_debugInfo.phpt Outdated
@paulfedorow
FixphpGH-11310: __debugInfo() does nothing on classes extending Date…
54f607d
…Time
Makes the `__debugInfo` method work in classes that extend `DateTime`
and `DateTimeImmutable`.
* Implemented `__debugInfo` for `DateTime` and `DateTimeImmutable`.
* Modified the `get_properties_for` handlers of `DateTime` and
`DateTimeImmutable` to invoke `__debugInfo`.
derickr
derickr requested changes Mar 18, 2024
View reviewed changes

@derickrderickr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this not needed for the other Date classes?

Comment threadext/date/php_date.stub.php

public function __unserialize(array $data): void {}

/** @tentative-return-type */

@derickrderickrMar 18, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does this have @tentative-return-type?

@prescriptionlifeline

prescriptionlifeline commented Jul 7, 2024

Copy link
Copy Markdown

Any update on this?

@kocsismate

kocsismate commented Oct 1, 2024

Copy link
Copy Markdown
Member

@paulfedorow Could you please rebase + update your PR according to Derick's comment? Since it seems like a useful fix (btw the same inconsistency exists in other places), so I'll finish it if you don't have time for this anymore.

DanielEScherzer
DanielEScherzer reviewed Oct 9, 2024
View reviewed changes
Comment threadext/date/tests/date_debugInfo.phpt
}
}

class MyDateTimeZone extends DateTimeZone {

@DanielEScherzerDanielEScherzerOct 9, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggest adding a test case for MyDateTimeZone, otherwise why define the class at all?

@DanielEScherzerDanielEScherzer mentioned this pull request Oct 9, 2024
Open
@DanielEScherzer

DanielEScherzer commented Oct 9, 2024

Copy link
Copy Markdown
Member

Note that my patch at #16333 should fix this in a more general way

@paulfedorow

paulfedorow commented Oct 10, 2024

Copy link
Copy Markdown
Author

I'm closing this in favor of the more general fix provided in #16333

@DanielEScherzer

DanielEScherzer commented Oct 10, 2024

Copy link
Copy Markdown
Member

I'm closing this in favor of the more general fix provided in #16333

Actually my comment may have been a bit premature, it seems that my more general approach may not be desirable

@DanielEScherzerDanielEScherzer mentioned this pull request Oct 21, 2024
Closed
@iluuu1994iluuu1994 reopened this Oct 22, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GirgiasGirgiasGirgias left review comments
@DanielEScherzerDanielEScherzerDanielEScherzer left review comments
@derickrderickrderickr requested changes
@kocsismatekocsismateAwaiting requested review from kocsismatekocsismate is a code owner

Assignees

No one assigned

Labels

Extension: date

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

__debugInfo() does nothing on classes extending DateTime

7 participants

@paulfedorow@prescriptionlifeline@kocsismate@DanielEScherzer@derickr@Girgias@iluuu1994