MessageFormatter parse error improvements

[pilif]

Looking into MessageFormatter and the underlying ICU pattern documentation, I was blown away by the functionality offered by the patterns that MessageFormatter understands. This is incredibly useful and I was totally going to look into making use of this in our main product which already relies on intl for number and date formatting, but uses a home-grown solution for message localization.

However, while playing around with this, I came across a usability-issue around MessageFormatter::formatMessage() and especially MessageFormatter::__construct():

As the formatting patterns can be pretty complex, it's likely that they contain parsing errors (especially in my case as I was just getting into this), so it would be helpful to get to details of such parsing errors.

Unfortunately, MessageFormatter::formatMessage() via intl_get_error_message() would just report a generic U_PATTERN_SYNTAX_ERROR and, even worse, MessageFormatter:__construct() would turn any possible internal error into a generic Constructor failed error message.

It doesn't have to be that way though: ICU is capable of reporting additional information about a parser error and indeed, the intl extension is already making use of this in other places.

So this PR contains two commits that

1. fix the problem for MessageFormatter::formatMessage() and intl_get_error_message() to report the additional parsing error message (see updated tests)
2. fix MessageFormatter::__construct() to report the actual error message rather than a generic "Constructor failed"

[pilif]

oh crap - you've just cut the PHP-8 branch.

If you think this could go into PHP 8.0, I'll rebase and re-target the PR

[Girgias]

Test failure is unrelated and has been marked as XFAIL on master.

[nikic]

This looks good to me, and I think it should go into PHP 7.4 (unless it's conflict-y). Doesn't change any behavior, just improves diagnostics.

[nikic]

This null check is unnecessary, spprintf always produces a non-NULL string.

[pilif]

I'll add a fixup (and rebase on top of 7.4), but FYI, this code is somewhat "heavily inspired" by

php-src/ext/intl/transliterator/transliterator_methods.c

Lines 171 to 179 in c3365bb

	parse_error_str = intl_parse_error_to_string( &parse_error );
	spprintf( &msg, 0, "transliterator_create_from_rules: unable to "
	"create ICU transliterator from rules (%s)", parse_error_str.s? ZSTR_VAL(parse_error_str.s) : "" );
	smart_str_free( &parse_error_str );
	if( msg != NULL )
	{
	intl_errors_set_custom_msg( INTL_DATA_ERROR_P( to ), msg, 1 );
	efree( msg );
	}

I'll make a followup PR to get rid of the null check there too 😝

[nikic]

Again, this check is not needed.

[pilif]

there will be a few conflicts on PHP-7.4 (mostly due to format string changes in tests). I'll make a second PR targeting 7.4 - then you can pick which one you'd be willing to merge.

[pilif]

rebased on top of current master (which means that the false-positive failure due to the timelib changes now went away) and implemented the suggestion by @nikic.

I have also made a secondary PR (#6325) on top of PHP-7.4 if you prefer to merge this into 7.4

[nikic]

Closing this as I merged #6325.