Array level schema errors are not reported, fix wrong schemas identified

Some notes on what I did there.

1. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTestBase.php
   @@ -110,17 +114,13 @@ protected function checkValue($key, $value) {
   -        else {
   -          // @todo throw an exception due to an incomplete schema. Only possible
   -          //   once https://drupal.org/node/1910624 is complete.
   -        }
   -        $class = get_class($element);
            if (!$success) {
   +          $class = get_class($element);
              $this->fail("{$this->configName}:$key has the wrong schema. Variable type is $type and schema class is $class.");
            }
   

   Note that this @todo can be removed, because AFAIS the !$success already covers this entirely. When in this removed else case, $success was false.

2. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTestBase.php
   @@ -129,7 +129,18 @@ protected function checkValue($key, $value) {
   +          $this->fail("{$this->configName}:$key is not defined as a mapping or sequence but is not a scalar value.");
   

   There are lots of NOTs here, any feedback welcome on better message.

3. +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTestBase.php
   @@ -129,7 +129,18 @@ protected function checkValue($key, $value) {
   +      catch (SchemaIncompleteException $e) {
   +        $this->fail("{$this->configName}:$key has no schema.");
   +      }
   

   There was no catching this error before. We only caught missing schema for scalars. That means a new index for a mapping that was undefined vs. a mapping entirely undefined looked the same from the errors. Now the entirely undefined mapping throws an error itself.

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigSchemaTestBase.php
@@ -129,7 +129,18 @@ protected function checkValue($key, $value) {
+        if (!($element instanceof Mapping) && !($element instanceof Sequence)) {
+          $this->fail("{$this->configName}:$key is not defined as a mapping or sequence but is not a scalar value.");

We can also test for ArrayElement instead for a more general approach with contrib extensions in mind that are array based. Then the fail should be more general.

More general array test as per #2.

Great, the patch now found several undefined group level items that did not have children so were not picked up by the prior test :) All fails seem to be legit and need fixes. Woot!

Retitled so we can fix the ones found here, which makes sense since we have tests to prove they work. :)

Here are some fixes with fail examples:

views.view.who_s_new:display.default.display_options.cache.options has no schema. et. al. indicated display options lack an options key defined. Note that most other keys did not define types either, so I did not define the type here. Since this is an array type, I think we are onto more fails about its type now :D We should figure out whether this is a sequence of some things or a mapping with potential keys...

update_test.settings:xml_map is not defined as an array type (eg. mapping or sequence) but is not a scalar value. and similar are funny; the type was misspelled in the schema :P

views.view.taxonomy_term:display.default.display_options.arguments.term_node_tid_depth.validate_options.bundles is not defined as an array type (eg. mapping or sequence) but is not a scalar value. in this case the bundle was defined as a boolean while the actual value is an array; I *think* this is a sequence of bundle names, so gave that in the schema.

search.page.user_search:configuration is not defined as an array type (eg. mapping or sequence) but is not a scalar value. is a dynamic typed item per plugin type but there was no definition for the user_search plugin (as opposed to the node search one for example); however user search seems to have no configuration options based on the plugin code, so this is an empty mapping

This is IMHO a great sampling of what kind of schema errors are found by this little rounding out of the schema test. There are some other items that I did not yet cover, but wanted to send this set for a round of testing.

The other three missing things are good use cases in themselves too:

field.instance.node.article.field_tags:default_value is not defined as an array type (eg. mapping or sequence) but is not a scalar value. as well as the settings both of which are field_type dependent dynamic types. Turns out the tags field on articles has **no field_type** specified in the default config. That is a bit surprising... My active storage copy has it, so I think its somehow fixed in the install process when its resaved... Oh well.

field.instance.user.user.user_picture:default_value is not defined as an array type (eg. mapping or sequence) but is not a scalar value. and same for article image field indicates the image field default value has no typing.

entity.form_display.node.article.default:hidden has no schema. apparently entity form displays got a list of hidden elements that were not documented yet; I *think* this is a sequence of things but not sure of what, need to figure out.

This may still fail on type mismatches as indicated above but at least found all the places where things were misspecified/missing.

Fixed the two places where I defined the key but not the type although the item was array type. This now should pass but still most of my "fixes" are just fixing the fail itself and not documenting the type proper. Need to figure out the right types more granularly. Anyone up to help? :)

Should now only fail on views. Managed to figure out image field default keys for the mapping and that entity displays hidden components are a string keyed boolean list. Did not manage to track down where the views cache options are stored and how... Seems to be the only one to fix the current scope of this issue.

All right, the options are already somewhat defined, the empty options need to be defined too though. This will merge type definitions well I think and more in line with how base types vs. actual types are defined in views and elsewhere.

Retitled for array level only not claiming any other ones are fixed here. There are more issues elsewhere.

Opened #2268439: String introspection backwards data typing is confusing / incorrect as the scalar level counterpart of this one to tighten up schema validation.

Was thinking how to define "an empty array that will always be empty" and we don't really have a direct way in schema to define that. So opted to use a sequence with items of type undefined. Since these will never have any items, the undefined type will never be reached but is syntactically required as the definition of schema. I *think* this will finally pass all tests.

I personally verified all items in this schema are correct.

Definition of type item should be on the right level.

As for #27.2: yeah this test is what you have for now :D Then we should start throwing exceptions from the casting stuff, which will unearth further issues.

RTBC as per review in #27 and #28 :)

Rerolled. Best to wait for testbot I guess.

Yay, superb!

All right, now opened #2270815: Make schema testing code available as generic API to abstract this code out. This was part of the original goal but then all the bugs we found made it impractical to do it here.