Create plugin per file YAML discovery

1. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,138 @@
   +   *   Defaults to 'id'.
   

   Let's skip that, its pointless IMHO

2. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,138 @@
   +        catch (InvalidDataTypeException $e) {
   +          throw new DiscoveryException("The $file contains invalid YAML");
   +        }
   

   Can we add the exception onto the other exception as previous exception?

3. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,138 @@
   +    foreach ($this->directories as $provider => $directories) {
   +      $directories = (array) $directories;
   +      foreach ($directories as $directory) {
   +        if (is_dir($directory)) {
   +          foreach (scandir($directory) as $file) {
   

   What about using a iterator?

4. +++ b/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php
   @@ -0,0 +1,149 @@
   +    $this->assertEquals(1, count($data['test_2']), 'test_4 provides 1 item');
   ...
   +    $this->assertEquals(1, count($data['test_1']), 'test_1 provides 1 item');
   

   Let's use $this->assertCount()

5. +++ b/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php
   @@ -0,0 +1,149 @@
   +   *
   +   * @covers ::findAll
   +   */
   +  public function testDiscoveryNoIdException() {
   +    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1/item_1.test.yml contains no data in the identifier key \'id\'');
   

   We could put @covers ::getIdentifier

/me waits for damian to appear on this issue.

All looks pretty good to me, just wondering do we need this cast?

+++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
@@ -0,0 +1,138 @@
+      $directories = (array) $directories;
+      foreach ($directories as $directory) {

Yes, I spoke to Daniel briefly about suggesting an iterator instead. Looks like you've already done that - nice!

If we're already using the FileSystem iterator, IMO it would be nice to have the logic encapsulated in an iterator of its own. This similar pattern must be used at least a couple of times (maybe more) in core alone? So having one goto way to get and iterate YAML files in a directly would be nice.

Something like this maybe.. Thoughts?

EDIT: Sorry, should have given this file a .txt extension!

+++ b/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php
@@ -0,0 +1,157 @@
+    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1/item_1.test.yml contains invalid YAML');

Nit: Is this the preferred way to set expected exceptions now? I prefer using the annotation myself. Just seems easier to spot when scanning the test.

Sure, follow up is OK. That was not a finished product, just a suggestion.

Created #2671708: Add a RegexDirectoryIterator for that.

Nit: Is this the preferred way to set expected exceptions now? I prefer using the annotation myself. Just seems easier to spot when scanning the test.

https://thephp.cc/news/2016/02/questioning-phpunit-best-practices

Skinr module would actually need to be able to find .yml files recursively. e.g. /modules/skinr/skins/plugin1/plugin1.yml and /modules/skinr/skins/plugin2/plugin2.yml
Layout module actually goes a step further and allows arbitrary recursion depth. So the following should also be a valid path: /modules/layout/layouts/static/one-col/one-col.yml

Talking with @dawehner it seems we should probably allow specifying the iterator so you could use a recursive iterator or a more basic one.

I'm not sure this is useful for core or other modules, but I need to be able to find out the .yml file's path. The YamlDirectoryDiscovery component doesn't include this with the data returned, only the provider (module or theme name) is returned.
The reason I would need this info is that the .yml plugin includes paths to css and js files, which are relative to the plugin's directory. Requiring paths in the .yml file to be relative to the module's or theme's root (instead of the plugin's root) would quickly become tedious and error prone.

+++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
@@ -142,4 +139,22 @@ protected function findFiles() {
+   * @param $directory

Nitpick: should be typehinted with string

1. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,160 @@
   +        $data[static::DISCOVERY_PATH] = $file;
   

   This is a bit confusing. Don't we want to do something like $data['plugin_definition_file'] = $file or static::DISCOVERY_PATH?

2. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,160 @@
   +  protected function getIdentifier($file, array $data) {
   

   missing docs on parameter $file

Thank you alex, this clarifies it.

+1 Just looked through the most recent patch again - looks grrrrreat!

Would it make sense to use 'pathname' like the Extension class to keep things consistent, instead of '_discovered_file_path'? Of course, Extension is an object and only exposes the getPathname() function, not the actual pathname variable. And there is a chance to conflict with a 'pathname' key in the .yml file. But I thought I'd throw this out there.

@moonray Well the point alex made earlier is: we want to not expose the information by default, at least not super visible. Most plugins should really not care about their location.

Overall this looks good but a lot of inconsistencies/typos in the comments.

1. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,162 @@
   +   * The suffix the file cache key.
   

   The suffix for the file cache key.

2. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,162 @@
   +   * The key which contains the value to key the data by.
   

   This is very confusing - could we replace it with something closer to the @param docs?

3. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,162 @@
   +   *   An array of directories to scan, keyed by the provider.
   

   This doesn't match the property documentation.

4. +++ b/core/lib/Drupal/Component/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,162 @@
   +   * This method is exists so that it is easy to replace this functionality in
   

   This method is exists.

5. +++ b/core/lib/Drupal/Core/Plugin/Discovery/YamlDirectoryDiscovery.php
   @@ -0,0 +1,35 @@
   + * Allows directories that contain YAML files to define plugin definitions.
   

   "Allows directories containing YAML files to define plugin definitions."

   The directories themselves don't define the plugin definitions.

Committed/pushed to 8.1.x, thanks!