Change EntityInterface type hints to NodeInterface where appropriate

Basically yes, but it might make sense to start somewhere, e.g. node.module. and first change those and check the test results.

The reason is that we want to have a specific type hint so that IDE's can autocomplete on the methods. We already had them, but had to change it due to do something that since has been changed again, the way translations are handled. See #1391694: Use type-hinting for entity-parameters for one related issue.

+++ b/core/modules/node/tests/modules/node_test/node_test.module
@@ -159,7 +158,7 @@ function node_test_node_update(EntityInterface $node) {
-function node_test_entity_view_mode_alter(&$view_mode, Drupal\Core\Entity\EntityInterface $entity, $context) {
+function node_test_entity_view_mode_alter(&$view_mode, Drupal\node\NodeInterface $entity, $context) {

This is an $entity, not a node, so this one shouldn't be replaced.

This looks great already. This overlaps with a few big issues (page callback conversions, node BC removal, converting search to plugins) that will remove or touch some of those functions. Those are currently more important to get in, so this will require a re-roll after they are.

You might want to follow #1939994: Complete conversion of nodes to the new Entity Field API, #2003482: Convert hook_search_info to plugin system and the not yet committed, node related issues in #1971384: [META] Convert page callbacks to controllers and wait with a re-roll until they landed.

The blockers are through now, would be nice to finish this :)

Thanks! (remember to set issues to needs review when you upload a patch)

node hooks work with node entities. Passing anything else to them is not valid. So the type hint should be on NodeInterface. Otherwise, modules would have to add an if ($node instanceof NodeInterface) to their code to be able to rely on methods provided by that interface. There are separate hook_entity_* hooks that are invoked for all entity types, the point of hook_node_* (hook_$ENTITY_TYPE_) is that they only receive entities of a specific type.

1. +++ b/core/modules/node/lib/Drupal/node/NodeAccessController.php
   @@ -87,7 +87,7 @@ public function createAccess($entity_bundle = NULL, AccountInterface $account =
       * {@inheritdoc}
       */
   -  protected function checkAccess(EntityInterface $node, $operation, $langcode, AccountInterface $account) {
   +  protected function checkAccess(NodeInterface $node, $operation, $langcode, AccountInterface $account) {
   

   You can't change it here, becauseEntityInterface is enforced by the parent class.

2. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
   @@ -93,40 +92,40 @@ public function buildHeader() {
       * {@inheritdoc}
       */
   -  public function buildRow(EntityInterface $entity) {
   +  public function buildRow(NodeInterface $node) {
   

   Same here, everywhere where it says "@ineheritdoc" means that it's enforced by a parent class or interface to be EntityInterface.

3. +++ b/core/modules/node/lib/Drupal/node/NodeTranslationController.php
   @@ -18,8 +17,8 @@ class NodeTranslationController extends ContentTranslationController {
      /**
       * Overrides ContentTranslationController::entityFormAlter().
       */
   -  public function entityFormAlter(array &$form, array &$form_state, EntityInterface $entity) {
   -    parent::entityFormAlter($form, $form_state, $entity);
   +  public function entityFormAlter(array &$form, array &$form_state, NodeInterface $node) {
   +    parent::entityFormAlter($form, $form_state, $node);
   

   Or an outdated Overrides/Implements docblock in a class method.

Finding weird things in the documentation but that's not the problem of this issue. Thanks, looks good!

1. +++ b/core/modules/node/node.api.php
   @@ -546,7 +545,7 @@ function hook_node_load($nodes) {
     * @link node_access Node access rights @endlink for a full explanation.
     *
   - * @param \Drupal\Core\Entity\EntityInterface|string $node
   + * @param \Drupal\node\NodeInterface|string $node
     *   Either a node entity or the machine name of the content type on which to
     *   perform the access check.
   

   This isn't true anymore I think, but let's not touch that there.

2. +++ b/core/modules/node/node.api.php
   @@ -766,7 +765,7 @@ function hook_node_validate(\Drupal\Core\Entity\EntityInterface $node, $form, &$
     *
     * @ingroup node_api_hooks
     */
   -function hook_node_submit(\Drupal\Core\Entity\EntityInterface $node, $form, &$form_state) {
   +function hook_node_submit(\Drupal\node\NodeInterface $node, $form, &$form_state) {
      // Decompose the selected menu parent option into 'menu_name' and 'plid', if
      // the form used the default parent selection widget.
      if (!empty($form_state['values']['menu']['parent'])) {
   

   Didn't we kill that stuff? :) Again, not our problem here.

+++ b/core/modules/node/node.module
@@ -217,13 +218,13 @@ function node_entity_form_display_alter(EntityFormDisplayInterface $form_display
  * Entity URI callback.
  *
- * @param \Drupal\Core\Entity\EntityInterface $node
+ * @param \Drupal\Core\Entity\NodeInterface $node
  *   A node entity.
  *

This doesn't exist.

Also not 100% if we should change this, if this would be a default method on the entity class, we wouldn't be allowed to do so.

Otherwise the re-roll looks fine.

Thanks.