Contrib.social

[policy, no patch] Decide how to handle parameter renaming now that PHP 8 allows it to be used as an API

Open on Drupal.org →
Created on 20 January 2023, over 2 years ago

Problem/Motivation

In 📌 Fix or ignore words that start with "v", excluding real non-English words Fixed , 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

📌 Task
Status

Active

Version

10.1 ✨

Component
Other  →

Last updated about 2 hours ago

Created by

🇺🇸United States xjm

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024