[clang-format] Improve Incomplete detection for (text) protos

Summary:
This patch improves detection of incomplete code for protos and text protos.
This is especially important for text protos in raw string literals, since they
might be partial strings concatenated, and we'd like to disable formatting in
these cases.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D44203

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330016 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 49e21520b702e44fbf2603956fcbaedaa04209d2..f76ae86b4c6211c70c7d7ddb9b8fe83d4e471426 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -592,7 +592,8 @@ private:
           return false;
       }
     }
-    return true;
+    // There are no top-level unbalanced braces in text protos.
+    return Style.Language != FormatStyle::LK_TextProto;
   }
 
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
@@ -712,6 +713,11 @@ private:
       } else if (Contexts.back().ContextKind == tok::l_paren) {
         Tok->Type = TT_InlineASMColon;
       }
+      // Detects trailing pieces like key:
+      if ((Style.Language == FormatStyle::LK_Proto ||
+           Style.Language == FormatStyle::LK_TextProto) &&
+          !CurrentToken)
+        return false;
       break;
     case tok::pipe:
     case tok::amp:
@@ -798,6 +804,10 @@ private:
           if (Previous && Previous->Type != TT_DictLiteral)
             Previous->Type = TT_SelectorName;
         }
+      } else if (Style.Language == FormatStyle::LK_TextProto ||
+                 Style.Language == FormatStyle::LK_Proto) {
+        // In TT_Proto and TT_TextProto, tok::less cannot be a binary operator.
+        return false;
       } else {
         Tok->Type = TT_BinaryOperator;
         NonTemplateLess.insert(Tok);
@@ -809,13 +819,16 @@ private:
     case tok::r_square:
       return false;
     case tok::r_brace:
-      // Lines can start with '}'.
-      if (Tok->Previous)
+      // Lines can start with '}' except in text protos.
+      if (Tok->Previous || Style.Language == FormatStyle::LK_TextProto)
         return false;
       break;
     case tok::greater:
-      if (Style.Language != FormatStyle::LK_TextProto)
-        Tok->Type = TT_BinaryOperator;
+      // In protos and text protos tok::greater cannot be a binary operator.
+      if (Style.Language == FormatStyle::LK_Proto ||
+          Style.Language == FormatStyle::LK_TextProto)
+        return false;
+      Tok->Type = TT_BinaryOperator;
       break;
     case tok::kw_operator:
       if (Style.Language == FormatStyle::LK_TextProto ||

diff --git a/unittests/Format/FormatTestProto.cpp b/unittests/Format/FormatTestProto.cpp
index 4b1480b074aa76eb8ce80b30d884c821389758e3..8634a36b9d96ef3c711e2bd0b6c9f96c8b9e496f 100644 (file)
--- a/unittests/Format/FormatTestProto.cpp
+++ b/unittests/Format/FormatTestProto.cpp
@@ -18,13 +18,21 @@ namespace clang {
 namespace format {
 
 class FormatTestProto : public ::testing::Test {
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete };
+
 protected:
   static std::string format(llvm::StringRef Code, unsigned Offset,
-                            unsigned Length, const FormatStyle &Style) {
+                            unsigned Length, const FormatStyle &Style,
+                            StatusCheck CheckComplete = SC_ExpectComplete) {
     DEBUG(llvm::errs() << "---\n");
     DEBUG(llvm::errs() << Code << "\n\n");
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
-    tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+    FormattingAttemptStatus Status;
+    tooling::Replacements Replaces =
+        reformat(Style, Code, Ranges, "<stdin>", &Status);
+    bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
+    EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
+        << Code << "\n\n";
     auto Result = applyAllReplacements(Code, Replaces);
     EXPECT_TRUE(static_cast<bool>(Result));
     DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
@@ -41,6 +49,12 @@ protected:
     EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
     EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
+
+  static void verifyIncompleteFormat(llvm::StringRef Code) {
+    FormatStyle Style = getGoogleStyle(FormatStyle::LK_Proto);
+    EXPECT_EQ(Code.str(),
+              format(Code, 0, Code.size(), Style, SC_ExpectIncomplete));
+  }
 };
 
 TEST_F(FormatTestProto, FormatsMessages) {
@@ -492,5 +506,12 @@ TEST_F(FormatTestProto, AcceptsOperatorAsKeyInOptions) {
                "};");
 }
 
+TEST_F(FormatTestProto, IncompleteFormat) {
+  verifyIncompleteFormat("option (");
+  verifyIncompleteFormat("option (MyProto.options) = { bbbbbbbbb:");
+  verifyIncompleteFormat("option (MyProto.options) = { bbbbbbbbb: <\n");
+  verifyIncompleteFormat("option (MyProto.options) = { bbbbbbbbb: [\n");
+}
+
 } // end namespace tooling
 } // end namespace clang

diff --git a/unittests/Format/FormatTestTextProto.cpp b/unittests/Format/FormatTestTextProto.cpp
index 4e4387b9d382654a46d1d101314cd2884e40d60f..608059c9ea860ebfb41b0be1d5154d741dbff4e3 100644 (file)
--- a/unittests/Format/FormatTestTextProto.cpp
+++ b/unittests/Format/FormatTestTextProto.cpp
@@ -19,12 +19,20 @@ namespace format {
 
 class FormatTestTextProto : public ::testing::Test {
 protected:
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete };
+
   static std::string format(llvm::StringRef Code, unsigned Offset,
-                            unsigned Length, const FormatStyle &Style) {
+                            unsigned Length, const FormatStyle &Style,
+                            StatusCheck CheckComplete = SC_ExpectComplete) {
     DEBUG(llvm::errs() << "---\n");
     DEBUG(llvm::errs() << Code << "\n\n");
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
-    tooling::Replacements Replaces = reformat(Style, Code, Ranges);
+    FormattingAttemptStatus Status;
+    tooling::Replacements Replaces =
+        reformat(Style, Code, Ranges, "<stdin>", &Status);
+    bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
+    EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
+        << Code << "\n\n";
     auto Result = applyAllReplacements(Code, Replaces);
     EXPECT_TRUE(static_cast<bool>(Result));
     DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
@@ -45,6 +53,12 @@ protected:
     Style.ColumnLimit = 60; // To make writing tests easier.
     verifyFormat(Code, Style);
   }
+
+  static void verifyIncompleteFormat(llvm::StringRef Code) {
+    FormatStyle Style = getGoogleStyle(FormatStyle::LK_TextProto);
+    EXPECT_EQ(Code.str(),
+              format(Code, 0, Code.size(), Style, SC_ExpectIncomplete));
+  }
 };
 
 TEST_F(FormatTestTextProto, KeepsTopLevelEntriesFittingALine) {
@@ -495,5 +509,38 @@ TEST_F(FormatTestTextProto, PutsMultipleEntriesInExtensionsOnNewlines) {
                "}", Style);
 }
 
+TEST_F(FormatTestTextProto, IncompleteFormat) {
+  verifyIncompleteFormat("data {");
+  verifyIncompleteFormat("data <");
+  verifyIncompleteFormat("data [");
+  verifyIncompleteFormat("data: {");
+  verifyIncompleteFormat("data: <");
+  verifyIncompleteFormat("data: [");
+  verifyIncompleteFormat("key:");
+  verifyIncompleteFormat("key:}");
+  verifyIncompleteFormat("key: ]");
+  verifyIncompleteFormat("key: >");
+  verifyIncompleteFormat(": value");
+  verifyIncompleteFormat(": {}");
+  verifyIncompleteFormat(": <>");
+  verifyIncompleteFormat(": []");
+  verifyIncompleteFormat("}\n"
+                         "key: value");
+  verifyIncompleteFormat("]\n"
+                         "key: value");
+  verifyIncompleteFormat("> key: value");
+  verifyIncompleteFormat("data { key: {");
+  verifyIncompleteFormat("data < key: [");
+  verifyIncompleteFormat("data [ key: {");
+  verifyIncompleteFormat("> key: value {");
+  verifyIncompleteFormat("> key: [");
+  verifyIncompleteFormat("}\n"
+                         "key: {");
+  verifyIncompleteFormat("data { key: 1 id:");
+  verifyIncompleteFormat("}\n"
+                         "key {");
+  verifyIncompleteFormat("> <");
+}
+
 } // end namespace tooling
 } // end namespace clang