search

celest-time / celest

Celest provides a alternative date and time API for PHP.
http://celest-time.org/
GNU General Public License v2.0
9 stars 1 forks source link

PHP Errors when parsing using string patterns #2

Closed MazeChaZer closed 7 years ago

MazeChaZer commented 7 years ago

When I try to parse a date using a string pattern, I get PHP errors:

>>> LocalDate::parseWith('12.10.2017', DateTimeFormatter::ofPattern('dd.MM.yyyy'))
PHP error:  Undefined index: DayOfMonth in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 18
PHP error:  Undefined index: MonthOfYear in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 18
PHP error:  Undefined index: YearOfEra in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 18
PHP error:  Undefined index: ProlepticMonth in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 31
PHP error:  Undefined index: Era in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 31
PHP error:  Undefined index: Year in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 53
PHP error:  Undefined index: Year in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 18
PHP error:  Undefined index: HourOfDay in /home/jonas/PhpstormProjects/bps/vendor/celest/celest/src/Celest/Temporal/FieldValues.php on line 53
=> Celest\LocalDate {#421}
hanikesn commented 7 years ago

What php version do you use and what version of the library?

MazeChaZer commented 7 years ago

Thanks for the quick response! It happens on PHP 5.6.30 and PHP 7.1.10 with the latest master (1d27634dbe32cfadead808ab695c60fb22ef68f0).

MazeChaZer commented 7 years ago

@hanikesn I investigated this issue further, and it seems that the cause of these errors is the use of the @-operator. See https://github.com/celest-time/celest/blob/b5ab3612a32931ae881c47b17f976322f22a7a4f/src/Celest/Temporal/FieldValues.php#L18 In our framework we use a strict error handler function, which turns all errors, warnings and notices into exceptions (via set_error_handler), so this is a problem for us. I believe it would be good practice not to use the @-operator and instead properly check if an index exists using array_key_exists or such. Further information at https://stackoverflow.com/questions/7380782/error-suppression-operator-and-set-error-handler What do you think about this? If you like the idea, I can prepare a pull request to remove usages of the @-operator.

hanikesn commented 7 years ago

Ah interesting error case. A patch is very much welcome. I'm also thinking about possible ways to modify the test suite to catch more errors like that.

MazeChaZer commented 7 years ago

👍 Great I will work on a pull request

hanikesn commented 7 years ago

As this is a bigger thing I started working on it myself. As there are quite a few occurrences of it.

MazeChaZer commented 7 years ago

:+1: Alright