[PP-1] Deprecate drupal_static() and drupal_static_reset()

Seriously .. there is really little value in doing this here, if you ask me, because most instances should be already converted and the other should be converted.

@dawehner what about FormBuilder? Is there a clear path to removing drupal_static_reset from it?

If not, I'd argue this is still worthwhile.

@dawehner what about FormBuilder? Is there a clear path to removing drupal_static_reset from it?

If not, I'd argue this is still worthwhile.

What about cats? Aren't they worthwhile to be added to core? Yes I think so. FormBuilder for example just uses a reset on drupal_html_id, which will be dropped anyway.

i agree with dawehner
Moving on #2121713: Move drupal_html_id() and drupal_html_class() to Drupal\Component\Utility is certainly more valuable

Well, sooner all later most of them will go away. Opening issues to kill them one by one and assigning #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() as parent is probably a better course of action.
Dont get me wrong, i am just saying.. you can of course spend your time as you like :)

I don't really get the point of this issue... bootstrap.inc won't go away anytime soon, so it's not about auto-loading. The change makes DrupalStatic itself injectable, but it's still using a static variable itself, so it's still a global state construct, and thus the dependency still exists as-is, it's just moved from A to B. I'm not able to see the improvement of doing that.

Instead of duct-taping drupal_static(), I'd rather refactor the actual dependency (e.g. drupal_html_id()) into a class/service. Even if that would end up to just be a nonsense/mini/stateful service, it would at least be a step in the right direction.

But of course I agree with @ParisLiakos, this is just some sanity feedback; you can obviously use your time to work on whatever you want ;)

  * @param $name
  *   Name of the static variable to reset. Omit to reset all variables.
...
 function drupal_static_reset($name = NULL) {
-  drupal_static($name, NULL, TRUE);
+  \Drupal::service('event_dispatcher')->dispatch('drupal_static_reset', new StaticResetEvent($name));
 }

+++ b/core/lib/Drupal/Core/DrupalStatic/StaticResetEvent.php
@@ -0,0 +1,43 @@
+  public function __construct($name) {

The reason this fails is that the NULL behavior for resetting all statics is not covered.

To my knowledge, the NULL case is actually the primary use-case in HEAD now. (although it's possible that I've been working on too much low-level stuff recently)

This looks much better and simpler now, but I still don't think this is helpful, so I don't want to RTBC this change.

I think we should profile this patch as well.

It was just a suggestion but according to @tim.plunkett in IRC

i don't think profiling a language level difference is worth holding the issue up

so I withdraw my suggestion. :)

This needs a reroll. Not sure if it's too late for 8.0?

Rerolling patch from #12. Had to do a little cleanup in core/testsDrupal/Tests/Core/Form/FormTestBase.php

Basically I removed the following code:

namespace {

  function test_form_id() {
    $form['test'] = array(
      '#type' => 'textfield',
      '#title' => 'Test',
    );
    $form['options'] = array(
      '#type' => 'radios',
      '#options' => array(
        'foo' => 'foo',
        'bar' => 'bar',
      ),
    );
    $form['value'] = array(
      '#type' => 'value',
      '#value' => 'bananas',
    );
    $form['actions'] = array(
      '#type' => 'actions',
    );
    $form['actions']['submit'] = array(
      '#type' => 'submit',
      '#value' => 'Submit',
    );
    return $form;
  }

}

Please let me know, if I did it right. The code did not make sense to me and looked like a test code. If this is wanted, I can add it back. Rest of the conflicts were pretty easy to fix.

@AjitS test_form_id() is needed. The changes in FormTestBase from #12 are irrelevant now that there is an Html utility class. Some of the methods in TestFormBuilder should be using it. However, I think those methods are never called. They could possibly be removed in a separate issue.

#2456983: Five methods in class TestFormBuilder are dead code. Remove them.

Let's improve the patch a bit:

• Move that into a component, called StaticStorage. It can be reused
• Call it from without UnitTestCase::setup()
• use it for the two most usecases in core: SafeMarkup, Html

