[clang-format] square parens that are followed by '=' are not Objective-C message sends

[arphaman]

Summary

The commit r322690 introduced support for ObjC detection in header files. Unfortunately some C headers that use designated initializers are now incorrectly detected as Objective-C.
This patch fixes it by ensuring [ ... ] is not annotated as an Objective-C message send when the expression is followed by = as it's not a commonly used Objective-C pattern.

rdar://45504376

[jkorous]

Hi Alex, thanks for fixing this.

I assume chance of testing an Objective-C header with assignment to method result without any other specifically Objective-C (detectable) construct is significantly lower than chance of testing a C99 header with designated initializers. In other words - having potential for false Objective-C negatives in parseSquare() is probably fine given the context. Is that right?

If my assumption is correct I'd say LGTM unless anyone says otherwise within 1 week.

[dexonsmith]

Note that a message send needs to have two expressions without an operator in between. Can we also rule out all square brackets that just have a single identifier or number token?

[jkorous]

I was thinking the same (that we might be able to make the detection more precise) but I got the impression these cases are handled later (L524 and below).

[arphaman]

Do not annotate one token [ .... ] expressions as Objective-C message sends as suggested by Duncan.

[benhamilton]

lib/Format/TokenAnnotator.cpp

501

I think you need to check if CurrentToken->Previous is null before dereferencing it.

503

I think it's actually required to have more than one token. Can we reduce the change to that?

[arphaman]

Update for Ben's comments.

lib/Format/TokenAnnotator.cpp

bool parseSquare() {
 
     Contexts.back().ColonIsObjCMethodExpr = StartsObjCMethodExpr;
     Contexts.back().InCpp11AttributeSpecifier = IsCpp11AttributeSpecifier;
 
     while (CurrentToken) {
       if (CurrentToken->is(tok::r_square)) {
         if (IsCpp11AttributeSpecifier)
           CurrentToken->Type = TT_AttributeSquare;
-        else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+        else if (CurrentToken->Next &&
+                 (CurrentToken->Next->is(tok::l_paren) ||
+                  CurrentToken->Next->is(tok::equal)) &&
                  Left->is(TT_ObjCMethodExpr)) {
-          // An ObjC method call is rarely followed by an open parenthesis.
+          // An ObjC method call is rarely followed by an open parenthesis or
+          // an assignment.
           // FIXME: Do we incorrectly label ":" with this?
           StartsObjCMethodExpr = false;
           Left->Type = TT_Unknown;
         }
         if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
           CurrentToken->Type = TT_ObjCMethodExpr;
           // If we haven't seen a colon yet, make sure the last identifier
           // before the r_square is tagged as a selector name component.

unittests/Format/FormatTestObjC.cpp

TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
   Style = getStyle("{}", "a.h", "none", "NSSet *Foo();\n");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 }
 
+TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) {
+  auto Style = getStyle("LLVM", "a.h", "none",
+                        "static const char *names[] = {[0] = \"foo\",\n"
+                        "[kBar] = \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+}
+
 TEST_F(FormatTestObjC, FormatObjCTryCatch) {
   verifyFormat("@try {\n"
                "  f();\n"
                "} @catch (NSException e) {\n"
                "  @throw;\n"
                "} @finally {\n"
                "  exit(42);\n"
                "}");