Remove unused local variables from the node module

All is well.

Actually, in this case, I wonder if this is actually highlighting a missing test below? And if not, then we should adjust the comment above because we are no longer preserving timestampS.

Simply removing the s would resolve #6

    // Store the timestamp.
    $created = $node->getCreatedTime();

    $node->save();

A patch matching #8 is attached

Patch re-rolled.

Please check that all other unused local variables are removed from the node module as well, and add those to this patch.

@xjm, I'll be looking into other unused variables. I missed out your comment before uploading patch.

Adding patch for test queue.

Thanks @rahul.shinde!

@xjm, Adding patch to remove/update unused variables for node module.

Works. Thank you very much!

Thanks; this is close... however, it's not clear to me why 979534800 was chosen as the comparison value here:

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeSaveTest.php
@@ -94,6 +94,7 @@ function testTimestamps() {
     $this->assertEqual($node->getCreatedTime(), $created, 'Updating a node preserves "created" timestamp.');
+    $this->assertNotEqual(979534800, $changed, 'Updating a node does not preserve "changed" timestamp.');

Does replacing 979534800 with $node->getUpdatedTime() per stevepurkiss in #11 not work?

I've changed the "979534800" line of code as it isn't actually testing anything.

Problem is this check:

$this->assertEqual($node->getCreatedTime(), $created, 'Updating a node preserves "created" timestamp.');

this check can't be performed here, because preSave uses REQUEST_TIME for 'changed' property update and this constant does not change during request.

  public function preSave(EntityStorageControllerInterface $storage_controller) {
    parent::preSave($storage_controller);

    // Before saving the node, set changed and revision times.
    $this->changed->value = REQUEST_TIME;
  }

So we need to move this check to another test or change logic of the preSave() method and use time() instead.

In the attached patch I simply removed this check.

Oops, I've attached wrong files.

node-remove-unused-variables-2064639-35.patch no longer applies.

error: patch failed: core/modules/node/lib/Drupal/node/Tests/NodeTokenReplaceTest.php:44
error: core/modules/node/lib/Drupal/node/Tests/NodeTokenReplaceTest.php: patch does not apply

rerolled

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTokenReplaceTest.php
@@ -67,6 +67,7 @@ function testNodeTokenReplacement() {
     $node->save();
 
+
     // Generate and test sanitized tokens.

Not need for this empty line :)

Sorry I didn't notice the extra line, I fixed it and added interdiff

Thanks Salah. I have tested patch and its working fine for me.

Rerolled. This patch applies cleanly at commit 428d0f0.

A casualty of #2174997: Random fail in ImageFieldDisplayTest

44: 2064639-remove-unused-vars-node-44.patch queued for re-testing.

I have removed unused variables from the following files:

File core/modules/node/lib/Drupal/node/NodeFormController.php
Line 325: Unused variable $account.

File core/modules/node/lib/Drupal/node/NodeFormController.php
Line 325: Unused variable $account.

File core/modules/node/lib/Drupal/node/Tests/NodeTypePersistenceTest.php
Line 32: Unused variable $forum_disable.

File core/modules/node/node.tokens.inc
Line 136: Unused variable $field_langcode.

providing interdiff .. all looks ok maybe other committed patch has fix identical issues.

one more ?
Location      file [drupal8x] - ...\core\modules\node\lib\Drupal\node\Controller\NodeController.php   Problem synopsis      Unused local variable $uri (at line 132)

Removed Unused local variable $uri (at line 132) at core\modules\node\lib\Drupal\node\Controller\NodeController.php.

Patch no longer applies, tagging for reroll.

reroll ( Change: The removed variable in NodeController.php has already beeen removed in HEAD )

Patch applied cleanly.
Rerolled done.

Re-rolled because of:
PSR-4
#2116363: Unified repository of field definitions (cache + API) removed one of the unused variables a previous patch removed in node.token.inc
#2238077: Rename EntityFormController to EntityForm renamed one of the files that had an unused variable (NodeFormController.php to NodeForm.php)

I'm unsure why but phpStorm doesn't say that $account is an unused variable although it clearly is and was removed with a previous patch. This line is in NodeForm.php line 312 if anyone else wants to check it out.
- if (!empty($form_state['values']['uid']) && !($account = user_load_by_name($form_state['values']['uid']))) {
+ if (!empty($form_state['values']['uid']) && !user_load_by_name($form_state['values']['uid'])) {

All the unused variables in the node module seem to be removed now via phpStorm's inspect code on the node module.

My first patch at the Austin sprint!

I ran Inspect Code in PhpStorm on latest of 8.x. It found 2 unused local variables: $forum_enable, $created.

I applied the patch from #56 and ran Inspect Code again. This time it did not find any unused local variables. So, it looks to me that the patch works and does what it's supposed to.

Marking as RTBC.

Committed 5417fcf and pushed to 8.x. Thanks!