Replace all module_exists() deprecated function calls.

I can take a look at this over the weekend. Keen to start looking into some core development.

I have run through all the instances of module_exists and replaced with the updated method. I have also removed the deprecated function as per Crells recommendation. It didn't seem to break anything, hopefully it can pass the testbot so it doesn't have to be rerolled too many times.

Here is a version without the deprecated function removed. Either one should work depending on the final decision made in this issue.

I noticed a few places where \Drupal had not been 'use'd, so I will wait for the test results to come back negative and then fix up the individual cases when they arise.

I need to setup the test suite locally.

This patch makes appropriate use of \Drupal where required and includes the deprecated function as per tim.plunkett's suggestion.

Thanks for link. I believe the patch conforms to the standard after reading the docs.

Yeah, I will manually repeat the work required for this patch as I doubt re-rolling would be very clean either. Will repost and hit up someone on IRC to have it applied quickly after it's creation.

I have attached another patch. Would be good to get this committed as soon as it passes testing to prevent further duplication of work.

The current patch replaces module_exists() with \Drupal::moduleHandler.

I suggest that this patch broken into two. One part to deal with the procedural code (everything not in a /lib/... directory. And another to do the OO code correctly.

All the changes to procedural code in this patch look great!

What follows is a review of the OO code.

+++ b/core/modules/system/lib/Drupal/system/Tests/System/InfoAlterTest.phpundefined
@@ -32,7 +32,7 @@ function testSystemInfoAlter() {
-    $this->assertTrue(module_exists('module_test'), 'Test module is enabled.');
+    $this->assertTrue(\Drupal::moduleHandler()->moduleExists('module_test'), 'Test module is enabled.');

@@ -47,7 +47,7 @@ function testSystemInfoAlter() {
-    $this->assertFalse(module_exists('module_test'), 'Test module is disabled.');
+    $this->assertFalse(\Drupal::moduleHandler()->moduleExists('module_test'), 'Test module is disabled.');

This is just one example from the patch... there are so many more I did not bother to list them all. In tests the simpletest runner has access to the container and therefore we can use $this->container->get('module_handler') instead of \Drupal::moduleHandler().

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/CustomBlockTypeFormController.phpundefined
@@ -53,7 +53,7 @@ public function form(array $form, array &$form_state) {
-    if (module_exists('content_translation')) {
+    if (\Drupal::moduleHandler()->moduleExists('content_translation')) {

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/CustomBlockTypeListController.phpundefined
@@ -20,7 +20,7 @@ class CustomBlockTypeListController extends ConfigEntityListController {
-    if (module_exists('field_ui')) {
+    if (\Drupal::moduleHandler()->moduleExists('field_ui')) {

+++ b/core/modules/block/lib/Drupal/block/BlockAccessController.phpundefined
@@ -73,7 +73,7 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
-      elseif (module_exists('php')) {
+      elseif (\Drupal::moduleHandler()->moduleExists('php')) {

+++ b/core/modules/block/lib/Drupal/block/BlockFormController.phpundefined
@@ -85,7 +85,7 @@ public function form(array $form, array &$form_state) {
-      if (module_exists('php') && $access) {
+      if (\Drupal::moduleHandler()->moduleExists('php') && $access) {

@@ -108,7 +108,7 @@ public function form(array $form, array &$form_state) {
-    if (module_exists('language') && language_multilingual()) {
+    if (\Drupal::moduleHandler()->moduleExists('language') && language_multilingual()) {

+++ b/core/modules/config/tests/config_test/lib/Drupal/config_test/ConfigTestFormController.phpundefined
@@ -47,7 +47,7 @@ public function form(array $form, array &$form_state) {
-    if (module_exists('image')) {
+    if (\Drupal::moduleHandler()->moduleExists('image')) {

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/VocabularyFormController.phpundefined
@@ -43,17 +43,17 @@ public function form(array $form, array &$form_state) {
-    if (module_exists('language')) {
+    if (\Drupal::moduleHandler()->moduleExists('language')) {

@@ -86,14 +86,14 @@ protected function actions(array $form, array &$form_state) {
-      if (module_exists('language')) {
+      if (\Drupal::moduleHandler()->moduleExists('language')) {
...
-      if (module_exists('content_translation')) {
+      if (\Drupal::moduleHandler()->moduleExists('content_translation')) {

These can be injected by implementing EntityControllerInterface and adding a createInstance method. For an example have a look at \Drupal\action\ActionAddFormController

+++ b/core/modules/hal/lib/Drupal/hal/Normalizer/EntityNormalizer.phpundefined
@@ -80,7 +80,7 @@ public function denormalize($data, $class, $format = NULL, array $context = arra
-    elseif (module_exists('language')) {
+    elseif (\Drupal::moduleHandler()->moduleExists('language')) {

This should be injected by updating serializer.normalizer.entity.hal definition in hal.services.yml perhaps using setter injection.

+++ b/core/modules/history/lib/Drupal/history/Plugin/views/field/HistoryUserTimestamp.phpundefined
@@ -34,7 +34,7 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
-      if (module_exists('comment') && !empty($this->options['comments'])) {
+      if (\Drupal::moduleHandler()->moduleExists('comment') && !empty($this->options['comments'])) {

@@ -50,7 +50,7 @@ protected function defineOptions() {
-    if (module_exists('comment')) {
+    if (\Drupal::moduleHandler()->moduleExists('comment')) {

@@ -78,7 +78,7 @@ public function render($values) {
-      $last_comment = module_exists('comment') && !empty($this->options['comments']) ?  $this->getValue($values, 'last_comment') : 0;
+      $last_comment = \Drupal::moduleHandler()->moduleExists('comment') && !empty($this->options['comments']) ?  $this->getValue($values, 'last_comment') : 0;

+++ b/core/modules/history/lib/Drupal/history/Plugin/views/filter/HistoryUserTimestamp.phpundefined
@@ -75,7 +75,7 @@ public function query() {
-    if (module_exists('comment')) {
+    if (\Drupal::moduleHandler()->moduleExists('comment')) {

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/field/Node.phpundefined
@@ -33,7 +33,7 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
-      if (module_exists('translation')) {
+      if (\Drupal::moduleHandler()->moduleExists('translation')) {

+++ b/core/modules/node/lib/Drupal/node/Plugin/views/field/Revision.phpundefined
@@ -31,7 +31,7 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
-      if (module_exists('translation')) {
+      if (\Drupal::moduleHandler()->moduleExists('translation')) {

@@ -66,7 +66,7 @@ function render_link($data, ResultRow $values) {
-      if (module_exists('translation')) {
+      if (\Drupal::moduleHandler()->moduleExists('translation')) {

This can be injected with by creating a static create method on the class... see Drupal\views\Plugin\views\style\Table

Hi Alex,

Thanks for the review. I will take a look.

Sam

I have attached the patch for the procedural code only. The OO code can be tackled in a later patch.

no longer applies.

I tried to re-rolling.

The patch in #24 applied cleanly for me and everything looks OK. (Although I had to delete sites/default/files/php to get up and running again.)

As per #25 - patch applied cleanly, but had to empty sites/default/files/php/ to get back up and running.

Re-roll as the comment module was failing to apply. And fix from #27

#28 looks good and applies cleanly.

#28 applied cleanly and looks fine.

Reroll of #28 (the 20.78kb one) as the first hunk in node.views.inc has been removed already in #2057401: Make the node entity database schema sensible

Otherwise it is the same.

#32: 2045931-32.patch queued for re-testing.

And another reroll as comment.module and comment.views.inc are already done in #2003498: Replace drupal_container() with Drupal::service() in the comment module

Committed/pushed to 8.x, thanks!

Reopening as there are still loads of calls to module_exists().

Ok here is all but the function itself. Should that be included as well or marked as deprecated?

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/rest/resource/DBLogResource.php
@@ -27,7 +27,7 @@ class DBLogResource extends ResourceBase {
+    if (\Drupal::moduleHandler()->moduleExists('dblog')) {
...
-    if (module_exists('dblog')) {

This is in OO code, so shouldn't use \Drupal. There are several examples of this in the patch.

Ok here is all but the function itself. Should that be included as well or marked as deprecated?

Let's leave it for now, and then remove all the module_* functions together for the meta. It would be good to expand the comment though, to make it clear that it's not going to be around much longer. Suggested comment as discussed on #2158871: [Policy, no patch] Clearly denote when @deprecated code is slated to be removed would be

/**
 * Determines whether a module implements a hook.
 *
 * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0-beta1, see https://drupal.org/node/1916134
 *
 * @see https://drupal.org/node/1894902
 *
 * @see \Drupal\Core\Extension\ModuleHandler::implementsHook()
 */

Regarding using \Drupal in OO code, the same problem applies in #2055371: Replace all module_invoke_all() deprecated function calls in OO code, where it has been decided to use \Drupal for now, then do propery dependency injection in a later patch. We can do the same here DI is going to be tricky, but at the very least CustomBlockTypeFormController has a $moduleHandler property that we can use (I've not checked the others).

Replaced two \Drupal::moduleHandler() calls in CustomBlockTypeFormController and ConfigTestFormController as mentioned in #44.

moving back to needs work, based on @ianthomas_uk's suggestion in #41 to either remove the deprecated function, or expand the comments.

Actually, the functions for this meta are all next to each other, so if we remove or edit the docblock on one then it will trigger a reroll for one of the other patches.

Let's just remove the uses, then we can follow up with a quick patch to remove the functions themselves.

We still need a followup to switch to DI.

Yes removing uses then separate patch to remove the actual functions is the easiest way to do these - reduces the chance of broken HEAD due to commit conflicts. And the removal patches themselves are much easier to re-roll/roll-back in case of conflicts.