Multi-layer encoding
When calling assert like so:
Assert::count(..., message: $message);
$message is forwarded to sprintf. As a result if you are to do something like this:
Assert::count(
...,
message: 'This is my custom message %s', // <- the placeholder will be filled up automatically, great!
);
Which does mean if you are using your own sprintf you should escape it first:
Assert::count(
...,
message: 'This is my %%w custom message %s', // <- manually escape "%w" as this would make sprintf fail (it is an invalid format specifier)
);
// Automatic escaping:
Assert::count(
...,
message: str_replace('%', '%%', 'This is my %%w custom message %s'),
);
So far so good. Now the problem: this does not work for all assertions. Indeed the example above with Assert::count() will still result in ValueError: Unknown format specifier "w".
The root of the issue is that an assertion can rely on other assertions internally, e.g. ::count() calls ::eq(). So the custom escaped message becomes unescaped when passing it to ::eq(). Making it work requires the user to find out how many items the string is evaluated to escape it accordingly (in this example twice).
I think this is a design flaw and the way it could be fixed is to escape the message passed within Assert when relying on inner assertions:
// Assert.php
public static function count($array, $number, $message = '')
{
static::eq(
\count($array),
$number,
\sprintf(
self::sprintfEscape($message) ?: 'Expected an array to contain %d elements. Got: %d.',
$number,
\count($array)
)
);
}
public static function sprintfEscape(string $value): string {
return str_replace('%', '%%', $value);
}
Will need to think about this one some more. I use a variety of custom messages, but generally I format the custom message before passing it to Assert::foo.
Maybe #221 would be a better solution to this?
@shadowhand I think those are different problems entirely, but I am certainly in support of #221.
generally I format the custom message before passing it to Assert::foo
I do as well from time to time, but I don't like to figure out the "make mixed into a human readable string" logic which the library already does internally. So leveraging the placeholder is a nice feature