[clang-format] Do not break after ObjC category open paren

[benhamilton]

Summary

Previously, clang-format would break Objective-C
category extensions after the opening parenthesis to avoid
breaking the protocol list:

% echo "@interface ccccccccccccc (ccccccccccc) <ccccccccccccc> { }" | \
  clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface ccccccccccccc (
    ccccccccccc) <ccccccccccccc> {
}

This looks fairly odd, as we could have kept the category extension
on the previous line.

Category extensions are a single item, so they are generally very
short compared to protocol lists. We should prefer breaking after the
opening < of the protocol list over breaking after the opening (
of the category extension.

With this diff, we now avoid breaking after the category extension's
open paren, which causes us to break after the protocol list's
open angle bracket:

% echo "@interface ccccccccccccc (ccccccccccc) <ccccccccccccc> { }" | \
  ./bin/clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface ccccccccccccc (ccccccccccc) <
    ccccccccccccc> {
}

Test Plan: New test added. Confirmed test failed before diff and

passed after diff by running:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

[djasper]

Both patch and comment inside patch say "break after closing paren" and yet that's not what the test or example in the description actually do. Why is that?

[benhamilton]

Fair point. I'll update the diff description.

The net effect of the patch is to break after the closing paren; the way I
implemented that was to raise the penalty for breaking after the opening
paren.

[djasper]

I understand that, but the test example does not break after the closing paren. It breaks after the subsequent "<".

[benhamilton]

True. I will reword the description to clarify.

[djasper]

Looks good.

lib/Format/TokenAnnotator.cpp

2276 ↗

(On Diff #142033)

This is still using the old wording.

[benhamilton]

• Update comment.

lib/Format/TokenAnnotator.cpp

2276 ↗

(On Diff #142033)

Fixed, thanks.

cfe/trunk/lib/Format/TokenAnnotator.cpp

unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
 
   // In Objective-C method expressions, prefer breaking before "param:" over
   // breaking after it.
   if (Right.is(TT_SelectorName))
     return 0;
   if (Left.is(tok::colon) && Left.is(TT_ObjCMethodExpr))
     return Line.MightBeFunctionDecl ? 50 : 500;
 
+  // In Objective-C type declarations, avoid breaking after the category's
+  // open paren (we'll prefer breaking after the protocol list's opening
+  // angle bracket, if present).
+  if (Line.Type == LT_ObjCDecl && Left.is(tok::l_paren) && Left.Previous &&
+      Left.Previous->isOneOf(tok::identifier, tok::greater))
+    return 500;
+
   if (Left.is(tok::l_paren) && InFunctionDecl &&
       Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
     return 100;
   if (Left.is(tok::l_paren) && Left.Previous &&
       (Left.Previous->isOneOf(tok::kw_if, tok::kw_for) ||
        Left.Previous->endsSequence(tok::kw_constexpr, tok::kw_if)))
     return 1000;
   if (Left.is(tok::equal) && InFunctionDecl)

cfe/trunk/unittests/Format/FormatTestObjC.cpp

verifyFormat("@interface Foo (HackStuff) <MyProtocol> {\n"
                "+ (id)init;\n"
                "@end");
 
   Style.ColumnLimit = 40;
   verifyFormat("@interface ccccccccccccc () <\n"
                "    ccccccccccccc, ccccccccccccc,\n"
                "    ccccccccccccc, ccccccccccccc> {\n"
                "}");
+  verifyFormat("@interface ccccccccccccc (ccccccccccc) <\n"
+               "    ccccccccccccc> {\n"
+               "}");
   Style.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
   verifyFormat("@interface ddddddddddddd () <\n"
                "    ddddddddddddd,\n"
                "    ddddddddddddd,\n"
                "    ddddddddddddd,\n"
                "    ddddddddddddd> {\n"
                "}");