Reroll.

I think we wanted the drupal_static() docblock mostly emptied. Could whoever rolls this next take care of that and let's do #23.

This is #25 and #23-1. We still need to:

• Call it from without UnitTestCase::setup()
• use it for the two most usecases in core: SafeMarkup, Html

1. +++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
   @@ -0,0 +1,179 @@
   + * All functions requiring a static variable to persist or cache data within
   + * a single page request are encouraged to use this function unless it is
   + * absolutely certain that the static variable will not need to be reset during
   + * the page request. By centralizing static variable storage through this
   + * function, other functions can rely on a consistent API for resetting any
   + * other function's static variables.
   

   "function" should be "class".

2. +++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
   @@ -0,0 +1,179 @@
   + * Example:
   + * @code
   + * function example_list($field = 'default') {
   + *   $examples = &drupal_static(__FUNCTION__);
   + *   if (!isset($examples)) {
   + *     // If this function is being called for the first time after a reset,
   + *     // query the database and execute any other code needed to retrieve
   + *     // information.
   + *     ...
   + *   }
   + *   if (!isset($examples[$field])) {
   + *     // If this function is being called for the first time for a particular
   + *     // index field, then execute code needed to index the information already
   + *     // available in $examples by the desired field.
   + *     ...
   + *   }
   + *   // Subsequent invocations of this function for a particular index field
   + *   // skip the above two code blocks and quickly return the already indexed
   + *   // information.
   + *   return $examples[$field];
   + * }
   + * function examples_admin_overview() {
   + *   // When building the content for the overview page, make sure to get
   + *   // completely fresh information.
   + *   drupal_static_reset('example_list');
   + *   ...
   + * }
   + * @endcode
   

   The examples are demonstrating the global functions instead of this class.

The StaticStorage docblock needs a good looking-over.

@dawehner After that is done, I think changing usages should be follow-ups. There are a lot.

@dawehner After that is done, I think changing usages should be follow-ups. There are a lot.

I think we should try to ensure that the fast pattern can actually used in an OO version

@dawehner Fair enough - could you please explain comment #23 in more detail?

I fixed up some of the documentation before moving on.

Who else thinks this is evil :)

#31: I think this looks pretty genius actually.

Discussed with msonnabaum and dawehner in IRC:

This allows two things:

a) Reset
b) Scoping

Both are important.

While it would be way better to not have any statics and have everything in classes, this is not gonna happen and there is also legacy code.

=> Lets get this in ...

Do we want to use it in SafeMarkup? it seems like the current class static is good enough?

Do we want to use it in SafeMarkup? it seems like the current class static is good enough?

No, you want to have a central place to reset those statics ... see the comment from fabianx

I don't see how this provides any scoping?

Also - do we know if there is a performance difference in calling the static method rather than a function?

Addressed feedback in #39. Patch attached.

A documentation specialist ought to look over the docblock.

A reroll.

Oops, ResettableStaticUnitTest crept in.

Well, I'd like to see this, but it seems clear it isn't going to make 8.0.

I think this certainly can still land in 8.0.x as it solves problems we have.

I agree it could land in 8.0.x to make our tests easier, etc.

Adapted the issue summary

Possibly documentation duplication here: #422352: Improve code comments for static caching API

This is 8.1.x material now, this is for sure.

Trying the reroll against 8.1.x.

Currently 46 calls to drupal_static() in core, according to api.drupal.org.

I like StaticStorage as an OOP replacement for our static pattern. Just one thing:

