Remove deprecated legacy include files from Drupal 9

> PHP Fatal error: Uncaught Error: Call to undefined function drupal_set_time_limit() in /var/www/html/core/scripts/run-tests.sh:162

Looks like we missed a call ;)

That should be backported to D8 I suppose, so separate issue maybe?

btw Maybe better to split this patch into per-file base to make it easy reviewable?

That's up to framework managers, but IMHO, this is pretty fine, these kind of patches are fairly easy to review. What's more important is that we separate non-test calls out into separate issues that can be backported, like already done in #17.

Thanks voleger -- for persisting with this issue.

I see 4 additional coding standard bugs in the test output

It is the curse of the unused used statement.

I think it needs 2 follow-ups because there's a changes which are out of scope and they backportable

1. +++ b/core/modules/file/file.module
   @@ -155,7 +155,7 @@ function file_load($fid, $reset = FALSE) {
   - * @see file_unmanaged_copy()
   + * @see \Drupal\Core\File\FileSystemInterface::copy()
   
   @@ -565,7 +565,7 @@ function file_validate_image_resolution(FileInterface $file, $maximum_dimensions
   - * @see file_unmanaged_save_data()
   + * @see \Drupal\Core\File\FileSystemInterface::saveData()
   
   +++ b/core/modules/toolbar/src/Controller/ToolbarController.php
   @@ -63,7 +63,7 @@ public function checkSubTreeAccess($hash) {
   -   * @see drupal_render()
   +   * @see \Drupal\Core\Render\RendererInterface::render()
   
   +++ b/core/scripts/run-tests.sh
   @@ -159,7 +160,7 @@
   -drupal_set_time_limit(0);
   +Environment::setTimeLimit(0);
   
   +++ b/core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php
   @@ -110,7 +110,7 @@ protected function tearDown(TestDatabase $test_database, $db_url) {
   -   * @see file_unmanaged_delete_recursive()
   +   * @see \Drupal\Core\File\FileSystemInterface::deleteRecursive()
   
   +++ b/core/tests/Drupal/Tests/Core/Command/QuickStartTest.php
   @@ -308,7 +308,7 @@ public function testServerWithNoInstall() {
   -   * @see file_unmanaged_delete_recursive()
   +   * @see \Drupal\Core\File\FileSystemInterface::deleteRecursive()
   

   This needs separate issue to clean-up mentions of deprecated functions, it is backportable to 8.8.x at least

2. +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
   @@ -86,7 +66,7 @@ public function testHtmlEscapedText($text, $expected, $message) {
   -   * Data provider for testCheckPlain() and testEscapeString().
   +   * Data provider for testEscapeString().
       *
       * @see testCheckPlain()
   

   Probably it needs separate quickfix for this inconsistency

1.

time limit already has a (RTBC) issue.

The file comment references, yeah, maybe, not sure if it's worth it, it's nothing functional.

2. This referes IIRC a (legacy) test function that is removed in this patch, it still exists in 8.x and is not removed.

Filed patch #22.2 #3092791: Fix wrong reference to testEscapeString() from SafeMarkupTest.php

Right, time_limit is at #3092268: Replace call of deprecated drupal_set_time_limit() function

Pager also in separate issue #3087517: Remove BC layer for Pager service

And the issue in #24 is IMHO no longer a blocker, per my comment there. On 8.x, the method name is just wrong but still exists, so that issue fixes that and here we can remove it.

So, lets reroll this without the pager part :)

Attempt to re-roll after pager fixed

There are 141 @deprecated within core/includes before the patch and just one after applying it, that is the REQUEST_TIME constant that we can't remove yet. And there are no @trigger_error() left, 113 are removed by the patch.

What this issue doesn't cover is related constructor deprecations in services/controllers/forms/.. that were introduced as part of the deprecation removal of the functions that we remove here, e.g. file_system. But I think unlike the entity manager, where that included instanceof checks of the removed interface, these are easier to detect and remove in the issues for the respective modules/components.

I also had a look at the remaining contents of these include files and while it is unfortunate that we couldn't finish some of the ongoing deprecations in time, like drupal_get_filename(), render() related things or file_create_url(), there isn't too much left and I think we could in a follow-up see if some of the left-overs could be merged into common.inc, so have to load fewer files on every request.

There are still a handful of deprecated function removals that could possibly be backported to D8, I extracted them in the attached patch, with the following command. It's only pseudo-code in hook examples and documentation, so nothing functional:

git diff HEAD core/modules/toolbar/src/Controller/ToolbarController.php core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php core/modules/file/file.module core/lib/Drupal/Core/Utility/token.api.php core/lib/Drupal/Core/Form/form.api.php core/lib/Drupal/Component/FileSecurity/FileSecurity.php > patches/3062757-30-D8-reference-fixes.patch

I think getting this in asap is beneficial to being able to do real tests against D9 for contrib modules. According to https://dev.acquia.com/drupal9/deprecation_status/graphs#topErrorStatus, this removes 7 of the top 15 deprecated things, with only simpletest, getMock(), Drupal:: methods and Unicode left. And it looks ready to me, so RTBC.

Committed 144c05e and pushed to 9.0.x. Thanks!

I also committed #30... Committed and pushed 91f4c1edfe to 8.9.x and d68d8dfa42 to 8.8.x. Thanks!

As these are docs backports to 8.8.x and 8.9.x. I'm leaving the issue at 9.0.x because that's where the bulk of this work is done.

Thanks for all the great work in here! Very exciting to see so much of this code going away.

I wish d.o made it easier to associate realistic git commit messages with individual patches, not the entire issue. Seeing commit d68d8df on the 8.8.x branch seems like a cherry-pick error:

"Issue #3062757 by voleger, Berdir, andypost: Remove deprecated legacy include files from Drupal 9"

That should have said something like:

"Issue #3062757 by voleger, Berdir, andypost: Fix documentation to not reference deprecated functions" (more or less).

Commenting here for posterity, in case anyone is looking at the git logs, confused, opens this issue, and happens to find this. ;)

Cheers,
-Derek

@dww I should have fixed that on commit and put a better title in. My bad.

@alexpott No worries! It totally happens. Y'all core committers have plenty to worry about, already. That's why I was saying I wished d.o made this task easier for you. Oh right, I (used to) maintain all those tools. Perhaps I should fix it. ;)

Cheers,
-Derek