Highlight deprecated modules and themes at admin/reports/status page, providing warning and link with explanation

1. +++ b/core/modules/system/src/Controller/SystemController.php
   @@ -225,7 +226,8 @@ public function themesPage() {
   -      $theme->is_experimental = isset($theme->info['experimental']) && $theme->info['experimental'];
   

   Do we need any BC here, or are we assuming that only core provided experimental modules and themes?

2. +++ b/core/modules/system/src/Controller/SystemController.php
   @@ -225,7 +226,8 @@ public function themesPage() {
   +      $theme->is_experimental = isset($theme->info[ExtensionLifecycle::LIFECYCLE_IDENTIFIER])
   +        && $theme->info[ExtensionLifecycle::LIFECYCLE_IDENTIFIER] === ExtensionLifecycle::EXPERIMENTAL;
   

   Should this be moved to a method on the value object, so we can just call ->isExperimental()? This code is duplicated in a few places.

   edit: there is no value object that handles ->info, this is blocked by #3015812: Introduce new Theme extension object and properly deprecate REGIONS_VISIBLE and REGIONS_ALL

Tried to fixed custom code failure #6

So if we're going to do #3250342: Deprecate "experimental: true" in .info.yml I think we need to leave the code that handles experimental: true alone in this issue and add lifecycle: experimental as an additional way of flagging experimental extensions, so modules/themes can use either until Drupal 10.

What is happening to the 'experimental;' property? Deprecation? If so, then more work for that. Would be nice to have that in the proposed resolution.

1. +++ b/core/lib/Drupal/Core/Extension/Extension.php
   @@ -192,4 +192,16 @@ public function __wakeup() {
   +    return (isset($this->info['experimental']) && $this->info['experimental'])
   +    || (isset($this->info[ExtensionLifecycle::LIFECYCLE_IDENTIFIER])
   

   I think this needs a comment explaining why two properties are being tested. And if one is being removed then an @todo to the followup.

2. +++ b/core/modules/system/system.install
   @@ -63,13 +67,19 @@ function system_requirements($phase) {
   +    $obsolete_extensions = [];
   

   Let's move this below the comment, and change the comment. See below.

3. +++ b/core/modules/system/system.install
   @@ -63,13 +67,19 @@ function system_requirements($phase) {
        // Warn if any experimental modules are installed.
   

   This comment, and the one below for themes, needs updating because the code is now doing experimental and obsolete.

1. +++ b/core/modules/system/system.install
   @@ -79,14 +91,26 @@ function system_requirements($phase) {
          if (isset($data->info['experimental']) && $data->info['experimental']) {
   

   Let's put the @todo here as well.

2. +++ b/core/modules/system/tests/themes/experimental_theme_test/experimental_theme_test.info.yml
   @@ -3,4 +3,4 @@ type: theme
   -experimental: true
   +lifecycle: experimental
   

   Do we need a legacy experimental test theme in order to keep testing experimental: true?

Thanks, I think this is getting close. Another round of review, mostly nitpicks:

1. +++ b/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php
   @@ -68,69 +69,103 @@ public function testExperimentalConfirmForm() {
   +   * Data Provider.
   

   "Data provider for experimental test themes"?

2. +++ b/core/lib/Drupal/Core/Extension/Extension.php
   @@ -192,4 +192,20 @@ public function __wakeup() {
   +    // @todo Remove the check for 'experimental: true' as part of https://www.drupal.org/node/3250342
   

   Line needs rewrapping.

3. +++ b/core/modules/system/system.install
   @@ -79,14 +91,30 @@ function system_requirements($phase) {
   +      // @todo Remove the check for 'experimental: true' as part of https://www.drupal.org/node/3250342
   

   Line needs rewrapping.

4. +++ b/core/modules/system/system.install
   @@ -94,6 +122,15 @@ function system_requirements($phase) {
   +      $requirements['obsolete_extensions'] = [
   +        'title' => t('Obsolete extensions enabled'),
   +        'value' => t('Obsolete extensions found: %extensions. <a href=":url">Obsolete extensions</a> are provided for compatibility purposes only. Use at your own risk.', ['%extensions' => implode(', ', $obsolete_extensions), ':url' => 'https://www.drupal.org/core/deprecation']),
   +        'severity' => REQUIREMENT_WARNING,
   

   Do we have a test that covers this?

5. +++ b/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php
   @@ -68,69 +69,103 @@ public function testExperimentalConfirmForm() {
   +   * @return \string[][]
   

   No \ before string.

6. +++ b/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php
   @@ -68,69 +69,103 @@ public function testExperimentalConfirmForm() {
   +   * @todo Turn the check for 'Testing legacy Key/Value pair "experimental: true"' into a @legacy test triggering a deprecation
   +   * as part of https://www.drupal.org/node/3250342
   

   Lines need rewrapping.

Made the changes requested in #22, except #22.4, thanks!

@Spokje I don't have a strong preference for which direction to merge the issues (maybe into this one since the MR is up-to-date), but it would definitely be good to close one or the other as duplicate and update issue summaries/titles if necessary.

Carried over the original title from #3215045: [Duplicate] Highlight obsolete module's status at admin/reports/status page, providing warning and link with explanation and updated the issue summary. Also applying credits from there.

Also applying parent and related issue.

Also this applies to all extensions not just modules, so expanding title.

Looks like this needs test coverage. We should have test cases for both modules and themes.

Making sure no-one tries to deal with profiles here.

This is blocking deprecation of core modules, so bumping to critical.

Hmm, i wonder if you could just install a deprecated module, then update the lifecycle config to say its obselete instead of using the installer flow.

Updated MR, but I have no clue how to enable an obsolete extension without triggering the ObsoleteExtensionException and thus preventing the enabling.

I don't think we need test coverage for that situation, it's what the exception is for. Having some language there for the very rare cases someone manages to do it doesn't hurt either way.

Added this under #3251854: [META] Requirements for tagging Drupal 10.0.0-alpha1 as a should have since it would be really nice to land this before alpha1 as per @catch :)

Updating the title, IMO deprecated modules are the priority here, obsolete would be completely optional because they should never be installed anyway.

Added deprecated modules and themes as separate warnings, just the same as experimental. The links are just TBD because I didn't like the idea of sending an admin to a core development page about deprecation policy. I haven't searched but this may need a new page that describes the extension lifecycle for an administrator.

Also, added an after screenshot, which includes an obsolete module and theme.

Added testing of the warning messages. Leaving the needs tests tag because the code for 'obsolete' is still in the MR and maybe we want to test that as well.

Looked at the docs and found two places where information about the lifecycle could be added

1. Chapter 11. Extending and Customizing Your Site This is in the curated user guide, which requires an issue in the User Guide project.
2. The Overview section of Extending Drupal

Suggestions?

In the IS there is nothing about deprecated and/or obsolete modules and/or themes.

Made issue summary clearer, inlined screenshot.

I also agree that testing would be impossible for obsolete extensions given you can't install them in the test.

All code changes look good to me.
All my points have been addressed.
The IS is in order.
For me it is RTBC.

Let's remove the 'Click the links' language per https://www.drupal.org/project/drupal/issues/3250585#mr1567-note66044

Otherwise agreed this is RTBC.

The documentation link for obsolete extensions points to https://www.drupal.org/core/TBD

Otherwise this looks ready to go.

Since lifecycle: obsolete also always comes hand in hand with a lifcycle_link, we _could_ do the same as we did for lifecycle: deprecated and make the name of the extension a link to each lifcycle_link?

Yes, good plan! :)

We should not generate HTML manually without ensuring it is properly escaped.

Testbot is not happy.

Looks good to me, not sure how to trigger retesting on MRs.

All points have been addressed.
Back to RTBC.

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

      $requirements['deprecated_modules'] = [
        'title' => t('Deprecated modules enabled'),
        'value' => t('Deprecated modules found: %module_list.', [
          '%module_list' => t(implode(',', $deprecated_modules_link_list)),
        ]),
        'severity' => REQUIREMENT_WARNING,
      ];

I think this is a misuse of t() in the %module_list parameter, translating the list like that doesn't make sense. Will open a followup.

Opened #3259996: Clean up t() usage for list of links in system_requirements().