Looking at Drupal\node\Tests\NodeTokenReplaceTest from the perspective of #2226533: Changes to the Language class due to the LanguageInterface (followup)

This has highlighted a variable with an unfortunate variable name...

Consider this snippet of code from NodeTokenReplaceTest::testNodeTokenReplacement()

    foreach ($tests as $input => $expected) {
      $output = $this->tokenService->replace($input, array('node' => $node), array('langcode' => $this->languageInterface->id, 'sanitize' => FALSE));
      $this->assertEqual($output, $expected, format_string('Unsanitized node token %token replaced.', array('%token' => $input)));
    }

$this->languageInterface->id attempts to dynamically pull a property from a variable designated as an interface.
( Only constants can reasonably be accessed from an interface )

Other issue in the queue are trying to change this to ->id to ->getId()
see #2226533: Changes to the Language class due to the LanguageInterface (followup)
but that patch is already to unwieldy for this side issue.

I will post a patch shortly

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107’s picture

Issue summary: View changes
martin107’s picture

Status: Active » Needs review
FileSize
9.88 KB

Locally testing of

TokenReplacementUnitTest.php AND
NodeTokenReplaceTest.php

shows both are green

For reviewing purposes I have done nothing more than a localised replacement of $languageInterface for $language.

As a relative drupal newbie can you please advise as to wheather this should have a QuickFix tag .. I would like this to be committed before its parent issue.
( I will jump on the reroll this causes )

YesCT’s picture

looking at TokenReplaceUnitTestBase
I see the doc on that var is:
* The interface language.
so it is definitely a language and not a language interface.

I think interfaceLanguage might be a better name than just language .. if it is really the interface language (could be related to language negotiation and context stuff).

YesCT’s picture

Issue tags: +Quick fix
FileSize
10.06 KB

same patch but with interfaceLanguage
(done with a phpstorm refactor rename)

YesCT’s picture

Issue tags: +D8MI
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

perfect

sun’s picture

+1 also makes sense to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit a0f1611 on 8.x by webchick:
    Issue #2270339 by YesCT, martin107:  is a misleading variable name in...

Status: Fixed » Closed (fixed)

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

martin107’s picture

Assigned: martin107 » Unassigned