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
Comment #2
xjmComment #4
quietone commentedThe 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."
Comment #5
smustgrave commentedNot sure best to review but what @quietone says makes sense.
Comment #6
quietone commentedFor completeness, the decision from the related is documented at https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...
Comment #7
catchYeah I think this is a duplicate of #3183180: [policy, no patch] Document named arguments BC policy. Going to close as outdated.
Comment #8
quietone commentedChanging to latest version when this was closed.
Comment #9
quietone commented