PluginFormInterface must document $form means the plugin's own elements

This change seems OK but I'm not convinced it solves the problem stated in the issue summary. Maybe in addition to calling it "the plugin form", you could add a few words to indicate it is possibly just a small piece of a larger form?

Actually though, why would that be the case?

And it seems to me that if $form_state->getValue* doesn't work, that is a larger problem than just plugin forms. I'm sure we have this assumption all over core for form builders. None of them will work if the form is embedded elsewhere, so it sounds like no form should make this assumption?

OK, fair enough.

So in order to really document this well, so that this type of problem will not happen in the future, what I would suggest is:

a) Put something into the FormStateInterface methods where you need to pass in the parents that says something like:

If you are calling this from an embeddable form (a form meant to be embedded in a larger form), you must pass in (whatever the parents parameter is called).

b) In PluginFormInterface in the class docs header, document that plugin forms are meant to be embedded and that you need to be careful about assumptions when you get values out of the $form_state.

c) I still think that the patch you've submitted here doesn't do enough for those methods either... My suggestion would be instead of just changing the docs on those methods for the $form argument from saying "form" to "plugin form", let's say "embeddable plugin form", and then on $form_state, let's say "for the entire form (including the embedded plugin form)" or something like that to get across the point that $form is just the plugin portion, and $form_state is the entire form state.

Does that make sense?

Thanks for the new patches! I think this still needs some work though:

1. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -10,18 +10,22 @@
   + * Provides an interface for a plugin that contains a embeddable form.
   

   a embeddable -> an embeddable

   Also this one line doc just seems wrong. As it reads now, it says it is an interface for a plugin, but I think it is actually an interface for a plugin *form* right? Or maybe it's a plugin configuration form really? I think that is what it actually is.

   So the first line needs a rewrite.

2. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -10,18 +10,22 @@
   + * to have these properties available when building the plugin form's elements,
   + * let buildConfigurationForm() return a form element that has a #process
   + * callback and build the rest of the form in the callback. By the time the
   + * callback is executed, the element's #parents and #array_parents properties
   

   A code example would be helpful here, or it could go into the buildConfigurationForm() method. Actually, most of this documentation is specific to buildConfigurationForm() so maybe it should go there instead?

3. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -31,7 +35,7 @@
       * Form constructor.
       *
       * @param array $form
   -   *   An associative array containing the structure of the form.
   +   *   An associative array containing the initial structure of the plugin form.
       * @param \Drupal\Core\Form\FormStateInterface $form_state
   

   Maybe in this method, make sure that again it is stressed that #array_parents and #parents are not set in $form?

4. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -31,7 +35,7 @@
       * @param \Drupal\Core\Form\FormStateInterface $form_state
       *   The current state of the form.
   

   Why would it be so terrible on the $form_state docs here to add the word "entire" so that it is clearer that $form_state and $form are not referring to the same thing?

5. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -44,9 +48,12 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   +   *   An associative array containing the structure of the plugin form as built
   +   *   by static::buildForm().
       * @param \Drupal\Core\Form\FormStateInterface $form_state
   

   Um, is it really buildForm() or buildConfigurationForm()?

6. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -44,9 +48,12 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   +   *   The current state of the form. The values submitted for the plugin form
   

   Again, could we say "state of the entire form" here?

7. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -44,9 +48,12 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   +   *   \Drupal\Component\Utility\NestedArray::getValue($form_state->getValues(), $form['#parents']);
   

   Any way to split this line to less than 80 characters? Also probably should use @code ... @endcode since this is not just one function name but a code example.

8. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -54,9 +61,12 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
   +   *   by static::buildForm().
   

   again, is this really from buildForm() or buildConfigurationForm()?

9. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -54,9 +61,12 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
   +   *   The current state of the form. The values submitted for the plugin form
   

   Again, could we say "entire form" here?

10. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
    @@ -54,9 +61,12 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
    +   *   \Drupal\Component\Utility\NestedArray::getValue($form_state->getValues(), $form['#parents']);
    

    Again, use @code / @endcode

Also, just as a note, our coding standards for docs say that when referring to a class/method in docs, you put in the entire namespace and class name (or in this case, interface name) rather than using shortcuts like static::methodName().

I like it! Just a couple of nitpicks and one substantive question/comment...

1. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -30,10 +19,25 @@
   +   * to have these properties available when building the plugin form's elements,
   

   nitpick: this crosses 80 characters.

2. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -30,10 +19,25 @@
   +   * let buildConfigurationForm() return a form element that has a #process
   

   If this is staying here (see next comment), maybe say "this method" instead of "buildConfigurationForm()" ?

3. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -30,10 +19,25 @@
   +   * During validation and submission, the plugin form's elements' #parents and
   +   * #array_parents properties have already been populated and can be used
   +   * directly to find the location of the submitted form values in the form's
   +   * state.
   +   *
   

   Well, the problem with moving this whole block of docs to the buildConfigurationForm() method doc block is that this part is not related to this method, so it doesn't belong on this method doc... not sure what to do? I guess on the validate/submit methods, you could say something like "Unlike on the build method, ..." or something?

4. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -44,9 +48,14 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
   +   *   The current state of the complete form. The values submitted for the plugin form
   

   over 80 chars, rewrap

5. +++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
   @@ -54,9 +63,14 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
   +   *   The current state of the complete form. The values submitted for the plugin form
   

   80 chars

Now, this looks good and I think it accomplishes the goals of this issue: getting across to the developer that they need to be careful with this stuff. !! Great!

+++ b/core/lib/Drupal/Core/Plugin/PluginFormInterface.php
@@ -44,9 +44,14 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
+   *   The current state of the complete form. The values submitted for the
+   *   plugin form can be retrieved through
+   *   @code
+   *   \Drupal\Component\Utility\NestedArray::getValue($form_state->getValues(), $form['#parents']);
+   *   @endcode

@@ -54,9 +59,14 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
+   *   The current state of the complete form. The values submitted for the
+   *   plugin form can be retrieved through
+   *   @code
+   *   \Drupal\Component\Utility\NestedArray::getValue($form_state->getValues(), $form['#parents']);
+   *   @endcode

Please leave this out until #2536646: [meta] PluginFormInterface implementations assume they are top-level and friends are decided upon.

Also putting this into documentation prevents the subsystem maintainers from easily finding their issues.

Sorry, didn't realize it was controversial or I definitely would have put it into that component.

I meant this.

This seems acceptable to me. Provisionally setting to RTBC unless Xano objects.

Docs are not frozen in beta. Committed b116b58 and pushed to 8.0.x. Thanks!