Skip to content

Add factory methods for DatePeriod and ReflectionMethod #6754

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Add factory methods for DatePeriod and ReflectionMethod #6754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92e8ca5
Add factory methods for DatePeriod
kocsismate Mar 5, 2021
640830c
Add factory method for ReflectionMethod
kocsismate Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 91 additions & 20 deletions ext/date/php_date.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4124,28 +4124,16 @@ static int date_period_initialize(timelib_time **st, timelib_time **et, timelib_
return retval;
} /* }}} */

/* {{{ Creates new DatePeriod object. */
PHP_METHOD(DatePeriod, __construct)
{
php_period_obj *dpobj;
php_date_obj *dateobj;
zval *start, *end = NULL, *interval;
zend_long recurrences = 0, options = 0;
char *isostr = NULL;
size_t isostr_len = 0;
static void construct_date_period(
INTERNAL_FUNCTION_PARAMETERS, zval *object, zval *start, zval *end, zval *interval, zend_long recurrences,
zend_long options, char *isostr, size_t isostr_len
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer it if this initialisation function gets split as well. The old __construct should use the new (3) initialisation methods, which also makes it easier to clean up in for example PHP 9.0. It can also mean that error and bounds checking (for example, recurrences can't be negative) to the right and logical position.

) {
php_period_obj *dpobj;
php_date_obj *dateobj;
timelib_time *clone;
zend_error_handling error_handling;

if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOl|l", &start, date_ce_interface, &interval, date_ce_interval, &recurrences, &options) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOO|l", &start, date_ce_interface, &interval, date_ce_interval, &end, date_ce_interface, &options) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|l", &isostr, &isostr_len, &options) == FAILURE) {
zend_type_error("DatePeriod::__construct() accepts (DateTimeInterface, DateInterval, int [, int]), or (DateTimeInterface, DateInterval, DateTime [, int]), or (string [, int]) as arguments");
RETURN_THROWS();
}
}
}

dpobj = Z_PHPPERIOD_P(ZEND_THIS);
dpobj = Z_PHPPERIOD_P(object);
dpobj->current = NULL;

if (isostr) {
Expand Down Expand Up @@ -4225,7 +4213,90 @@ PHP_METHOD(DatePeriod, __construct)

dpobj->initialized = 1;
}
/* }}} */

PHP_METHOD(DatePeriod, createFromRecurrences)
{
zval *start, *interval;
zend_long recurrences, options = 0;
zval object;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOl|l", &start, date_ce_interface, &interval, date_ce_interval, &recurrences, &options) == FAILURE) {
RETURN_THROWS();
}

object_init_ex(&object, zend_get_called_scope(execute_data));

construct_date_period(INTERNAL_FUNCTION_PARAM_PASSTHRU, &object, start, NULL, interval, recurrences, options, NULL, 0);
if (EG(exception)) {
zval_ptr_dtor(&object);
RETURN_THROWS();
}

ZVAL_COPY_VALUE(return_value, &object);
}

PHP_METHOD(DatePeriod, createFromDates)
Copy link
Member Author

Choose a reason for hiding this comment

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

other candidates I thought about: createBetweenDates, createBetweenStartAndEnd, createFromStartAndEnd

Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer createFromStartAndEnd, and I would also switch the order of arguments to:

    public static function createFromStartAndEnd(DateTimeInterface $start, DateTimeInterface $end, DateInterval $interval, int $options = 0): static {}

Copy link
Contributor

Choose a reason for hiding this comment

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

Another suggestion would be to use createFromEndpoints or createFromDatepoints to imply that the DatePeriod endpoints are expressed using DateTimeInterface objects.

{
zval *start, *end, *interval;
zend_long options = 0;
zval object;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "OOO|l", &start, date_ce_interface, &interval, date_ce_interval, &end, date_ce_interface, &options) == FAILURE) {
RETURN_THROWS();
}

