I've the code like:

/**
 * Topic index callback.
 */
function postcodeanywhere_index_page() {
  /*
  $output = theme('advanced_help_topic', array(
    'module' => 'help_example',
    'topic' => 'about-php',
  ));
  */
  $output = '';
  $output .= ' ' . t('Click the help icon to view help about the PCA Predict (formely Postcode Anywhere).');
  return $output;
}

However when running: phpcs --standard=Drupal,DrupalPractice postcodeanywhere.module it complains with ERRORs:

ERROR | [x] Line indented incorrectly; expected 2 spaces,
| found 4
ERROR | [x] Line indented incorrectly; expected 2 spaces,
| found 4
ERROR | [x] Line indented incorrectly; expected 3 spaces,
| found 2

which points to the lines with multi-line comment. Secondly, I think it should be a warning.

Comments

kenorb created an issue. See original summary.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman at Acquia commented

I am seeing this as well.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at European Commission and European Union Institutions, Agencies and Bodies commented

I personally wouldn't bother adding the complexity needed to support indentation of commented out code. It's a bad practice to commit commented out code, we shouldn't encourage this.

Commented out code should be temporary anyway, just slap // @codingStandardsIgnoreStart and // @codingStandardsIgnoreEnd around it and call it a day.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi at jobiqo - job board technology commented
Status: Active » Postponed (maintainer needs more info)

Agreed, you shouldn't have commented out code in the first place and for the exceptions we can use @codingStandardsIgnoreStart. Is that good enough for you?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi at jobiqo - job board technology commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Closing this for now, please reopen if you have more input.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Closed (won't fix) » Active

In #2868078: Use new array syntax in PHP files outside of /core. there's a proposal to include sites/default/default.settings.php in the PHPCS check and that seems to be OK. But the file contains commented-out code and this is also OK because that file is like a dist file. So, at least in core, we will, probably, have such cases.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at European Commission and European Union Institutions, Agencies and Bodies commented

default.settings.php has a valid use case for containing commented out code since it contains examples for different environments. But this is a very rare exception, I would just use @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd for that.

Coder is intended to make sure any tested code adheres to the coding standards and there is no mention in the coding standards on how to format commented out code. So technically we cannot adopt it until the coding standards change. That is not likely to happen since committing commented out code is bad practice.

I think this is just something we need to accept. Core is big and there will be exceptions. There are some other coding standards that are also willfully broken by core. For example Views plugins don't follow the right naming conventions. This is also not fixable without breaking backwards compatibility.

In the end that's fine, for those rare edge cases we can use @codingStandardsIgnoreStart or add an exception to the ruleset, these mechanisms exist for that reason.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at European Commission and European Union Institutions, Agencies and Bodies commented

As a side note for people reading this and looking for ways to commit their commented out code and are not happy with "Don't do it!": if you comment out your code using hashes, then Coder doesn't complain:

# if (file_exists($app_root . '/' . $site_path . '/settings.local.php')) {
#   include $app_root . '/' . $site_path . '/settings.local.php';
# }

So this can also be used as an alternative to @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd.