[clang-format] Keep protobuf "package" statement on one line

[dchai]

Summary

Top-level "package" and "import" statements should generally be kept on one
line, for all languages.

[MyDeveloperDay]

lib/Format/TokenAnnotator.cpp

1079 ↗

(On Diff #191627)

Nit:

could we use

CurrentToken->isOneOf(Keywords.kw_option,Keyswords.kw_package)

[dchai]

Use "isOneOf" instead of two calls to "is".

[MyDeveloperDay]

LGTM

[hokein]

This patch has caused a regression issue, see the test:

verifyFormat("// Detached comment\n\n"
                 "// Leading comment\n"
                 "syntax = \"proto2\"; // trailing comment\n\n"
                 "// in foo.bar package\n"
                 "package foo.bar; // foo.bar package\n");

I have reverted it in rL356912.

[dchai]

Just to confirm, the regression is in the number of spaces before a trailing comment?

Before (2 spaces):

package foo.bar;  // foo.bar package

After (1 space):

package foo.bar; // foo.bar package

Top-level options are handled the same way. I'll see if I can address these both.

cfe/trunk/lib/Format/TokenAnnotator.cpp

LineType parseLine() {
 
     // If this line starts and ends in '<' and '>', respectively, it is likely
     // part of "#define <a/b.h>".
     if (CurrentToken->is(tok::less) && Line.Last->is(tok::greater)) {
       parseIncludeDirective();
       return LT_ImportStatement;
     }
 
-    // In .proto files, top-level options are very similar to import statements
-    // and should not be line-wrapped.
+    // In .proto files, top-level options and package statements are very
+    // similar to import statements and should not be line-wrapped.
     if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
-        CurrentToken->is(Keywords.kw_option)) {
+        CurrentToken->isOneOf(Keywords.kw_option, Keywords.kw_package)) {
       next();
       if (CurrentToken && CurrentToken->is(tok::identifier))
         return LT_ImportStatement;
     }
 
     bool KeywordVirtualFound = false;
     bool ImportStatement = false;
 

cfe/trunk/unittests/Format/FormatTestProto.cpp

TEST_F(FormatTestProto, FormatsOptions) {
   verifyFormat("option (MyProto.options) = {\n"
                "  app_id: 'com.javax.swing.salsa.latino'\n"
                "  head_id: 1\n"
                "  headheadheadheadheadhead_id: 1\n"
                "  product_data { product { 1 } }\n"
                "};");
 }
 
+TEST_F(FormatTestProto, DoesntWrapPackageStatements) {
+  verifyFormat(
+      "package"
+      " some.really.long.package.that.exceeds.the.column.limit00000000;");
+}
+
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
                "  rpc Search(SearchRequest) returns (SearchResponse) {\n"
                "    option foo = true;\n"
                "  }\n"
                "};");
 }