Add check to Message::getText() to avoid unnecessary argument processing

[nikanderson]

In Message::getText() all arguments are processed regardless of whether their keys are present in the message type's template. It is possible for expensive callbacks to exist in arguments that are not used.

For example, the Commerce Message module adds a callback (commerce_message_order_summary) to all messages that have a Commerce Order EntityReference which returns the output of a view. This causes each call to Message::getText() on an affected message to execute the view, even if the result is then ignored.

This PR adds a check to ensure that the argument key exists in the output and will be matched by strtr() before preparing the arguments for output, thus skipping any potentially expensive callbacks whose return values would go unused.

[amitaibu]

I guess this is the change, right? It's smart!

To keep the diff smaller - and I tend to prefer return early you can

if (strpos($output, $key) === FALSE) {
  // The replacement key is not in the output, so save computation.
  continue;
}

I wonder if there are cases where we would still want to computation. We can add a force_computation (or another better name), in the $options.

(This will require a simpleTest)

[nikanderson]

Thanks for the feedback; yes, the change is just a one-line check and comment wrapping the business inside the foreach. I like your approach, I'll incorporate that.

I also share your concern that argument callbacks might be (mis)used in a way that creates needed side effects. Adding an option to force callbacks to be executed anyway would allow any such misuers to update to force legacy behavior.

[nikanderson]

I feel like I might not be doing PR correctly in some way; please let me know if I should be amending my original commit or just be adding a second like I have.

[amitaibu]

Adding more commits is fine. 👍