clang-format: Support the Java 8 'default' method modifier

[MyDeveloperDay]

Summary

Java 8 introduced the use of using the 'default' keyword as modifier in
interface method declarations[1]. Previously it was being parsed as
being part of a label, which put the parser into a very weird state it
could not get out of.

This change adds support for 'default' by treating it as a normal
identifier in Java when the parser is expecting a declaration.

1: http://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html

[srhines]

Looks good, but I I want to make sure that someone else more familiar with this is ok with it too. Thanks.

[lhchavez]

Gentle ping?

[lodato]

I know nothing about the C++ code. I only know the git-clang-format script.

[djasper]

lib/Format/UnwrappedLineParser.cpp

297

Same as below.

865

Change the order here. I.e.

if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration)
  break;
...

I think then you almost don't even need the comment.

870

Is there a test case that hits this codepath? IIUC, it would need to have a "default" of a declaration that's not at the start of the line.

[lhchavez]

Addressed feedback

[lhchavez]

lib/Format/UnwrappedLineParser.cpp

870

Added two more test cases.

[djasper]

Looks good. Thank you!

[MyDeveloperDay]

While reviewing old revisions either not landed or unreviewed I stumbled across this accepted review. Looking at the current code I think its not needed anymore. Perhaps if we merge your test in with the current trunk we can determine if this is still needed

[MyDeveloperDay]

This revision is no needed, the test will pass without any additional code changes

lib/Format/UnwrappedLineParser.cpp

case tok::l_brace:
       break;
     case tok::r_brace:
       if (HasOpeningBrace)
         return;
       nextToken();
       addUnwrappedLine();
       break;
     case tok::kw_default:
+      if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration) {
+        parseStructuralElement();
+        break;
+      }
+      // fallthrough
     case tok::kw_case:
       if (!SwitchLabelEncountered &&
           (Style.IndentCaseLabels || (Line->InPPDirective && Line->Level == 1)))
         ++Line->Level;
       SwitchLabelEncountered = true;
       parseStructuralElement();
       break;
     default:
case tok::kw_while:
     return;
   case tok::kw_do:
     parseDoWhile();
     return;
   case tok::kw_switch:
     parseSwitch();
     return;
   case tok::kw_default:
+    if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration)
+      break;
     nextToken();
     parseLabel();
     return;
   case tok::kw_case:
     parseCaseLabel();
     return;
   case tok::kw_try:
   case tok::kw___try:
     parseTryCatch();
     return;
   case tok::kw_extern:
     nextToken();
     if (FormatTok->Tok.is(tok::string_literal)) {
       nextToken();
       if (FormatTok->Tok.is(tok::l_brace)) {
         parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);

unittests/Format/FormatTestJava.cpp

EXPECT_EQ("/*\n"
             "void f() {}",
             format("  /*\n"
                    "   * Really multi-line\n"
                    "   * comment.\n"
                    "   */\n"
                    "  void f() {}"));
 }
 
+TEST_F(FormatTestJava, UnderstandsDefaultModifier) {
+  verifyFormat("class SomeClass {\n"
+               "  default void f() {}\n"
+               "  public default void g() {}\n"
+               "  default public void h() {}\n"
+               "  int i() {\n"
+               "    switch (0) {\n"
+               "      case 0:\n"
+               "        break;\n"
+               "      default:\n"
+               "        break;\n"
+               "    }\n"
+               "  }\n"
+               "}",
+               getGoogleStyle(FormatStyle::LK_Java));
+}
+
 } // end namespace tooling
 } // end namespace clang