[meta] Commented-out code in Drupal

Created on 16 September 2017, about 7 years ago
Updated 1 June 2024, 5 months ago

Problem/Motivation

There's lots of commented-out code in the core codebase, especially. Code might be commented for a lot of reasons -- missed cleanup on a patch, accidentally leaving something commented out after debugging, skipped tests for later functionality -- but it is almost always wrong and a code smell that can indicate bugs, incomplete APIs, etc.

Any commented-out code purposefully left for later development should be accompanied by an @todo linking to the issue that will address the point in the future.

Proposed resolution

  1. Find commented-out code in the Drupal core codebase.
  2. In each case, use git log -L for the commented lines to figure out when and why the code was commented in the first place, and how the lines around it have changed.
  3. Do research into the issues that you find. Read them carefully and check comments and interdiffs for how and why the code got commented out.
  4. Based on this, make a recommendation as to what to do. Possibilities:
    • The commented code should be accompanied with an @todo and followup issue (find or file it).
    • The commented code is leftover cruft that can be removed. (If this is the case, read the code very carefully to see what the purpose of the code was and that it's no longer needed! Check for test coverage of the code.
    • For test code, commented test methods should possibly be marked skipped instead.
  5. Check with the maintainers for the subsystem to make sure your changes are correct.

Remaining tasks

Search core for commented out code.

This grep, grep -r --include \*.php "^\s*//.*=.*;" core finds commented out code in the following methods.

  • \Drupal\Tests\ban\Functional\IpAddressBlockingTest::testIPAddressValidation
  • \Drupal\field\Plugin\migrate\process\d6\FieldInstanceDefaults::transform
  • \Drupal\field\Plugin\migrate\process\d6\FieldInstanceSettings::transform
  • \Drupal\views\Plugin\views\query\Sql::execute
  • \Drupal\Tests\views\Functional\Handler\FieldWebTest::testAlterUrl
  • \Drupal\Tests\views_ui\Functional\DefaultViewsTest::testDefaultViews
  • \Drupal\FunctionalTests\Core\Recipe\StandardRecipeInstallTest::setUpSite
  • \Drupal\KernelTests\Core\Entity\Element\EntityAutocompleteElementFormTest::testEntityAutocompleteIdInput
  • \Drupal\KernelTests\Core\Image\ToolkitGdTest::testGifTransparentImages
  • \Drupal\Tests\Component\Annotation\Doctrine\DocParserTest::testSyntaxErrorWithUnknownCharacters
  • \Drupal\Tests\Core\Database\ConditionTest::dataProviderTestCompileWithKnownOperators

Where to find other issues

Some examples from #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard and the related issues:


+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
@@ -111,7 +111,7 @@ public static function open(array &$connection_options = []) {
   //elseif (phpversion('pdo_pgsql') < 'version_this_was_fixed_in') {

+++ b/core/modules/update/update.module
@@ -349,7 +349,7 @@ function update_get_available($refresh = FALSE) {
     //update_create_fetch_task($project);

+++ b/core/modules/user/src/Tests/Views/AccessRoleTest.php
@@ -128,8 +128,8 @@ public function testRenderCaching() {
   //$this->assertTrue(in_array('user.roles', $build['#cache']['contexts']));
   //$this->assertEqual([], $build['#cache']['tags']);

+++ b/core/modules/views/tests/src/Functional/Plugin/ArgumentDefaultTest.php
@@ -131,11 +131,7 @@ public function testArgumentDefaultFixed() {
 //function testArgumentDefaultPhp() {}
-
 /**
  * Test node default argument.
  */

+++ b/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php
@@ -353,7 +353,7 @@ public function testEntityAutocompleteAccess() {
   //$form = $form_builder->getForm($this);

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityTypeBundleInfoTest.php
@@ -100,7 +100,7 @@ protected function setUp() {
   //$container->get('typed_data_manager')->willReturn($this->typedDataManager->reveal());

Completed issues

🌱 Plan
Status

Active

Version

11.0 🔥

Component
Other 

Last updated about 11 hours ago

Created by

🇺🇸United States xjm

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇳🇿New Zealand quietone

    \Drupal\Tests\ban\Functional\IpAddressBlockingTest::testIPAddressValidation
    \Drupal\field\Plugin\migrate\process\d6\FieldInstanceDefaults::transform
    \Drupal\field\Plugin\migrate\process\d6\FieldInstanceSettings::transform
    \Drupal\views\Plugin\views\query\Sql::execute
    \Drupal\Tests\views\Functional\Handler\FieldWebTest::testAlterUrl
    \Drupal\Tests\views_ui\Functional\DefaultViewsTest::testDefaultViews
    \Drupal\FunctionalTests\Core\Recipe\StandardRecipeInstallTest::setUpSite
    \Drupal\KernelTests\Core\Entity\Element\EntityAutocompleteElementFormTest::testEntityAutocompleteIdInput
    \Drupal\KernelTests\Core\Image\ToolkitGdTest::testGifTransparentImages
    \Drupal\Tests\Component\Annotation\Doctrine\DocParserTest::testSyntaxErrorWithUnknownCharacters
    \Drupal\Tests\Core\Database\ConditionTest::dataProviderTestCompileWithKnownOperators

Production build 0.71.5 2024