[clang] [clang-format] Add BreakBeforeTemplateClose option (PR #118046)

[via cfe-commits]

================
@@ -11220,6 +11220,333 @@ TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
       Style);
 }
 
+TEST_F(FormatTest, BreakBeforeTemplateCloser) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  // Begin with tests covering the case where there is no constraint on the
+  // column limit.
+  Style.ColumnLimit = 0;
+  // When BreakBeforeTemplateCloser is turned off, the line break that it adds
+  // shall be removed:
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Bar>\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+
+  Style.BreakBeforeTemplateCloser = FormatStyle::BBTCS_BlockIndent;
+  // BreakBeforeTemplateCloser should NOT force template declarations onto
+  // multiple lines.
+  verifyFormat("template <typename Foo>\n"
+               "void foo() {}",
+               Style);
+  verifyFormat("template <typename Foo, typename Bar>\n"
+               "void foo() {}",
+               Style);
+  // It should allow a line break, even when the typename is short.
+  // verifyNoChange is needed because the default behavior is one line.
+  verifyNoChange("template <\n"
+                 "    typename Foo\n"
+                 ">\n"
+                 "void foo() {}",
+                 Style);
+  verifyNoChange("template <\n"
+                 "    typename Foo,\n"
+                 "    typename Bar\n"
+                 ">\n"
+                 "void foo() {}",
+                 Style);
+  verifyNoChange("template <typename Foo,\n"
+                 "          typename Bar>\n"
+                 "void foo() {}",
+                 Style);
+  // It should add a line break before > if not already present:
+  verifyFormat("template <\n"
+               "    typename Foo\n"
+               ">\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo>\n"
+               "void foo() {}",
+               Style);
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar>\n"
+               "void foo() {}",
+               Style);
+  // When within an indent scope, the > should be placed accordingly:
+  verifyFormat("struct Baz {\n"
+               "  template <\n"
+               "      typename Foo,\n"
+               "      typename Bar\n"
+               "  >\n"
+               "  void foo() {}\n"
+               "};",
+               "struct Baz {\n"
+               "  template <\n"
+               "      typename Foo,\n"
+               "      typename Bar>\n"
+               "  void foo() {}\n"
+               "};",
+               Style);
+
+  // Test from issue #80049:
+  verifyFormat(
+      "void foo() {\n"
+      "  using type = std::remove_cv_t<\n"
+      "      add_common_cv_reference<\n"
+      "          std::common_type_t<std::decay_t<T0>, std::decay_t<T1>>,\n"
+      "          T0,\n"
+      "          T1\n"
+      "      >\n"
+      "  >;\n"
+      "}",
+      "void foo() {\n"
+      "  using type = std::remove_cv_t<\n"
+      "      add_common_cv_reference<\n"
+      "          std::common_type_t<std::decay_t<T0>, std::decay_t<T1>>,\n"
+      "          T0,\n"
+      "          T1>>;\n"
+      "}",
+      Style);
+
+  // Test lambda goes to next line:
+  verifyFormat("void foo() {\n"
+               "  auto lambda = []<\n"
+               "                    typename T\n"
+               "                >(T t) {\n"
+               "  };\n"
+               "}",
+               "void foo() {\n"
+               "  auto lambda = []<\n"
+               "  typename T>(T t){\n"
+               "  };\n"
+               "}",
+               Style);
+  // With no column limit, two parameters can go on the same line:
+  verifyFormat("void foo() {\n"
+               "  auto lambda = []<\n"
+               "                    typename T, typename Foo\n"
+               "                >(T t) {\n"
+               "  };\n"
+               "}",
+               "void foo() {\n"
+               "  auto lambda = []<\n"
+               "  typename T, typename Foo>(T t){\n"
+               "  };\n"
+               "}",
+               Style);
+  // Or on different lines:
+  verifyFormat("void foo() {\n"
+               "  auto lambda = []<\n"
+               "                    typename T,\n"
+               "                    typename Foo\n"
+               "                >(T t) {\n"
+               "  };\n"
+               "}",
+               "void foo() {\n"
+               "  auto lambda = []<\n"
+               "  typename T,\n"
+               "  typename Foo>(T t){\n"
+               "  };\n"
+               "}",
+               Style);
+
+  // Note that this is the same line (no \n):
+  verifyFormat("void foo() {\n"
+               "  auto lambda = []<typename "
+               "Looooooooooooooooooooooooooooong>("
+               "Looooooooooooooooooooooooooooong t) {};\n"
+               "}",
+               Style);
+
+  // Test template usage goes to next line too:
+  verifyFormat("void foo() {\n"
+               "  myFunc<\n"
+               "      T\n"
+               "  >();\n"
+               "}",
+               "void foo() {\n"
+               "  myFunc<\n"
+               "  T>();\n"
+               "}",
+               Style);
+
+  // Now test that it handles the cases when the column limit forces wrapping.
+  Style.ColumnLimit = 40;
+  // When the column limit allows it, the template should be combined back into
+  // one line:
+  verifyFormat("template <typename Foo, typename Bar>\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+  // But not when the name is looong. Note that these names are exactly 1
+  // character too long for the ColumnLimit.
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Barrrrrrrrrrrrrrrrrrrr\n"
+               ">\n"
+               "void foo() {}",
+               Style);
----------------
leijurv wrote:

Hm. Not really what I meant - I don't mind if it's a boolean or an enum, I guess slight preference for enum because I could imagine people have other opinions on when to break, similar to AlignAfterOpenBracket. So I didn't mean to say that I prefer a boolean.

All I meant to say is that the specific code that decides which of these happens:

```
template <typename Foo,
          typename Bar>
```

versus

```
template <
   typename Foo,
   typename Bar
>
```

was being inconsistent/confusing, and, I didn't want to make it consistent in this PR. Importantly, I feel this way because this PR is not intended to affect that behavior. I only intend to affect where the `>` goes in that second case. Not whether the first or second case is selected.

I really just meant to explain why I had removed some tests that were previously there.

I'm fine with this PR as-is, but if these last few messages have changed something for you, I'll change it back to boolean if you say so, but my preference (currently) is enum.

https://github.com/llvm/llvm-project/pull/118046