Incorrect:

$items['my/path']  = array();

Correct:

$items['my/path'] = array();

There is already a sniff that works for simple variable assignment:

  $id =  variable_get('variable_name', 'default');

Which produces:

  581 | ERROR   | [x] Expected 1 space after "="; 2 found

So it looks like it's not working for direct assignment to array keys.

Comments

psynaptic’s picture

Issue summary: View changes
klausi’s picture

This is difficult because we allow alignment of assignment lines, see https://drupal.org/coding-standards#functcall

So we would need an advanced sniff that checks if the next line and the line before also have an assignment and how that is indented. I think there is something similar already in the Squiz standard, maybe we could copy parts from there.

  • klausi committed 0cf8018 on 8.x-2.x
    feat(MultipleStatementAlignmentSniff): Add a sniff to check spacing...
klausi’s picture

Status: Active » Fixed

Pushed a new sniff.

pfrenssen’s picture

Status: Fixed » Needs work

Oh no, I'm afraid this work has been in vain because this coding standard changed recently in #2687941: [Policy, no patch] Within the Function Calls section, delete explicit mention of padding spacing in a block of related assignments. I wasn't aware of this issue or I could have closed it before you put all the effort in fixing it :(

Beforehand we had two conflicting sections in the coding standard: one that said that assignment operators should be preceded by "a space", and another section that specifically allowed multiple spaces for alignment purposes.

The section about aligning the operators has now been removed, and as mentioned in the issue in comment #2687941-36: [Policy, no patch] Within the Function Calls section, delete explicit mention of padding spacing in a block of related assignments, "a space" can currently be interpreted as "one or more spaces":

We discussed this, and at this time, the Coding Standards committee supports the interpretation that "should have a space" DOES NOT prohibit having more than one space.

A followup issue has been opened to further discuss and clarify this: #2816445: Forbid using more than one space around operators. Depending on the outcome of that issue the wording will be updated to "should have ONE space" or "should have at least one space" so there is no more confusion possible.

But at this very moment, developers are free to put as many spaces in front of their operators as they like, and are free to use any kind of alignment they like, they could even make christmas trees out of them if they so desire.

klausi’s picture

Status: Needs work » Fixed

Hehe, I took care of all the different allowed standards right now, so the sniff should be fine :)

Please test!

pfrenssen’s picture

I didn't test it myself but I saw from the code that this:

--- /dev/null
+++ b/coder_sniffer/Drupal/Test/Formatting/MultipleStatementAlignmentUnitTest.inc
+$c   = 1;
+$dd   = 2;

... now gets autofixed to this:

--- /dev/null
+++ b/coder_sniffer/Drupal/Test/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed
+$c  = 1;
+$dd = 2;

... but according to the current coding standards any spacing before the assignment operator is now allowed, so this case doesn't need to be flagged as an error, and it doesn't need autofixing.

The part about the aligning has been removed from the coding standards, so having any code that will autofix them to be aligned is not going to be necessary any more.

Depending on the outcome of #2816445: Forbid using more than one space around operators we will either have one single space (no alignment allowed), or any random number of spaces allowed (alignment allowed but not enforced).

I have the feeling that #2816445: Forbid using more than one space around operators might take a very long time to reach consensus, or it might not even be reached at all since people tend to be very polarized about their personal spacing preferences. We noticed this as well in the previous issue which is why we settled on allowing people to space however they want for the time being.

Edit: anyway, personally I think this change will always result in cleaner code so I'm OK for leaving it like this for the time being, at least until #2816445 is concluded.

The spirit of the coding standards is to let people choose single spacing or aligning for their personal projects and not impose any limitation on that. That's why we allow the freedom for now. I think practically everyone will be fine with how it works now. If someone might have a good argument for a 'christmas tree style' operator alignment, they can still open a new issue.

klausi’s picture

Arbitrary spacing before the alignment operator does not make sense. Either you use 1 space or you align our assignments.

In case people use weird spacing it defaults to fix it to aligned assignments right now.

Status: Fixed » Closed (fixed)

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