When an object method is wrapped onto a second line it is supposed to be indented two spaces.

e.g.:

  $query = db_select('node');
  $query->condition('type', 'article')
    ->condition('status', 1)
    ->execute();

The following should throw an error:

  $query = db_select('node');
  $query->condition('type', 'article')
      ->condition('status', 1)
      ->execute();

Currently it doesn't seem to detect this issue.

Comments

DamienMcKenna created an issue. See original summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Status: Active » Postponed (maintainer needs more info)

I think there is no coding standard specified for this - could you link to a place at https://www.drupal.org/coding-standards or subpages that specifies this?

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

You are correct, so I opened an issue in the Coding Standards issue queue: #2690599: [policy, no patch] Standardize indenting on chained method calls

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Postponed (maintainer needs more info) » Needs work

#2690599: [policy, no patch] Standardize indenting on chained method calls has agreed on 2 spaces.

  • klausi committed a775d0f on 8.x-2.x 
    Issue #2690325 part 1: re-enable object operator indentation sniff
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented

I pushed a start for this, but we will need proper testing for ObjectOperatorIndentSniff.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented

This now throws a false positive on this structure:

    $membership
      ->setUser($user)
      ->setGroup($collection)
      ->setState(OgMembershipInterface::STATE_ACTIVE)
      ->setRoles([OgRole::load($role_id)])
      ->save();

I get a false positive "Object operator not indented correctly; expected 8 spaces but found 6"

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented
Assigned: Unassigned » pfrenssen

Started on a PR: https://github.com/klausi/coder/pull/10

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented

I am having some failures that seem to be due to a bug in PHP_CodeSniffer. @Klausi maybe you have already encountered this before? The nested_parenthesis seems to be incorrect. I'll explain more in detail on the PR.

edit: there is no bug in PHP_CodeSniffer, I'm stupid.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented
Assigned: pfrenssen » Unassigned
Category: Bug report » Feature request
Status: Needs work » Needs review

PR is ready, I have done the following:

  • Fixed the false positive from comment #7.
  • Added test coverage.
  • Added auto-fixers for the indentation, as well as for the sniff that detects if the object operator (->) is at the end of the line.

Changing this also to a feature request, this is not a bug but rather an implementation of a newly defined coding standards.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Category: Feature request » Bug report
Status: Needs review » Needs work

Thanks, left a couple of comments on the PR.

It looks like your test case from above with $membership is missing?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Category: Bug report » Feature request

didn't mean to change the category.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented
Status: Needs work » Needs review

Thanks for the lightning fast review!! I fixed the remarks.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen as a volunteer commented

It looks like your test case from above with $membership is missing?

This is actually covered by this part in the test, my example was from Organic Groups, but I replaced it with some core code that exhibits the same problem, this is more recognizable for people unfamiliar with OG:

  $result = $entityTypeManager
    ->getStorage($this->entityTypeId)
    ->loadMultiple($this->ids);

  • klausi committed 848e79d on 8.x-2.x authored by pfrenssen 
    Issue #2690325: part 2: fixed sniff to check for indentation of chained...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Status: Needs review » Fixed

Merged, thanks!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Status: Fixed » Needs work

Tested it a bit on core, identified some false positives. Thinking about rewriting the whole sniff, work in progress at https://github.com/klausi/coder/pull/13

  • klausi committed 1642ede on 8.x-2.x 
    Issue #2690325: Fixed false positives with special object operator...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi as a volunteer commented
Status: Needs work » Fixed

Rewrote the sniff and all fixes for Drupal core look good now.

Status: Fixed » Closed (fixed)

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