Document how to add additional parameters to interface methods

If we can do 📌 Remove the debug classloader deprecation silencing for Drupal interfaces Closed: duplicate 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.

Add to IS a list of cases to cover. Anything else?

In my view, #2 covers case 1 from the IS, #5 covers case 5.

Would be good to have a position for all the cases.

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.

📌 Prepare FormBuilder for variadic functions Fixed 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 📌 [11.x] Adjust parameters in interfaces Active
*
* @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:

Remove the inline comment from the interface, exposing the new full signature:
- public function submitForm($form_arg, FormStateInterface &$form_state /* , mixed ...$args */);
+ public function submitForm($form_arg, FormStateInterface &$form_state, mixed ...$args);

remove the PHPCS ignore and @todo from the docblock,
implement the new signature in the concrete classes
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:

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%

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

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.

quietone → credited xjm → .

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!

Automatically closed - issue fixed for 2 weeks with no activity.