object_init_ex(&object, zend_get_called_scope(execute_data));

construct_date_period(INTERNAL_FUNCTION_PARAM_PASSTHRU, &object, start, end, interval, 0, options, NULL, 0);
if (EG(exception)) {
zval_ptr_dtor(&object);
RETURN_THROWS();
}

ZVAL_COPY_VALUE(return_value, &object);
}

PHP_METHOD(DatePeriod, createFromIso8601)
{
char *isostr;
size_t isostr_len;
zend_long options = 0;
zval object;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|l", &isostr, &isostr_len, &options) == FAILURE) {
RETURN_THROWS();
}

object_init_ex(&object, zend_get_called_scope(execute_data));

construct_date_period(INTERNAL_FUNCTION_PARAM_PASSTHRU, &object, NULL, NULL, NULL, 0, options, isostr, isostr_len);
if (EG(exception)) {
zval_ptr_dtor(&object);
RETURN_THROWS();
}

ZVAL_COPY_VALUE(return_value, &object);
}

/* Creates new DatePeriod object. */
PHP_METHOD(DatePeriod, __construct)
{
zval *start, *end = NULL, *interval;
zend_long recurrences = 0, options = 0;
char *isostr = NULL;
size_t isostr_len = 0;

if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOl|l", &start, date_ce_interface, &interval, date_ce_interval, &recurrences, &options) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOO|l", &start, date_ce_interface, &interval, date_ce_interval, &end, date_ce_interface, &options) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|l", &isostr, &isostr_len, &options) == FAILURE) {
zend_type_error("DatePeriod::__construct() accepts (DateTimeInterface, DateInterval, int [, int]), or (DateTimeInterface, DateInterval, DateTime [, int]), or (string [, int]) as arguments");
RETURN_THROWS();
}
}
}

construct_date_period(INTERNAL_FUNCTION_PARAM_PASSTHRU, ZEND_THIS, start, end, interval, recurrences, options, isostr, isostr_len);
}

/* {{{ Get start date. */
PHP_METHOD(DatePeriod, getStartDate)
Expand Down
6 changes: 6 additions & 0 deletions ext/date/php_date.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ public static function __set_state(array $array) {}

class DatePeriod implements IteratorAggregate
Copy link
Member Author

@kocsismate kocsismate Mar 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should decide what to do with the overloaded constructor:

  • do nothing so that the overloaded signature remains supported forever
  • deprecate it in 8.x and remove it in 9.0 altogether
  • choose one "canonical" signature, deprecate the others in 8.x, and get rid of them in 9.0

Depending on the outcome, we could only have 2 factory methods, and the 3rd signature could be implemented by the constructor

{
public static function createFromRecurrences(DateTimeInterface $start, DateInterval $interval, int $recurrences, int $options = 0): static {}

public static function createFromDates(DateTimeInterface $start, DateInterval $interval, DateTimeInterface $end, int $options = 0): static {}

public static function createFromIso8601(string $specification, int $options = 0): static {}

/**
* @param DateTimeInterface|string $start
* @param DateInterval|int $interval
Expand Down
27 changes: 26 additions & 1 deletion ext/date/php_date_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 108136459e578cc699cffcb84d3335a11f8d5c9d */
* Stub hash: 0affb8dcab45c4067e2dfd30b34d0aa240de746e */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_strtotime, 0, 1, MAY_BE_LONG|MAY_BE_FALSE)
ZEND_ARG_TYPE_INFO(0, datetime, IS_STRING, 0)
Expand Down Expand Up @@ -403,6 +403,25 @@ ZEND_END_ARG_INFO()

