Properly deprecate statistics_get()

The long @expectedDeprecation feels wrong, but it looks logically correct. 😉

Looks fine to me.

Ah yes, for backwards compatibility statistics_get returns an array, where as the newer service returns a StatisticsViewsResult object.

Except nit (which probably out of scope) it looks good

+++ b/core/modules/statistics/statistics.tokens.inc
@@ -40,25 +40,22 @@ function statistics_tokens($type, $tokens, array $data, array $options, Bubbleab
+        $replacements[$original] = \Drupal::service('date.formatter')->format($statistics->getTimestamp());

maybe move service access out of loop?

+++ b/core/modules/statistics/statistics.tokens.inc
@@ -40,25 +40,22 @@ function statistics_tokens($type, $tokens, array $data, array $options, Bubbleab
+    $statistics = \Drupal::service('statistics.storage.node')->fetchView($node->id());

This is fetching the statistics for the node up front, even if none of these tokens are used. We should just do a 1-1 replacement of the existing logic instead.

1. +++ b/core/modules/statistics/statistics.tokens.inc
   @@ -40,25 +40,22 @@ function statistics_tokens($type, $tokens, array $data, array $options, Bubbleab
    
      if ($type == 'node' & !empty($data['node'])) {
        $node = $data['node'];
   -
   +    /** @var \Drupal\statistics\StatisticsStorageInterface $stats_storage */
   +    $stats_storage = \Drupal::service('statistics.storage.node');
        foreach ($tokens as $name => $original) {
   

   Yeah, this is still a tiny overhead since most calls to this probably don't hav statistics tokens, but that's not going to be measurable.

2. +++ b/core/modules/statistics/tests/src/Functional/StatisticsTokenReplaceTest.php
   index 56e18eefad..556593fea6 100644
   --- a/core/modules/statistics/tests/src/Functional/Views/IntegrationTest.php
   
   --- a/core/modules/statistics/tests/src/Functional/Views/IntegrationTest.php
   +++ b/core/modules/statistics/tests/src/Functional/Views/IntegrationTest.php
   
   +++ b/core/modules/statistics/tests/src/Functional/Views/IntegrationTest.php
   @@ -78,11 +78,15 @@ public function testNodeCounterIntegration() {
   -    $expected = statistics_get($this->node->id());
   -    // Convert the timestamp to year, to match the expected output of the date
   -    // handler.
   -    $expected['timestamp'] = date('Y', $expected['timestamp']);
   -
   +    /** @var \Drupal\statistics\StatisticsViewsResult $statistics */
   +    $statistics = \Drupal::service('statistics.storage.node')->fetchView($this->node->id());
   +    $expected = [
   +      'totalcount' => $statistics->getTotalCount(),
   +      'daycount' => $statistics->getDayCount(),
   +      // Convert the timestamp to year, to match the expected output of the date
   +      // handler.
   +      'timestamp' => date('Y', $statistics->getTimestamp()),
   +    ];
        foreach ($expected as $field => $value) {
          $xpath = "//div[contains(@class, views-field-$field)]/span[@class = 'field-content']";
          $this->assertFieldByXpath($xpath, $value, "The $field output matches the expected.");
   

   Hm. Feels like it would actually be easier and less code to just drop the foreach and have 3 different xpath + assertFieldByXpath() lines? Was a neat trick when we had an array with (mostly) matching keys/values, but seems weird now.

   I also just noticed that there is a second loop below, but that just checks that the fields don't exist at all, so the specific value shouldn't matter and we could simplify that too? Multiple combined contains() for example like https://stackoverflow.com/questions/12152890/xpath-contains-one-of-multi...?

Works for me :)