+++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
@@ -0,0 +1,180 @@
+  public static function reset($name = NULL) {
+    if (!isset($name)) {
+      // Reset all: ($name == NULL). This needs to be done one at a time so that
+      // references returned by earlier invocations of StaticStorage::get() also
+      // get reset.
+      foreach (static::$default as $name => $value) {
+        static::$data[$name] = $value;
+      }
+    }

If you pass in a generated name that doesn't exist, you end up losing the benefit of caching for everything. Could we add resetAll() and be explicit? And then make the $name argument non-optional. If it doesn't exist, do nothing.

I wonder if a better name would be GlobalStorage or even GlobalStaticStorage? That way it's clearer that the storage is global to the entire request?

If you pass in a generated name that doesn't exist, you end up losing the benefit of caching for everything. Could we add resetAll() and be explicit? And then make the $name argument non-optional. If it doesn't exist, do nothing.

Great idea!

I wonder if a better name would be GlobalStorage or even GlobalStaticStorage? That way it's clearer that the storage is global to the entire request?

Having the static storage in there is IMHO an important bit. I'm happy with GlobalStatic, do you agree?

Yes, I like GlobalStatic. +1 from me.

Nice.

Not sure about the test failures... I guess everything uses drupal_static(), eh?

Couple things:

1. +++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
   @@ -158,23 +158,26 @@ public static function &get($name, $default_value = NULL) {
      public static function reset($name = NULL) {
   

   The only reason to have a default of NULL is so that you can call reset() and have nothing happen. :-)

2. +++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
   @@ -158,23 +158,26 @@ public static function &get($name, $default_value = NULL) {
   +  public static function resetAll() {
   +    // Reset all: ($name == NULL). This needs to be done one at a time so that
   

   Don't need the $name==null part in the documentation.

Good points @mile23

The patch in #60 addresses concerns in #59, and still applies.

The new static class fills a gap, though I hate statics. :-)

We have BC with the old global methods.

We have tests, and can migrate towards using this new class as needed.

I've added the 'Needs change record' tag, since it's a deprecation.

Updated the issue summary.

Generally RTBC-worthy except for some paperwork.

Thank you @mile23
Filed the change record

Looks good. Let's go. :-)

This was already discussed on #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() (edit: hmm partly on that issue, but I think more on another one), but the result of that discussion was just to gradually convert the procedural code using drupal_static() to services. Then any 'static caching' could be on protected properties of those classes then, if it's still needed.

I can't yet see the benefit of making drupal_static() itself 'nicer' - it was never supposed to be nice, it was a horrible hack to get around static caching issues in SimpleTest.

If there's a use case for it other than 'code that's not services yet', then it'd be good to see one. #33-35 mention SafeMarkup, but that's gone now.

Also is there any profiling done here? Without conversions that's hard to do, and I guess if there's a hit we could make the threshold for using the drupal_static_fast pattern lower, but also if it's sufficiently bad it starts to make the static caching in some cases pointless anyway.

This was already discussed on #1577902: [meta] Remove all drupal_static()s + drupal_static() itself (edit: hmm partly on that issue, but I think more on another one), but the result of that discussion was just to gradually convert the procedural code using drupal_static() to services. Then any 'static caching' could be on protected properties of those classes then, if it's still needed.

Throughout the cycle in 8.0.x we have actively added some static caches in places which didn't used to be some, like the filecache component, the extension discovery etc., so the base of the discussion changed over time.

Those bits currently have a normal static, so those caches cannot be cleared which already bites us in some places.

Those bits currently have a normal static, so those caches cannot be cleared which already bites us in some places.

Yep a static in a service is not really any better. Having the ability to clear static caches makes it easier to test stuff.

Yep a static in a service is not really any better. Having the ability to clear static caches makes it easier to test stuff.

OH yeah no quesiton. Its horrible that we have those kind of workarounds. Having though one central place for them, makes it really obvious how bad it actually is.

So the problem is: Items use static caching which we can't control, therefore we're complicating the testing process because there's a bunch of static data keeping the code from using our mock. We're also eating up memory that won't be garbage-collected during the request.

If the policy is that we shouldn't migrate away from drupal_static() (to StaticStorage), then we can still use this patch which has a BC layer leaving drupal_static() in place. Code that uses drupal_static() can have its cache cleared by a test using StaticStorage::reset(), without having to jump through hoops to autoload an .inc file.

The deprecation message could be to stop using statics rather than to start using StaticStorage, which could then be documented as a backend for drupal_static().

Hmm. I guess I should have marked this 'needs work.'

Updated the issue summary based on #65 onward.

If there's documentation on how to prove things with profiling, I'd love to see it. :-)

The patch in #60 needed a reroll, so I did that, and also updated the docblocks to reflect #69.

If this is acceptable we'll still need to work on the change record.

Also, @catch #65:

If there's a use case for it other than 'code that's not services yet', then it'd be good to see one.

Services are really supposed to be stateless, but I think that boat has sailed.

These changes look great! Now its just about time to find someone to review it.

I can't yet see the benefit of making drupal_static() itself 'nicer' - it was never supposed to be nice, it was a horrible hack to get around static caching issues in SimpleTest.

As long as the test runner for things that use WebTestBase or BrowserTestBase are able to access the database and services of the site under test we have this problem. Saying that services shouldn't use this seems premature. The test runner makes far too many API calls at present in the name of speed but not sanity and proper test coverage.

Updated the change record to reflect that we shouldn't use statics at all, and that this change is for testing legacy code. https://www.drupal.org/node/2661204

+++ b/core/includes/bootstrap.inc
@@ -902,11 +784,23 @@ function &drupal_static($name, $default_value = NULL, $reset = FALSE) {
+ * @deprecated in Drupal 8.x-dev, will be removed before Drupal 9.0.0. Do not

Updated this to be marked as deprecated in 8.1.x.

Also marked StaticStorage as deprecated with the same message.

+1 for telling people that statics aren't a good idea.

This difference would totally disappear when once we would have converted the entries over, but of course, for all other static caches, this would be worse again.

### FINAL REPORT

=== 8.1.x..8.1.x compared (56d454b883f33..56d45cf1ddec3):

ct  : 27,653|27,653|0|0.0%
wt  : 68,071|68,447|376|0.6%
mu  : 13,942,216|13,942,216|0|0.0%
pmu : 14,078,592|14,078,592|0|0.0%

=== 8.1.x..2260187 compared (56d454b883f33..56d45d040c011):

ct  : 27,653|27,665|12|0.0%
wt  : 68,071|67,917|-154|-0.2%
mu  : 13,942,216|14,029,256|87,040|0.6%
pmu : 14,078,592|14,166,128|87,536|0.6%

=== SUM: 2260187-summary..8_1_x-summary compared (56d45d12e6ef3..56d45d16d0e3d):

ct  : 2,768,348|2,766,979|-1,369|-0.0%
wt  : 7,138,631|7,308,247|169,616|2.4%
mu  : 1,398,126,040|1,396,466,312|-1,659,728|-0.1%
pmu : 1,412,306,272|1,410,145,608|-2,160,664|-0.2%

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage

### XHPROF-LIB REPORT

+-------------------+------------+------------+------------+------------+------------+
| namespace         |        min |        max |       mean |     median |       95th |
+-------------------+------------+------------+------------+------------+------------+
| Calls             |            |            |            |            |            |
|                   |            |            |            |            |            |
| 2260187           |     27,665 |     29,513 |     27,683 |     27,665 |     27,665 |
| 8_1_x             |     27,653 |     29,332 |     27,670 |     27,653 |     27,653 |
|                   |            |            |            |            |            |
| Wall time         |            |            |            |            |            |
|                   |            |            |            |            |            |
| 2260187           |     67,915 |     89,821 |     71,386 |     70,703 |     76,392 |
| 8_1_x             |     68,447 |    102,529 |     73,082 |     71,402 |     86,068 |
|                   |            |            |            |            |            |
| Memory usage      |            |            |            |            |            |
|                   |            |            |            |            |            |
| 2260187           | 13,952,528 | 14,367,328 | 13,981,260 | 13,952,608 | 14,318,240 |
| 8_1_x             | 13,942,008 | 14,351,880 | 13,964,663 | 13,942,216 | 14,307,504 |
|                   |            |            |            |            |            |
| Peak memory usage |            |            |            |            |            |
|                   |            |            |            |            |            |
| 2260187           | 14,088,880 | 15,028,464 | 14,123,063 | 14,089,088 | 14,454,672 |
| 8_1_x             | 14,078,288 | 14,525,408 | 14,101,456 | 14,078,592 | 14,443,856 |
|                   |            |            |            |            |            |
+-------------------+------------+------------+------------+------------+------------+

+++ b/core/lib/Drupal/Component/Utility/StaticStorage.php
@@ -0,0 +1,191 @@
+ * @deprecated in Drupal 8.1.x, will be removed before Drupal 9.0.0. Do not use
+ *   global static PHP variables. For globally-accessible in-memory caching, use
+ *   services.

I'm not sure what is meant by global static variables. A variable in the global PHP scope is not static ie something like global $user... this seems an odd turn of phrase.

Also isn't something like ExtensionDiscovery::$files shouldn't that be using StaticStorage.

And adding something already deprecated seems premature. I don't think this is a promise we can keep.

I agree, we should update the comment

'Global static PHP variables' is a bit awkward but it's supposed to mean that you can use statics as long as they're not global-scope, like those wrapped by drupal_static() or just global static $foo.

I suppose telling people to use StaticStorage is the best solution right now, since the problem we're trying to solve is being able to reset the static during tests without having to include an .inc file.

So:

Un-deprecated StaticStorage.

Changed the deprecation messages to be 8.2.x.

Hopefully clarified the language in the deprecation messages.

Removed 'needs profiling' tag re: #77

Thank you @mile23!

I'm still missing the use-cases for this, as asked for in #65.

ExtensionDiscovery looks like it has a bug where it's not keying the $files static by hash of $this->searchDirectories and whether tests are included. Also ExtensionDiscovery is the opposite of 'properly injected OOP code', see ExtensionDiscovery::setProfileDirectoriesFromSettings()

I don't know of any other classes in core that are using static properties that are causing problems in tests, and none are listed here apart from ExtensionDiscovery and FileCache.

The only non-test classes that are calling drupal_static_reset() in core are in migrate, shortcut and taxonomy - none of these are good examples either. For example TermStorage::resetCache() calls drupal_static_reset('taxonomy_term_count_nodes') - that function doesn't even exist in 7.x let alone 8.x

BrowserTestBase has three calls for non-existent statics too.

These are examples of static properties ... which all could be problematic in tests or for example when someone would run a PHP daemon.

core/lib/Drupal/Component/FileCache/FileCache.php
core/lib/Drupal/Component/FileCache/FileCacheFactory.php
core/lib/Drupal/Component/Utility/Html.php
core/lib/Drupal/Component/Utility/Timer.php
core/lib/Drupal/Component/Utility/Unicode.php
core/lib/Drupal/Component/Utility/UrlHelper.php
core/lib/Drupal/Component/Utility/Xss.php
core/lib/Drupal/Core/DrupalKernel.php
core/lib/Drupal/Core/Composer/Composer.php
core/lib/Drupal/Core/Datetime/Element/Datetime.php
core/lib/Drupal/Core/Extension/ExtensionDiscovery.php
core/lib/Drupal/Core/Extension/InfoParser.php
core/lib/Drupal/Core/Form/FormState.php
core/lib/Drupal/Core/Mail/MailFormatHelper.php
core/lib/Drupal/Core/Password/PhpassHashedPassword.php
core/lib/Drupal/Core/Render/Renderer.php
core/lib/Drupal/Core/Utility/Error.php

Thanks for the list. Had a look through all of them.

core/lib/Drupal/Component/FileCache/FileCache.php
core/lib/Drupal/Component/FileCache/FileCacheFactory.php

See below.

core/lib/Drupal/Component/Utility/Html.php

Already has an issue to remove it, StaticStorage doesn't help with that.

    // @todo Remove all that code once we switch over to random IDs only,
    // see https://www.drupal.org/node/1090592.
    if (!isset(static::$seenIdsInit)) {
      static::$seenIdsInit = array();
    }
    if (!isset(static::$seenIds)) {
      static::$seenIds = static::$seenIdsInit;
    }

#1305882: drupal_html_id() considered harmful; remove ajax_html_ids to use GET (not POST) AJAX requests

core/lib/Drupal/Component/Utility/Timer.php

Has its own API to clear the static. Should not be globally reset since that would break timers.

core/lib/Drupal/Component/Utility/Unicode.php

Horrible, but also has a setter for testing. Should not be globally reset since it is set during bootstrap and not on demand, and there's no useful default.

core/lib/Drupal/Component/Utility/UrlHelper.php

Has a setter, should not be globally reset since the value is set in DrupalKernel::preHandle() based on a container parameter, meaning there's no useful default if it gets reset.

core/lib/Drupal/Component/Utility/Xss.php

These are only statics because PHP < 5.6 doesn't allow array class constants. We don't ever want people to be able to change that list globally.

core/lib/Drupal/Core/DrupalKernel.php

Should not be globally reset since it exists very specifically to ensure that it only gets run once per PHP process. Method could possibly be made re-entrant so it doesn't need the static, or possibly already is and the static is cruft.

core/lib/Drupal/Core/Composer/Composer.php

'array constant' again.

core/lib/Drupal/Core/Datetime/Element/Datetime.php

This is a static so that the example always shows the exact same date for a request - i.e. so that two elements don't show 17:35:32 and 17:35:33

Could add a setter. Should probably however get request time from a request object and not be a static at all, or be replaced in JavaScript or something different entirely, or just remove it and accept the second rollover issue, since right now it'd break with render caching anyway.

StaticStorage doesn't help long running processes here, since in those cases you'd still want all date elements rendered from the master request to have a consistent date example, assuming we don't fix it properly above. Also no memory implications since it's a single object.

core/lib/Drupal/Core/Extension/ExtensionDiscovery.php

Discussed above and below.

core/lib/Drupal/Core/Extension/InfoParser.php

Discussed below.

core/lib/Drupal/Core/Form/FormState.php

There are three calls to FormState::hasAnyErrors() in core.

#1863020: View's build fails when an unrelated form on the same page has validation errors and #1305882: drupal_html_id() considered harmful; remove ajax_html_ids to use GET (not POST) AJAX requests would remove two.

The third is in FormBuilder:

      // If there are no errors and the form is not rebuilding, submit the form.
      if (!$form_state->isRebuilding() && !FormState::hasAnyErrors()) {
     

Looks dodgy in the same way that the call in ViewExecutable is - it's checking if there are any errors in any forms, not just this one.

So needs an issue for that last one, and should be @deprecated.

core/lib/Drupal/Core/Mail/MailFormatHelper.php

Makes my eyes hurt. But from a quick look could probably be replaced by passing in $urls as an array and adding to it by reference, since it is reset then checked within the same method and nowhere else.

core/lib/Drupal/Core/Password/PhpassHashedPassword.php

Static was added in 2012 when the class went in, as far as I can see should be a constant like the other constants for that class.

core/lib/Drupal/Core/Render/Renderer.php

   * This must be static as long as some controllers rebuild the container
   * during a request. This causes multiple renderer instances to co-exist
   * simultaneously, render state getting lost, and therefore causing pages to
   * fail to render correctly. As soon as it is guaranteed that during a request
   * the same container is used, it no longer needs to be static.

core/lib/Drupal/Core/Utility/Error.php

Another 'array constant'.

Of those, several should definitely not under any circumstances be changed to either drupal_static() or StaticStorage. Three main categories for these:

1. protected static = array() because we don't require PHP 5.6, after which they could be constants. Just needs a @todo. In the case of Xss tag lists, converting those to StaticStorage would allow contrib/custom code to introduce security issues where they currently can't (if one altered the global list to save passing in a method param, much worse things have been done before).

2. timers, environment status, re-entrance checks, all things which can't be globally reset, and have setters for testing. Ugly, but StaticStorage doesn't help.

3. Bugs/travesties, for which StaticStorage will not make them less of a bug/travesty, some of those have had issues open to refactor for some time. Conversion won't help those issues move faster, might hold them up.

The borderline ones for me are ExtensionDiscovery, InfoParser and FileCache, which are all maintaining static caches - so actually storing things that they explicitly don't want to recalculate during a request.

FileCache already injects a cache backend. So I wondered why it doesn't inject a chained cache backend fronted by MemoryCache and drop the static property, or use MemoryCache directly and do it's own chaining internally. Then I read the issue and found this comment #2395143-86: YAML parsing is very slow, cache it with FileCache by a certain @dawehner. So not sure, but I think that could use its own issue to revisit if we need to. For example we could potentially replace that with a 'statically caching memory backend' that has lots of comments explaining why it does it. Cache backends have a deleteAll() method which would cover the long-running process/memory reclaim issue.

ExtensionDiscovery and InfoParser are both mostly called by https://api.drupal.org/api/drupal/core%21modules%21system%21system.modul...

That entire area is very messy, and if we continue refactoring it, could probably end up with class properties rather than statics, although we'd likely run into the same issue with container rebuilds then in the same way that FileCache does.

Those three, very specific and similar cases in more or less the same subsystems do not really support adding a generic API for static storage, especially not one that is described as a replacement for drupal_static().

Given #185 should we rescope this issue to deprecate drupal_static() and pals, without adding StaticStorage? Then make follow-ups to do all the work suggested?

Linked to one wrong issue in #85, the Html::id() issue that's actually open is #1090592: [meta] Use HTML5 data-drupal-* attributes instead of #ID selectors in Drupal.settings, #85 links to the previous one.

@Mile23 there's a meta for drupal_static() at #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() , but as discussed in that issue, usually the drupal_static() removal is a by-product of a wider refactoring, so a single meta to track them is hard.

However in #83 I found at least two calls to drupal_static() that are dead code and could be removed, we could open a single issue to find and remove those calls since there may be others. That would then make it easier to see what's left.

Some of the cases in #85 could definitely have individual issues opened, like the array class constants and MailHandler etc.

I think it's worth an issue to talk about ExtensionDiscovery/FileCache/InfoParser more - potentially this issue could stay open for that?

+1 to #86.

For example, we already have a static cache bin that's just a memory backend. We could use that for some cases. One of the benefits we have there is that we have cache tag based invalidation. So maybe we can also get rid of some manual cache invalidations through that.

The use case for StaticStorage is admittedly weak: We wanted a replacement for drupal_static() that could be reset without loading .inc files or without booting a kernel in order to load an .inc file. This was for testing purposes.

Generally, use of statics like this, especially hidden behind a non-autoloaded global function, is bad form, so the desire was to provide an alternative.

However, it's clear the StaticStorage part is a no-go, which is why I asked about rescoping for deprecation in #86.

This issue would not be a meta for deprecation, but to apply the following patch which would mark drupal_static() and drupal_static_reset() as @deprecated for removal in Drupal 9.0.0.

That way, we can signal to contrib that they should stop using it, and also have a clear reason for core maintainers to reject its use. This would also make all those other issues implied in #85 actionable.

Related issue I just opened, which is hopefully not too confusing:
#2729725: Reset API / ResettableInterface (brainstorming)

• Adapted the patch to follow Drupal core deprecation policy
• Updated title and IS.
• Updated the change record
• Postponing on #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset()

The question now is how to remove the usage from procedural code. If we cannot, we can't unpostpone this one.

This is the parent issue of #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() , not the other way around.

Added #3037054: Deprecate drupal_static_reset() without @trigger_error()

I think we still can deprecate the usage for contrib and custom code callers. Then we gradually deprecate core usages by progressing with #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() . Some may argue that the solution is ugly but I find it effective.

The MR is not a continuation of previous PRs, even we can already add the `@deprecation` tags to both functions.

Should we not remove all usage of the two functions before we deprecate them?

Yes let's explicitly postpone this on #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() .