Problem/Motivation

In core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php providerTestSetLinkActiveClass() initializes $markup, but it never uses it.

Proposed resolution

Remove $markup.

        'query' => [],
     ];
     // Nothing to do.
-    $markup = '<foo>bar</foo>';
     $situations[] = ['context' => $context, 'is active' => FALSE, 'attributes' => []];
     // Matching path, plus all matching variations.
     $attributes = [
CommentFileSizeAuthor
unsused-variable-removed.patch692 byteshardik_patel_12

Comments

Hardik_Patel_12 created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

$markup is never used, but I am not sure '<foo>bar</foo>' wasn't supposed to be used as test markup.

adhershmnair’s picture

adhershmnair commented

$markup is an unused variable. I have reviewed and applied the patch #1 and it is working fine.

adhershmnair’s picture

adhershmnair commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

We still need to address #2 - this can be done by checking git log -S and/or git blame, and checking where the variable was added and whether any references to it were removed by mistake.

adhershmnair’s picture

adhershmnair commented
Status: Needs review » Reviewed & tested by the community

Seems the variable is used from initial commit of Issue #1979468

As per the issue Remove Unused local variables from system module
Here all other $markup = '<foo>bar</foo>'; has been removed from the file in this commit.

After these the filename changed to core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php from core/modules/system/tests/src/Unit/Controller/SystemControllerTest.php in the commit of issue Set the 'is-active' class for anonymous users in a Response Filter instead of a #post_render_cache callback

Also all instance of the $markup variable removed from the file.
As per the new implementation we don't want this local variable.

So it seems the $markup is an unused variable and can be removed. And the patch is verified and working for me.

RTBC!

  • catch committed 856ab5f on 9.1.x 
    Issue #3158290 by Hardik_Patel_12, adhershmnair: Unused local variables...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks for checking the history. Also read through in context and the $create_markup inline function appears to have replaced whatever this cruft was going to do.

Committed 856ab5f and pushed to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.