#define arginfo_class_DateInterval___set_state arginfo_class_DateTime___set_state

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DatePeriod_createFromRecurrences, 0, 3, IS_STATIC, 0)
ZEND_ARG_OBJ_INFO(0, start, DateTimeInterface, 0)
ZEND_ARG_OBJ_INFO(0, interval, DateInterval, 0)
ZEND_ARG_TYPE_INFO(0, recurrences, IS_LONG, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_LONG, 0, "0")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DatePeriod_createFromDates, 0, 3, IS_STATIC, 0)
ZEND_ARG_OBJ_INFO(0, start, DateTimeInterface, 0)
ZEND_ARG_OBJ_INFO(0, interval, DateInterval, 0)
ZEND_ARG_OBJ_INFO(0, end, DateTimeInterface, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_LONG, 0, "0")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DatePeriod_createFromIso8601, 0, 1, IS_STATIC, 0)
ZEND_ARG_TYPE_INFO(0, specification, IS_STRING, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_LONG, 0, "0")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_DatePeriod___construct, 0, 0, 1)
ZEND_ARG_INFO(0, start)
ZEND_ARG_INFO(0, interval)
Expand Down Expand Up @@ -498,6 +517,9 @@ ZEND_METHOD(DateTimeZone, __set_state);
ZEND_METHOD(DateInterval, __construct);
ZEND_METHOD(DateInterval, __wakeup);
ZEND_METHOD(DateInterval, __set_state);
ZEND_METHOD(DatePeriod, createFromRecurrences);
ZEND_METHOD(DatePeriod, createFromDates);
ZEND_METHOD(DatePeriod, createFromIso8601);
ZEND_METHOD(DatePeriod, __construct);
ZEND_METHOD(DatePeriod, getStartDate);
ZEND_METHOD(DatePeriod, getEndDate);
Expand Down Expand Up @@ -647,6 +669,9 @@ static const zend_function_entry class_DateInterval_methods[] = {


static const zend_function_entry class_DatePeriod_methods[] = {
ZEND_ME(DatePeriod, createFromRecurrences, arginfo_class_DatePeriod_createFromRecurrences, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
ZEND_ME(DatePeriod, createFromDates, arginfo_class_DatePeriod_createFromDates, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
ZEND_ME(DatePeriod, createFromIso8601, arginfo_class_DatePeriod_createFromIso8601, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
ZEND_ME(DatePeriod, __construct, arginfo_class_DatePeriod___construct, ZEND_ACC_PUBLIC)
ZEND_ME(DatePeriod, getStartDate, arginfo_class_DatePeriod_getStartDate, ZEND_ACC_PUBLIC)
ZEND_ME(DatePeriod, getEndDate, arginfo_class_DatePeriod_getEndDate, ZEND_ACC_PUBLIC)
Expand Down
23 changes: 23 additions & 0 deletions ext/date/tests/DatePeriod_createFromDates.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
Test DatePeriod::createFromDates()
--FILE--
<?php

class MyDatePeriod extends DatePeriod
{
}

$start = new DateTime();
$end = new DateTime("+2 days");
$interval = new DateInterval("P1D");

$p1 = MyDatePeriod::createFromDates($start, $interval, $end);
$p2 = new MyDatePeriod($start, $interval, $end);

var_dump($p1::class);
var_dump($p1 == $p2);

?>
--EXPECT--
string(12) "MyDatePeriod"
bool(true)
26 changes: 26 additions & 0 deletions ext/date/tests/DatePeriod_createFromIso8601.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--TEST--
Test DatePeriod::createFromIso8601()
--FILE--
<?php

class MyDatePeriod extends DatePeriod
{
}

$p1 = MyDatePeriod::createFromIso8601("R4/2012-07-01T00:00:00Z/P7D");
$p2 = new MyDatePeriod("R4/2012-07-01T00:00:00Z/P7D");

var_dump($p1::class);
var_dump($p1 == $p2);

try {
MyDatePeriod::createFromIso8601("");
} catch (Exception $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECT--
string(12) "MyDatePeriod"
bool(true)
DatePeriod::createFromIso8601(): Unknown or bad format ()
29 changes: 29 additions & 0 deletions ext/date/tests/DatePeriod_createFromRecurrences.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Test DatePeriod::createFromRecurrences()
--FILE--
<?php

class MyDatePeriod extends DatePeriod
{
}

$start = new DateTime();
$interval = new DateInterval("P1D");

$p1 = MyDatePeriod::createFromRecurrences($start, $interval, 2);
$p2 = new MyDatePeriod($start, $interval, 2);

var_dump($p1::class);
var_dump($p1 == $p2);

try {
MyDatePeriod::createFromRecurrences($start, $interval, 0);
} catch (Exception $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECT--
string(12) "MyDatePeriod"
bool(true)
DatePeriod::createFromRecurrences(): Recurrence count must be greater than 0
54 changes: 39 additions & 15 deletions ext/reflection/php_reflection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3043,29 +3043,19 @@ ZEND_METHOD(ReflectionUnionType, getTypes)
}
/* }}} */

/* {{{ Constructor. Throws an Exception in case the given method does not exist */
ZEND_METHOD(ReflectionMethod, __construct)
{
zend_object *arg1_obj;
zend_string *arg1_str;
zend_string *arg2_str = NULL;

static void construct_reflection_method(
INTERNAL_FUNCTION_PARAMETERS, zval *object, zend_object *arg1_obj, zend_string *arg1_str, zend_string *arg2_str
) {
zend_object *orig_obj = NULL;
zend_class_entry *ce = NULL;
zend_string *class_name = NULL;
char *method_name;
size_t method_name_len;
char *lcname;

zval *object;
reflection_object *intern;
zend_function *mptr;

ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_OBJ_OR_STR(arg1_obj, arg1_str)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_NULL(arg2_str)
ZEND_PARSE_PARAMETERS_END();

if (arg1_obj) {
if (!arg2_str) {
Expand Down Expand Up @@ -3109,7 +3099,6 @@ ZEND_METHOD(ReflectionMethod, __construct)
zend_string_release(class_name);
}

object = ZEND_THIS;
intern = Z_REFLECTION_P(object);

lcname = zend_str_tolower_dup(method_name, method_name_len);
Expand All @@ -3133,7 +3122,42 @@ ZEND_METHOD(ReflectionMethod, __construct)
intern->ref_type = REF_TYPE_FUNCTION;
intern->ce = ce;
}
/* }}} */

ZEND_METHOD(ReflectionMethod, createFromMethodName)
{
zend_string *method_name;
zval object;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(method_name)
ZEND_PARSE_PARAMETERS_END();

object_init_ex(&object, zend_get_called_scope(execute_data));

construct_reflection_method(INTERNAL_FUNCTION_PARAM_PASSTHRU, &object, NULL, method_name, NULL);
if (EG(exception)) {
zval_ptr_dtor(&object);
RETURN_THROWS();
}

ZVAL_COPY_VALUE(return_value, &object);
}

/* {{{ Constructor. Throws an Exception in case the given method does not exist */
ZEND_METHOD(ReflectionMethod, __construct)
{
zend_object *arg1_obj;
zend_string *arg1_str;
zend_string *arg2_str = NULL;

ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_OBJ_OR_STR(arg1_obj, arg1_str)
Z_PARAM_OPTIONAL
Z_PARAM_STR_OR_NULL(arg2_str)
ZEND_PARSE_PARAMETERS_END();

construct_reflection_method(INTERNAL_FUNCTION_PARAM_PASSTHRU, ZEND_THIS, arg1_obj, arg1_str, arg2_str);
}

/* {{{ Returns a string representation */
ZEND_METHOD(ReflectionMethod, __toString)
Expand Down
2 changes: 2 additions & 0 deletions ext/reflection/php_reflection.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ class ReflectionMethod extends ReflectionFunctionAbstract
{
public string $class;

public static function createFromMethodName(string $method): static {}

public function __construct(object|string $objectOrMethod, ?string $method = null) {}

public function __toString(): string {}
Expand Down
Loading