D6 user_mail tokens do not get converted

Since I've been working on the related issue, https://www.drupal.org/node/2353817, I'll have a go at this one.

@Ryan Weal, NP. I'm just glad I spotted it. And that I've learned enough to add to your work.

Updated the expected text for the tests.

I didn't see user_mail_user_mail_register_pending_approval_subject and user_mail_user_mail_register_pending_approval_body in D6, so changed them to mail_register_pending_approval_subject and user_mail_register_pending_approval_body which I did see.

Moved the plugin to the user module

Well, messing up Variable.php was bound to happen sooner or later. Fixed that. Note the interdiff is against the patch in #2 because #6 is just plain wrong, not just because of variable.php but also because it includes a diff on a . file.

Nice patch. Makes good sense and is well-tested.

+++ b/core/modules/user/src/Plugin/migrate/process/ConvertTokens.php
+ * ConvertTokens changes tokens to new values.

Can this explanation be a bit more detailed? Maybe something like "This plugin replaces old !token placeholders with proper [tokens]." Paraphrase as desired :)

+++ b/core/modules/user/src/Plugin/migrate/process/ConvertTokens.php
+  /**
+   * {@inheritdoc}
+   *
+   * ConvertTokensenates the strings in the current value.
+   */

I don't think we're supposed to combine {@inheritdoc} with a comment. Let's ditch the comment.

+++ b/core/modules/user/src/Plugin/migrate/process/ConvertTokens.php
+  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
+

Extra line of white space.

+++ b/core/modules/user/src/Plugin/migrate/process/ConvertTokens.php
+    if (!is_array($value)) {
+      return str_replace(array_keys($tokens), $tokens, $value);
+    }
+    else {
+      throw new MigrateException(sprintf('%s is an array', var_export($value, TRUE)));
+    }

I find this error message awkward. Can we change the !is_array() to just is_string(), change the exception to "Value must be a string" or similar, and remove the var_export(), since that will muddy up the exception message and make it difficult to read?

+++ b/core/modules/user/src/Tests/Migrate/d6/MigrateUserConfigsTest.php

Nice assertions -- very thorough. For bonus points, can we also add a test which checks that the plugin will throw an exception if given an array (or other non-string)? It should look something like this (in PHPUnit we would just use @expectedException, but we're not quite there yet):

public function testExceptionFromInvalidArgument() {
  try {
    $plugin->transform($invalid_value);
    $this->fail('Did not catch expected MigrateException when trying to transform invalid value.');
  }
  catch (MigrateException $e) {
    $this->pass('Caught expected MigrateException when trying to transform invalid value.');
  }
}

I have it from @xjm that this is kosher in SimpleTest.

1. Fixed
2. Fixed
3. Fixed
4. Fixed
5. Thx. No bonus points yet. I've not figure out how to create $plugin, as in your example.

Works for me.

No longer applies.

Simple reroll, not tested locally.
No interdiff, due to changes in Variable.php

As agreed on IRC, set to RTBC

Committed and pushed to 8.0.x. Thanks!