Problem/Motivation

There is a super fast PHP formatter written in Rust: mago https://github.com/carthage-software/mago

I'm currently working on the Drupal configuration for mago: https://github.com/carthage-software/mago/issues/854

Problem: some formatted code by mago is flagged by Coder, for example around arrays. The output that mago creates does not strictly violate Drupal Coding standards, so we should relax rules in Coder. That way Coder and mago are compatible and not contradicting each other.

Example code that has been formatted by mago and fails with Coder, but it should not fail:

// Test mago compatibility with single array value over multiple lines.
$response = new AjaxResponse();
$response->addCommand(new InvokeCommand('#edit-field-job-expiration-date-0-value-date', 'val', [$end_value->format(
  'Y-m-d',
)]));

foreach ($result['subscriptions'] as $subscription) {
  $group = $subscription->organization->entity;
  $context->addCacheableDependency($group);
  $context->addCacheableDependency($subscription);
  $context->addCacheTags([
    'node:'
      . \Drupal::service('example_organization.organization_helper')->getGroupOrganizationProfile($group)->id(),
  ]);
}

$form['frontend'] = [
  '#type' => 'select',
  '#required' => FALSE,
  '#title' => $this->t('Frontend'),
  '#description_display' => $this->t('Select the frontend'),
  '#options' =>
    ['all' => $this->t('All menus'), 'default' => $this->t('Default menus')]
    + example_common_get_available_frontends(),
  '#default_value' => $this->getRequest()->query->get('frontend') ?? 'all',
  '#name' => 'selected-frontend',
];

\Drupal::database()
  ->insert(DataManager::DATA_TABLE)
  ->fields(['uid', 'last_sent', 'reminders'], [
    $user->id(),
    \Drupal::time()->getCurrentTime()
      - (($i + 1) * (int) \Drupal::config('example_auth.settings')->get('email_verification_timeout')),
    $i,
  ])
  ->execute();

static::setMessage(t('The combined export file could not be created: @message.', ['@message' =>
  $e->getMessage()]), 'error');

Steps to reproduce

Run "mago format" on a Drupal code base. Then run Coder on the code base. Observe that Coder now flags some code that is strictly not a coding standard violation.

Proposed resolution

Relax rules in Coder so that mago formatted code passes.

Remaining tasks

Merge request

API changes

none

Issue fork coder-3565827

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

klausi created an issue. See original summary.

bramdriesen’s picture

Can you post a difference between what Mago formats and what coder is observing as "faulty"?

klausi’s picture

Issue summary: View changes

Done, added examples to the issue summary.

Those are also used in the merge request in the good.php test.

klausi’s picture

Issue summary: View changes

fixed typo

  • klausi committed 4c28be22 on 9.x
    fix(Array): Adapt array formatting and class declaration rules for mago...
klausi’s picture

Merged this first step, still comparing what else we need to fix. Keeping this issue open.

wim leers’s picture

👀 Interested in this, to reduce review friction/increase velocity on https://www.drupal.org/project/canvas

klausi’s picture

The Mago Drupal preset is proposed at https://github.com/carthage-software/mago/pull/905

Note that there are still some incompatibilities between Mago and Coder. Coder will report some reformatted code from Mago as bad, so we need to fix that in Coder or Mago. We can decide on a case-by-case basis which tool is more wrong about it :-)

  • klausi committed f409dc10 on 9.x
    fix(Array): Skip multi line function calls in arrays, ignore multi-line...
klausi’s picture

Pushed some more fixes.

A big show-stopper is that the Mago formatter completely ignores doc comments, not formatting them as Coder with phpcbf does. Opened https://github.com/carthage-software/mago/issues/906 to ask the Mago devs if they would accept formatting doc comments.

klausi’s picture

Component: Coder Sniffer » Code
Status: Active » Fixed

I will change the approach with Mago compatibility a bit, since I don't care about minor indentation mismatches between Mago and Coder.

Instead of making Mago and PHPCS compatible I'm proposing to disable sniffs that mago already covers in a new DrupalMago standard. New issue is #3585617: Create a new DrupalMago PHPCS standard.

I think we can call this issue done.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.