Convert $node->nid to $node->id() and $node->isNew()

You should not simply even try to review that via dreditor.

+++ b/core/modules/book/book.moduleundefined
@@ -307,7 +307,7 @@ function book_form_node_form_alter(&$form, &$form_state, $form_id) {
+    if (user_access('add content to books') && ((!empty($node->book['mlid']) && !$node->isNew()) || book_type_is_allowed($node->type))) {

Both isset($node->nid) and !empty($node->nid) should be converted to the same statement. It feels like just $node->id() is the proper thing to do

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/row/Rss.phpundefined
@@ -167,7 +167,7 @@ function render($row) {
-    $item->nid = $node->nid;
+    $item->id = $node->nid;

This change is potentially wrong. Shouldn't that be $item->nid = $node->id()?

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeAccessFieldTest.phpundefined
@@ -90,12 +90,12 @@ function testNodeAccessAdministerField() {
+    $this->drupalGet("node/" . $node->id());

Let's use single quotes as well.

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.phpundefined
@@ -434,7 +440,7 @@ function assertContentByXPath($xpath, array $arguments = array(), $value = NULL,
-  function assertLanguageSwitchLinks($node, $translation, $find = TRUE, $types = NULL) {
+  function assertLanguageSwitchLinks(NodeInterface $node, $translation, $find = TRUE, $types = NULL) {

@@ -462,7 +468,7 @@ function assertLanguageSwitchLinks($node, $translation, $find = TRUE, $types = N
-      if (!empty($translation->nid)) {
+      if ($translation->nid) {

+++ b/core/modules/translation/translation.moduleundefined
@@ -205,13 +205,13 @@ function translation_form_node_form_alter(&$form, &$form_state) {
-        if ($translation->nid != $node->nid) {
+        if ($translation->nid != $node->id()) {

So translation is also of type NodeInterface according to the clones above. Let's fix the documentation of assertLanguageSwitchLinks as well.

Both isset($node->nid) and !empty($node->nid) should be converted to the same statement. It feels like just $node->id() is the proper thing to do

I propose sticking to isNew(). Checking id() might be faster, but it was never made for this purpose, so we may set a wrong example by using it instead of isNew().

+++ b/core/modules/book/book.moduleundefined
@@ -447,11 +447,11 @@ function _book_add_form_elements(&$form, &$form_state, EntityInterface $node) {
+  $nid = $node->id() ? $node->id() : 'new';

At least this instance would make sense, most of the other ones actually care more about the id()

out of scope:

-  $this->assertBreadcrumb(NULL, $trail, $parent->title, $tree);
+  $this->assertBreadcrumb(NULL, $trail, $parent->label(), $tree);

We can add a separate issue for the remaining title -> label conversions.

Nice!

+++ b/core/modules/translation/translation.moduleundefined
@@ -566,7 +566,9 @@ function translation_language_switch_links_alter(array &$links, $type, $path) {
-        $links[$langcode]['href'] = 'node/' . $translations[$langcode]->nid . $matches[2];
+        // @todo: Unify translation objects.
+        $nid = $translations[$langcode] instanceof NodeInterface ? $translations[$langcode]->id() : $translations[$langcode]->nid;
+        $links[$langcode]['href'] = 'node/' . $nid  . $matches[2];

Asked berdir about this on IRC... he said "Looking at the code, I don't see how it could possibly ever be a NodeInterface"

Good catch in #16! This interdiff looked fine.

Committed ad13753 and pushed to 8.x. Thanks!