From Coding Standards http://drupal.org/coding-standards#operators

Operators

All binary operators (operators that come between two values), such as +, -, =, !=, ==, >, etc. should have a space before and after the operator, for readability. For example, an assignment should be formatted as $foo = $bar; rather than $foo=$bar;. Unary operators (operators that operate on only one value), such as ++, should not have a space between the operator and the variable or number they are operating on.

But I have a line like this

      '#size' => min(6, count($types)+1),

And coder does not bitch?

Comments

solotandem’s picture

Looking for well-formatted code. Try out the grammar parser module and see if it does not rewrite your code file according to coding standards.

sun’s picture

Component: Coder Format » Review/Rules
douggreen’s picture

Version: 6.x-2.0-beta1 » 7.x-2.x-dev

I am shocked that we miss this, but I just ran a few tests, and we do. I'll add some rules to 7.x-2.x-dev.

salvis’s picture

I like writing

a*b + c
a + b*c

over

a * b + c
a + b * c

Everyone who has ever gone to school has heard "multiplication and division first, then addition and subtraction" and grouping numerical expressions that way makes them easier to read and understand.

I've always appreciated this tiny bit of freedom that Coder has granted beyond our coding standards...

sun’s picture

@salvis: Heh. Our coding standards are crystal clear on that and this would be the wrong issue for discussing to change them. ;)

salvis’s picture

@sun: Would you agree with me, and do you think it might be worth opening an issue for such a discussion?

sun’s picture

@salvis: Not really ;) Pretty much -1 on the idea, because it would essentially be as inconsistent and complex to understand as our previous string concatenation standards in D6 and below. If you can describe a rule in three words "Spaces around operators." then that's KISS and that's best.

salvis’s picture

The D6 string concatenation standard was asymmetric depending on the operands! That was a completely different league!

Actually, the current rule is "Spaces around operators, except unary ones (except new, clone, and cast operators), the array operator, the scope resolution operator, and the member selector operator, unless the latter is chained and starts a new indented line."

Whenever someone reads a + b*c they intuitively grasp that the multiplication is carried out before the addition. There is no such clue in a + b * c and you have to consciously analyze the operator precedence to figure out what the expression means. The latter makes me want to add parentheses, the former is clear without them.

(Sorry about hijacking this issue — I'm stopping now.)

douggreen’s picture

I agree with @sun, we should not change the standards here. I actually prefer that you use parenthesis to show (and ensure) precedence. I think that this ticket is to enforce spacing per the current coding standards IRT mathematical expressions.

douggreen’s picture

Assigned: Unassigned » douggreen
thursday_bw’s picture

There is an inconsistency between the coding sniffs and the rest of coder module when it comes to string concatenation operators.

as per the standards here: https://drupal.org/coding-standards#concat

String Concatenations
Always use a space between the dot and the concatenated parts to improve readability.

Arguable, according to this rule the following code is valid, since there is 'a' space between the concatenated parts.

drupal_add_js(drupal_get_path('module', 'modulename') .    '/path/to/javascript/file.js');

And the coding sniffs do not flag this, however running drush coder-format on it generates the following diff

-  drupal_add_js(drupal_get_path('module', 'modulename') .    '/path/to/javascript/file.js');
+  drupal_add_js(drupal_get_path('module', 'modulename') . '/path/to/javascript/file.js');

I did say that this is arguably correct, but having said that I think it does mean
"Always use one and only one space between the dot and the concatenated parts to improve readability"

Either way, these two features are inconsistent.

In fact the file I have in question has a number of errors according the the sniffs, none of which are fixed by coder (fair enough, it can't write doc comments for us), however coder-format makes a number of changes to the file.

salvis’s picture

I'm not sure why the https://drupal.org/coding-standards#concat section exists at all. The dot operators ('.' and '.=') are covered by https://drupal.org/coding-standards#operators. The section was probably introduced to really drive home the change from D6, but it's not needed anymore.

Either way I would like to have multiple spaces accepted wherever one is requested.

Sometimes there is a structure in a multi-line code block that can be visualized by vertically aligning subexpressions. This can be helpful for the human reader (i.e. further "improve readability"), and collapsing the spaces would hurt readability.

I think we used to allow that explicitly for the => operator in array initializers, but I can't find the reference now. IAC, there are many places in core where we have that, e.g. function watchdog in bootstrap.inc.

thursday_bw’s picture

Well I totally agree that multiple spaces should be allowed.

Interesting that drush coder-format goes to the trouble to remove them. It don't mind all that much if it does.. I only use that command to clean already bad code.

Anything that has only one a two errors won't be touched by it.

Sound like it's really a simple fix of just remove that rewrite from the coder-format command.

klausi’s picture

Issue summary: View changes
Status: Active » Closed (won't fix)

Coder 7.x is frozen now and will not receive updates. Coder 8.x-2.x can be used to check code for any Drupal version, Coder 8.x-2.x also supports the phpcbf command to automatically fix conding standard errors. Please check if this issue is still relevant and reopen against that version if necessary.