Convert tablesort.inc functions to a static class

1. +++ b/core/lib/Drupal/Core/Utility/TableSort.php
   @@ -0,0 +1,175 @@
   +  public function init(array $header) {
   +    $ts = $this->getOrder($header);
   +    $ts['sort'] = $this->getSort($header);
   +    $ts['query'] = $this->getQueryParameters();
   

   all this methods depends on request object so while refactoring this to service it makes sense to add request as argument to all this methods

2. +++ b/core/tests/Drupal/KernelTests/Core/Render/Element/TableSortExtenderTest.php
   @@ -32,7 +32,7 @@ public function testTableSortInit() {
        \Drupal::getContainer()->get('request_stack')->push($request);
   -    $ts = tablesort_init($headers);
   +    $ts = \Drupal::getContainer()->get('table_sort')->init($headers);
   
   @@ -45,7 +45,7 @@ public function testTableSortInit() {
        \Drupal::getContainer()->get('request_stack')->push($request);
   -    $ts = tablesort_init($headers);
   +    $ts = \Drupal::getContainer()->get('table_sort')->init($headers);
   
   @@ -61,7 +61,7 @@ public function testTableSortInit() {
        \Drupal::getContainer()->get('request_stack')->push($request);
   ...
   -    $ts = tablesort_init($headers);
   +    $ts = \Drupal::getContainer()->get('table_sort')->init($headers);
   

   Looking at the code I thing the request object should be passed to init() method and use fallback to request stack if no request object passed

1. -        if (empty($default) && isset($header['sort']) && ($header['sort'] == self::ASC || $header['sort'] == self::DESC)) {
   -          $default = $header;
   -        }
   +        if (empty($default) && isset($header['sort']) && in_array($header['sort'], [self::ASC, self::DESC])) {
   +          $default = $header;
   +        }
     

2.     public function getSort(array $headers, Request $request = NULL) {
   -    $query = $this->request($request)->query;
   -    if ($query->has('sort')) {
   -      return (strtolower($query->get('sort')) == self::DESC) ? self::DESC : self::ASC;
   -    }
   +    $query = $this->request($request)->query->all();
   +    if (!empty($query['sort'])) {
   +      return (strtolower($query['sort']) == self::DESC) ? self::DESC : self::ASC;
   +    }
   
   +    // The user has not specified a sort. Use the default for the currently
   +    // sorted header if specified; otherwise use "asc".
   -    else {
   +      // Find out which header is currently being sorted.
   +      $ts = $this->getOrder($headers, $request);
   +      foreach ($headers as $header) {
   -        if (is_array($header) && isset($header['data']) && $header['data'] == $ts['name'] && isset($header['sort'])) {
   -          return $header['sort'];
   -        }
   +        if (is_array($header) && !empty($header['data']) && $header['data'] == $ts['name'] && !empty($header['sort']) && in_array($header['sort'], [self::ASC, self::DESC])) {
   +          return $header['sort'];
   +        }
   +      }
   -    }
   +    return self::ASC;
   +  }
   

   No need else statement in this case.

Added draft CR https://www.drupal.org/node/3009182 and it needs deprecated tests

Patch does more clean-up of new implementation and I bet it needs follow-up to remove protected methods in \Drupal\Core\Database\Query\TableSortExtender and polish its docs

Implementation \Drupal\Core\Utility\TableSort::getOrder() coiul;d be polished more to have less then 10 Cyclomatic Complexity.

Also I think \Drupal\Core\Utility\TableSortInterface::getQueryParameters() method should not be a part of new interface - probably convert it to inline implementation

+++ b/core/includes/tablesort.inc
@@ -78,9 +53,15 @@ function tablesort_header(&$cell_content, array &$cell_attributes, array $header
 function tablesort_get_query_parameters() {
...
+  return \Drupal::service('table_sort')->getQueryParameters();

+++ b/core/lib/Drupal/Core/Database/Query/TableSortExtender.php
@@ -78,10 +98,10 @@ protected function getSort() {
   protected function getQueryParameters() {
-    return tablesort_get_query_parameters();
+    return $this->tableSort()->getQueryParameters();

+++ b/core/modules/views/views.theme.inc
@@ -449,7 +449,7 @@ function template_preprocess_views_view_table(&$variables) {
-  $query = tablesort_get_query_parameters();
+  $query = \Drupal::service('table_sort')->getQueryParameters();

the only usage

I went ahead and did clean-up cos this methods just duplicates service's ones

Now it looks better but now the most cruel thing left - naming!

I'd renaned init to more understandable getFromRequest()or getContext()

Also getQueryParameters() probably should be removed or marked as @internal

CR updated & looks good to go

This looks great! couple of minor comments on the final deprecation message and a follow up request

1. +++ b/core/includes/tablesort.inc
   @@ -9,18 +9,22 @@
   + *   Instead, get a TableSort utility injected into your service from the
   ...
   +  @trigger_error(__FUNCTION__ . '() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, get a TableSort utility and call getContextFromRequest() on it. For example, \Drupal\Core\Utility\TableSort::getContextFromRequest($header, $request). See https://www.drupal.org/node/3009182', E_USER_DEPRECATED);
   
   @@ -37,39 +41,23 @@ function tablesort_init($header) {
   + *   Instead, get a TableSort utility injected into your service from the
   ...
   +  @trigger_error(__FUNCTION__ . '() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, get a TableSort utility and call header() on it. For example, \Drupal\Core\Utility\TableSort::header($cell_content, $cell_attributes, $header, $ts). See https://www.drupal.org/node/3009182', E_USER_DEPRECATED);
   
   @@ -78,9 +66,18 @@ function tablesort_header(&$cell_content, array &$cell_attributes, array $header
   + *   Instead, get a TableSort utility injected into your service from the
   ...
   +  @trigger_error(__FUNCTION__ . '() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, get a TableSort utility and call getQueryParameters() on it. For example, \Drupal\Core\Utility\TableSort::getQueryParameters($request). See https://www.drupal.org/node/3009182', E_USER_DEPRECATED);
   
   @@ -93,31 +90,18 @@ function tablesort_get_query_parameters() {
   + *   Instead, get a TableSort utility injected into your service from the
   ...
   +  @trigger_error(__FUNCTION__ . '() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, get a TableSort utility and call getOrder() on it. For example, \Drupal\Core\Utility\TableSort::getOrder($headers, $request). See https://www.drupal.org/node/3009182', E_USER_DEPRECATED);
   
   @@ -128,22 +112,16 @@ function tablesort_get_order($headers) {
   + *   Instead, get a TableSort utility injected into your service from the
   ...
   +  @trigger_error(__FUNCTION__ . '() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, get a TableSort utility and call getSort() on it. For example, \Drupal\Core\Utility\TableSort::getSort($headers, $request). See https://www.drupal.org/node/3009182', E_USER_DEPRECATED);
   

   there is no container here, its static, can we update these comments?

2. +++ b/core/lib/Drupal/Core/Database/Query/TableSortExtender.php
   @@ -35,67 +34,17 @@ public function __construct(SelectInterface $query, Connection $connection) {
   +    $context = TableSort::getContextFromRequest($header, \Drupal::request());
   

   Can we get a follow up to allow extenders to implement container injection?

Interesting point!
@larowlan any reason to make injection?
The only reason is to acess request stack if request not passed but surely this class looks more like final to me

I missed renderer dependency for header image render in header() method

Follow-up filed #3014607: Allow container injection for TableSortExtender

Rework of deprecation messages - no reason to provide examples because this functions examples itself

Adding review credit for @goodboy

Committed 52224e3 and pushed to 8.7.x. Thanks!

Published the change record

Great work folks, more includes down!