Problem/Motivation

In #3185640: Fix or ignore words that start with "v", excluding real non-English words, we have the following spelling error fix that was committed:

+++ b/core/modules/taxonomy/src/TermStorageInterface.php
@@ -116,16 +116,16 @@ public function resetWeights($vid);
-   * @param array $vocabs
-   *   (optional) A vocabularies array to restrict the term search. Defaults to
-   *   empty array.
+   * @param array $vids
+   *   (optional) an array of vocabulary IDs to restrict the term search.
+   *   Defaults to empty array.
    * @param string $langcode
    *   (optional) A language code to restrict the term search. Defaults to NULL.
    *
    * @return array
    *   An array of nids and the term entities they were tagged with.
    */
-  public function getNodeTerms(array $nids, array $vocabs = [], $langcode = NULL);
+  public function getNodeTerms(array $nids, array $vids = [], $langcode = NULL);

Using the abbreviation "vocabs" was already against our naming and documentation standards; cspell just surfaced the issue. However, there are definitely other reasons to rename parameters -- e.g., to convert them from camelCase to snake_case as part of bulk cleanups for constructor property promotion.

However, per @dpi on the linked issue:

Since PHP 8.0 parameter naming are a part of the API contract, as named arguments may be used by callers.

// Will break
\Drupal::entityTypeManager()->getStorage('taxonomy_term')->getNodeTerms(nids: [], vocabs: []);
As much as its unlikely to happen in this case, as it is the first non optional, it is still possible and will break usages. I've certainly made use of parameter names when its redundant, in the name of legibility.

How can we make sure parameters are not affected by these spelling issues?

Proposed resolution

Decide how to handle parameter renaming for PHP 8+.

If necessary, revert the change from TermStorageInterface before 11.1.0.

Remaining tasks

TBD

User interface changes

N/A

API changes

TBD

Data model changes

TBD

Release notes snippet

TBD

Comments

xjm created an issue. See original summary.

xjm’s picture

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Status: Active » Needs review

The related issue concluded by agreeing that "Named arguments should not be considered part of the public API." Since that is the case it may not be necessary to make a policy decision here. Instead, any renaming would be evaluation according to the existing guidelines, which state, "A reasonable effort will be made to keep the API stable and provide a backwards compatible (BC) layer. Wherever possible, old APIs will be deprecated for removal in the next major version instead of being removed immediately."

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Not sure best to review but what @quietone says makes sense.

quietone’s picture

For completeness, the decision from the related is documented at https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...

catch’s picture

Status: Reviewed & tested by the community » Closed (outdated)

Yeah I think this is a duplicate of #3183180: [policy, no patch] Document named arguments BC policy. Going to close as outdated.

quietone’s picture

Version: 11.x-dev » 11.2.x-dev

Changing to latest version when this was closed.

quietone’s picture