neverExplode value is not taken into account in processItem if thereis a custom separator

@mathilde_dumond could you add a test maybe?

I confirmed the root cause and wrote the test @anybody asked for in #6, and while testing MR !101 I hit a regression that I fixed.

neverExplode() has been dead code since the module started requiring Metatag 2.x. In processItem() the separator branch is tested first:

elseif ($this->schemaMetatagManager->hasSeparator()) {
  $explode = TRUE;
}
else {
  $explode = !in_array($key, $this->neverExplode());
}

hasSeparator() is is_callable([$this->metatagManager, 'getSeparator']), and since Metatag 2.0 MetatagManager uses the MetatagSeparator trait, so that method always exists. composer.json requires drupal/metatag:^2.0, so the else branch cannot be reached on any supported install. Every nested value is split on the separator, including free text.

MR !101 makes neverExplode() win unconditionally. That fixes the free text case, but it also stops the pivot from ever seeing more than one value, because exploding is what feeds the pivot. Driving the real classes with the site separator, MR !101 turns this input:

{
    "@type": "PostalAddress",
    "pivot": "1",
    "streetAddress": "1 High Street,2 Low Street,3 Mid Street",
    "addressLocality": "Patras,Athens,Rhodes"
}

into three addresses that each repeat the whole unsplit string:

[
    {"@type": "PostalAddress", "streetAddress": "1 High Street,2 Low Street,3 Mid Street", "addressLocality": "Patras"},
    {"@type": "PostalAddress", "streetAddress": "1 High Street,2 Low Street,3 Mid Street", "addressLocality": "Athens"},
    {"@type": "PostalAddress", "streetAddress": "1 High Street,2 Low Street,3 Mid Street", "addressLocality": "Rhodes"}
]

This is exactly what #3133218 ("streetAddress should explode") reports, so the two issues pull in opposite directions. With text added to the list the same thing happens to FAQ pages: a pivoted set of five questions collapses into one Question holding all five questions and all five answers in two strings.

Free text only needs to stay intact when nothing consumes the extra values. A pivot is the only process that turns a list of values back into a list of objects, so the two cases can be told apart:

if ($key === 0) {
  $explode = $this->multiple();
}
elseif (in_array($key, $this->neverExplode(), TRUE)) {
  $explode = $this->pivoted;
}
else {
  $explode = TRUE;
}

$this->pivoted is set once in output() by walking the value for a pivot key before the items are processed. Without a pivot the extra values would be rendered as several values of a property that takes one, which is what search engines report as a duplicate field, so there is nothing to lose there. With a pivot the current behaviour is kept exactly.

The patch also adds text to neverExplode(), as proposed in #3134041-23.

The one behaviour change beyond the bug is recipeInstructions without a pivot, which stops being split into a list. The list it produced was not reliable, since "Mix the flour, sugar and salt,Add the eggs,Bake for 30 minutes" already came out as four fragments rather than three steps. Sites that want the list can set a separator that does not occur in the text, or enable the pivot.

tests/src/Unit/SchemaNameBaseExplodeTest.php covers four cases: free text kept without a pivot, free text exploded with a pivot, other properties still exploded, and a custom separator. It builds the tag without the plugin system, since the Metatag plugin constructor needs the container, so it stays a unit test. ExplodeTestTag is a separate file to keep the one class per file rule.

Two things:

1. The pivot check is per tag, not per branch. If one branch of a tag is pivoted, free text elsewhere in the same tag still explodes. Making it per branch means replacing array_walk_recursive() with a walk that tracks the path. That is more precise but a bigger change, and the current shape never produces output worse than the release does today.
2. The hardcoded key list still lives in the base class, which is the thing @DamienMcKenna objected to in #3425455-4. Now that #3425455 introduces a long annotation on the sub property, the same flag would express "this is free text, do not explode it" without a list. That would be a good follow up once #3425455 lands, and would let neverExplode() be deprecated.

Since this issue and #3593489 describe the same defect, whichever one you prefer to keep, the other can point at it.