Document how to add additional parameters to interface methods

https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...

However, we reserve the ability to add methods to these interfaces in minor releases to support new features.

You mentioned adding methods in the issue summary, which is why I commented with that.

If we can do #3354595: Remove the debug classloader deprecation silencing for Drupal interfaces then I think the next step would be to add docs to https://www.drupal.org/about/core/policies/core-change-policies/bc-policy on how to use it to add parameters to interface methods.

For adding/changing type hints we could probably do something like removing a parameter then adding it back again, but should be separate issues probably.

adding a method to the interface
removing a method from the interface

These are already covered in the existing bc policy, IMO we should not bundle things into one issue but handle one at a time where there are deficiencies.

removing parameter(s) from existing method

this is also already covered by the existing bc policy.

#3355839: Prepare FormBuilder for variadic functions did exactly what we need to document here, so I think the next step is adding that as an example to the bc policy at https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...

Ok, to get this rolling i attacked a proposed addition to the core page on deprecations.

And readable text:
​

Adding arguments to interface methods

Leveraging on Symfony's DebugClassLoader, it's now possible to add arguments to methods in interfaces in next major release.

Since it's normally a BC break to introduce new arguments (because any implementing class that does not comply with the changed signature would fail at loading), this is a two steps process.

Step 1 - prepare the new signature

During the current release cycle, introduce the new argument(s) in an inline comment, for example:

-  public function submitForm($form_arg, FormStateInterface &$form_state);
+  public function submitForm($form_arg, FormStateInterface &$form_state /* , mixed ...$args */);

Document the new argument(s) in the docblock of the method, inside an PHPCS ignore block to prevent PHPCS to fire an error when parsing the signature of the method; in the comments include a @see to a follow-up issue that will take care of actually doing the change in the next major release:

    * phpcs:disable Drupal.Commenting
    * @todo Uncomment new method parameters before drupal:11.0.0.
    * @see https://www.drupal.org/project/drupal/issues/3354672
    *
    * @param mixed ...$args
    *   Any additional arguments are passed on to the functions called by
    *   \Drupal::formBuilder()->getForm(), including the unique form constructor
    *   function. For example, the node_edit form requires that a node object is
    *   passed in here when it is called. These are available to implementations
    *   of hook_form_alter() and hook_form_FORM_ID_alter() as the array
    *   $form_state->getBuildInfo()['args'].
    * phpcs:enable

Tag the follow-up issue with 'Major version only'.

This will let the testing framework trigger deprecation errors for the implementing classes that do not have, yet, the to-be signature in place. In order to prevent tests fail because of that, add an ignore line in .deprecation-ignore.txt, like e.g.

# Drupal 11.
%FormBuilder::getForm\(\).* will require a new "mixed \.\.\. \$args" argument in the next major version of its interface%
%FormBuilder::submitForm\(\).* will require a new "mixed \.\.\. \$args" argument in the next major version of its interface%

Step 2 - implement the new signature

Once the branch opens for issues for new major release issues:

1. Remove the inline comment from the interface, exposing the new full signature:

2. -  public function submitForm($form_arg, FormStateInterface &$form_state /* , mixed ...$args */);
   +  public function submitForm($form_arg, FormStateInterface &$form_state, mixed ...$args);
   

3. remove the PHPCS ignore and @todo from the docblock,
4. implement the new signature in the concrete classes
5. remove the ignore line from the .deprecation-ignore.txt file.

Updated covered bullet points as pointed out by catch

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Suggestion by @catch is to use something like:

-  public function foo(OldInterface $bar)
+  public function foo(/** OldInterface|NewInterface $bar */)

For changing interface typehints. Was trying to write this, but need to check it with some real code. Also not sure if if this would need to be an example as above of perhaps:

-  public function foo(array $bar)
+  public function foo(/** NewInterface $bar */)

That perhaps feels a little more realistic in context of Drupal code.

Ok, lets see, this seems like an good idea (tm). Added example for changing method signature:

Adding arguments or changing typehints of interface methods

Leveraging on Symfony's DebugClassLoader, it's now possible to add arguments to and change signatures of methods in interfaces in next major release.

Since it's normally a BC break to introduce new arguments (because any implementing class that does not comply with the changed signature would fail at loading), this is a two steps process.

Step 1 - prepare the new signature

During the current release cycle, introduce the new argument(s) in an inline comment, for example:

-  public function submitForm($form_arg, FormStateInterface &$form_state);
+  public function submitForm($form_arg, FormStateInterface &$form_state /* , mixed ...$args */);

Or change the typehint of an argument, for example:

-  public function foo(string $bar);
+  public function foo(/* string|Stringable $bar */);

Document the changes in the docblock of the method, inside an PHPCS ignore block to prevent PHPCS to fire an error when parsing the signature of the method; in the comments include a @see to a follow-up issue that will take care of actually doing the change in the next major release:

    * phpcs:disable Drupal.Commenting
    * @todo Uncomment new method parameters before drupal:11.0.0.
    * @see https://www.drupal.org/project/drupal/issues/3354672
    *
    * @param mixed ...$args
    *   Any additional arguments are passed on to the functions called by
    *   \Drupal::formBuilder()->getForm(), including the unique form constructor
    *   function. For example, the node_edit form requires that a node object is
    *   passed in here when it is called. These are available to implementations
    *   of hook_form_alter() and hook_form_FORM_ID_alter() as the array
    *   $form_state->getBuildInfo()['args'].
    * phpcs:enable

Tag the follow-up issue with 'Major version only'.

This will let the testing framework trigger deprecation errors for the implementing classes that do not have, yet, the to-be signature in place. In order to prevent tests fail because of that, add an ignore line in .deprecation-ignore.txt, like e.g.

# Drupal 11.
%FormBuilder::getForm\(\).* will require a new "mixed \.\.\. \$args" argument in the next major version of its interface%
%FormBuilder::submitForm\(\).* will require a new "mixed \.\.\. \$args" argument in the next major version of its interface%

Step 2 - implement the new signature

Once the branch opens for issues for new major release issues:

1. Remove the inline comment from the interface, exposing the new full signature:

2. -  public function submitForm($form_arg, FormStateInterface &$form_state /* , mixed ...$args */);
   +  public function submitForm($form_arg, FormStateInterface &$form_state, mixed ...$args);
   

   or

   -  public function foo(/* string|Stringable $bar */);
   +  public function foo(string|Stringable $bar);
   
   

3. remove the PHPCS ignore and @todo from the docblock,
4. implement the new signature in the concrete classes
5. remove the ignore line from the .deprecation-ignore.txt file.

Updated IS with proposed solution

Minor wording changes.

Update propopsed solution to only used fake classes/interfaces/methods/arguments

Talked with @bbrala about this in slack, I think the proposed wording is really clear, and covers as many cases as we could think of, so ready to go for me.

RTBC and tagging for documentation updates.

Documentation has been updated.

Docs changes look good but I wonder if we should move it directly under https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... since it's dealing with parameters?

Hmm, both could be right, the placement right now is classes -> interfaces.

I don't mind either way, both make sense in my mind.

> %FormBuilder::getForm\(\).* will require a new "mixed \.\.\. \$args" argument in the next major version of its interface%

Is the idea that implementations/subclasses can add the parameters as optional right away, then then they're compatible with both D10 (without the param) and D11 (with the param)?

I'm assuming this message isn't under our control -- but it would be nice if it said that or if we documented that somewhere.

Think you might wanna look at the is, that's close to what was actually posted. Does the doc page still show that form example? Thought I generalised it.

AFAICT, the IS doesn't explain how implementors should react to the deprecation.

And the IS says how to ignore the deprecation warning in a test, but it doesn't say where the warning is generated.

The warning is generated by Symfony's debug classloader (and this is why we have no control over the contents).

Implementors should add the extra parameter or type hint to their current codebase - PHP won't complain and it'll be forwards compatible for when the interface changes in the next major release.

I think that #27 and #28, about the placement of the new section on the page, has been resolved. xjm moved it to it's own heading, added the links to the tag and other tidying up. Adding credit for xjm for that work.

The new addition is at Interface method signature changes.

Thanks everyone!

Apologies in advance for the issue necromancy, but I noticed in reviewing an issue this past week that we're disabling the entire commenting sniff on methods where we use this approach. That seems a bit dangerous. @mondrake and I are exploring a more targeted approach in #3520730: Fix PHPStan arguments.count error related to interface argument additions in the next major.

Yes, adding the specific sniffs to ignore is an improvement. Instead of re-opening this lets do that in a new issue. I think that will help to make it easier to see the steps/process used to develop the documentation.

The new issue is #3531685: Ignore specifc sniffs when adding additional parameters to interface methods