r233304 Regression: All ObjC field access lands on the new line

[Quuxplusone]

Bugzilla Link	PR23224
Status	NEW
Importance	P normal
Reported by	Adam Strzelecki (ono@java.pl)
Reported on	2015-04-14 07:16:53 -0700
Last modified on	2015-05-19 14:26:34 -0700
Version	trunk
Hardware	All All
CC	djasper@google.com, klimek@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

The commit to blame is r233304

  clang-format: Force line break in trailing calls after multline exprs.

Actually it forces line break on any dot access not only calls, which causes
severe code style regressions in our ObjC project.

Also I think such change should have appropriate format setting rather than
making it default.

Some regression examples:

   CGFloat height =
       [cell.contentView
-          systemLayoutSizeFittingSize:UILayoutFittingCompressedSize].height;
+          systemLayoutSizeFittingSize:UILayoutFittingCompressedSize]
+          .height;

-  NSDate *fileDate = [[NSFileManager defaultManager]
-                         attributesOfItemAtPath:jsonPath
-                                          error:NULL].fileModificationDate;
+  NSDate *fileDate =
+      [[NSFileManager defaultManager] attributesOfItemAtPath:jsonPath
+                                                       error:NULL]
+          .fileModificationDate;

-  NSDate *fileDate = [[NSFileManager defaultManager]
-                         attributesOfItemAtPath:pathForDefinitions
-                                          error:NULL].fileModificationDate;
+  NSDate *fileDate =
+      [[NSFileManager defaultManager] attributesOfItemAtPath:pathForDefinitions
+                                                       error:NULL]
+          .fileModificationDate;

   self.layer.borderColor = [UIColor colorWithHue:hue
                                       saturation:saturation
                                       brightness:brightness * .8
-                                           alpha:alpha].CGColor;
+                                           alpha:alpha]
+

[Quuxplusone]

All those changes are intended. The commit message is wrong. Should have been
"trailing member accesses" or something.

Now, I am not an ObjC programmer, so I don't know, but things like

  NSDate *fileDate = [[NSFileManager defaultManager]
                         attributesOfItemAtPath:pathForDefinitions
                                          error:NULL].fileModificationDate;

Is exactly what it is meant to prevent for C++ calls. There is almost no
visible structure and the statements gets very hard to understand.

[Quuxplusone]

> Now, I am not an ObjC programmer, so I don't know, but things like (...)

Unfortunately I am almost everyday ObjC programmer, and really haven't seen
anyone breaking before .dotAccess, just because:

  obj.property

has exactly same meaning (and it is simple syntax sugar for:

  [obj property]

So following code:

  CGFloat height =
      [cell.contentView
          systemLayoutSizeFittingSize:UILayoutFittingCompressedSize]
          .height;

Has exactly the same meaning that:

  CGFloat height = [[cell.contentView
      systemLayoutSizeFittingSize:UILayoutFittingCompressedSize] height];

Actually dot notation was introduced rather recently (ObjC v2.0), to express
calls that serve as property reads. But technically these are same calls
regardless of notation.

Still as you can see in example above clang-format gives them completely
different formatting in these two cases.

What's worse the clang-format r233304 change was described as improving builder
type calls formatting. But samples above are barely builder type calls. Dot
access purpose is to express exactly opposite - a property call, not builder
type call.

So if you want to be consequent we should have, when using non dot notation:

  CGFloat height =
      [[cell.contentView
          systemLayoutSizeFittingSize:UILayoutFittingCompressedSize]
          height];

Same applies to:

  NSDate *fileDate =
      [[NSFileManager defaultManager] attributesOfItemAtPath:jsonPath
                                                       error:NULL]
          .fileModificationDate;

which looks very very awkward to me as ObjC programmer.

Therefore ideally it would be to have some setting where I can simply turn it
off, language type call basis preferred, and off by default for ObjC calls, on
for C/C++ calls.