This is a sub-issue of #1800046: [META] Add missing type hinting to core docblocks, fix Drupal.Commenting.FunctionComment.Missing* focused on correctly adding @param and @return type hinting to the Field_ui module.

Documentation patches that include type hinting are time consuming to both review and commit because one must dig into the actual code to confirm that the type hints are both correct and complete. Hence, please be patient and try to limit type hint patches to covering only a limited number of docblocks (20-25 as a guess).

How To Review This Issue

  1. Attempt to apply the patch to see if it needs a reroll.
  2. Use the phpcs one-liner to evaluate whether all the relevant standards errors have been resolved: https://gist.github.com/paul-m/227822ac7723b0e90647
  3. Look at each change and determine whether the type hint is correct.

Related sprint issues:

Sprint Topic Sub Issue
#1518116: [meta] Make Core pass Coder Review #1533234: Make field_ui module pass Coder Review
#1310084: [meta] API documentation cleanup sprint #1326638: Clean up API docs for field_ui module
#500866: [META] remove t() from assert message #1797170: Remove t() from assertion messages in tests for field_ui.module
CommentFileSizeAuthor
#20 1816782-20.patch4.28 KBshetpooja04
#6 1816782-6.patch6.55 KBnaveenvalecha
#2 1816782_2.patch8.49 KBmile23

Comments

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Parent issue: » #1800046: [META] Add missing type hinting to core docblocks, fix Drupal.Commenting.FunctionComment.Missing*
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 1816782_2.patch8.49 KB

Patch-o-la.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Still applies.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Needs review » Needs work
Issue tags: +Needs reroll

Bumping to 8.1.x. Patch no longer applies.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 1816782-6.patch6.55 KB

Admins-MacBook-Pro% phpcs --standard="Drupal" --extensions="module/php,php" --report-csv core/modules/field_ui | grep -F $'Missing param\nReturn type missing'

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.1.x-dev » 8.0.x-dev
Status: Needs review » Reviewed & tested by the community

As per @jhodgdon on the other similar issues, I'm setting this back to 8.0.x.

The patch still applies, and the hints look correct. phpcs says everything in the module has a hint.

catch’s picture

catch
he/him
Primary language English
commented
Component: field_ui.module » documentation
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Sorry, but I think this could use some more work:

  1. +++ b/core/modules/field_ui/field_ui.api.php
@@ -60,7 +60,7 @@ function hook_field_formatter_third_party_settings_form(\Drupal\Core\Field\Forma
- * @see \Drupal\field_ui\FormDisplayOverView.
+ * @see \Drupal\field_ui\FormDisplayOverView

    Technically, removing the . from @see in this patch is out of scope. Let's just take that out of the patch, and possibly file another issue to take care of this all over core. 

  2. +++ b/core/modules/field_ui/field_ui.api.php
@@ -110,7 +110,7 @@ function hook_field_formatter_settings_summary_alter(&$summary, $context) {
- * @see \Drupal\field_ui\FormDisplayOverView.
+ * @see \Drupal\field_ui\FormDisplayOverView

    Out of scope, see above. 

  3. +++ b/core/modules/field_ui/src/Tests/ManageDisplayTest.php
@@ -428,15 +428,15 @@ function assertNodeViewText(EntityInterface $node, $view_mode, $text, $message)
+   * @param \Drupal\Core\Entity\EntityInterface $node
    *   The node.

    I don't think EntityInterface is the best type hint for this $node parameter. Should be a node-specific interface? 

  4. +++ b/core/modules/field_ui/src/Tests/ManageDisplayTest.php
@@ -449,18 +449,18 @@ function assertNodeViewNoText(EntityInterface $node, $view_mode, $text, $message
+   * @param \Drupal\Core\Entity\EntityInterface $node
    *   The node.

    Same here.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Just a heads-up that the phpcs code snippet to review the patch has changed. It wasn't checking .inc files. See the gist here: https://gist.github.com/paul-m/227822ac7723b0e90647

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

:-) #2662208: Fix @see documentation in core

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

shetpooja04’s picture

shetpooja04 commented
Status: Needs work » Needs review
StatusFileSize
new 1816782-20.patch4.28 KB

I have uploaded patch with changes comparing from #6 in 8.9.x version
Few files already had changes so not including those in the patch
and also please mention whether to implement 3rd and 4th point from #9

used this to check
phpcs --standard="Drupal" --extensions="module/php,inc/php,php" --report-csv core/modules/field_ui | grep -F $'Missing param\nReturn type missing'

Please review !

quietone’s picture

quietone commented
Status: Needs review » Closed (duplicate)
Related issues: +#3207949: Fix Drupal.Commenting.FunctionComment.MissingParamType

This work is now being done by sniff. The work here is now in #3207949: Fix Drupal.Commenting.FunctionComment.MissingParamType, so closing this as a duplicate. I've identified credit to add over there, let me know if I got it wrong.