[clang-format][PR47290] Add ShortNamespaceLines format option

clang-format documentation states that having enabled
FixNamespaceComments one may expect below code:

c++
namespace a {
foo();
}

to be turned into:

c++
namespace a {
foo();
} // namespace a

In reality, no "// namespace a" was added. The problem was too high
value of kShortNamespaceMaxLines, which is used while deciding whether
a namespace is long enough to be formatted.

As with 9163fe2, clang-format idempotence is preserved.

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

clang/docs/ClangFormatStyleOptions.rst

@@ -2210,14 +2210,16 @@ the configuration (without a prefix: ``Auto``).
   not use this in config files, etc. Use at your own risk.
 
 **FixNamespaceComments** (``bool``)
-  If ``true``, clang-format adds missing namespace end comments and
+  If ``true``, clang-format adds missing namespace end comments for
-  fixes invalid existing ones.
+  short namespaces and fixes invalid existing ones. Short ones are
+  controlled by "ShortNamespaceLines".
 
   .. code-block:: c++
 
      true:                                  false:
      namespace a {                  vs.     namespace a {
      foo();                                 foo();
+     bar();                                 bar();
      } // namespace a                       }
 
 **ForEachMacros** (``std::vector<std::string>``)
@@ -2367,7 +2369,7 @@ the configuration (without a prefix: ``Auto``).
   the record members, respecting the ``AccessModifierOffset``. Record
   members are indented one level below the record.
   When ``true``, access modifiers get their own indentation level. As a
-  consequence, record members are indented 2 levels below the record,
+  consequence, record members are always indented 2 levels below the record,
   regardless of the access modifier presence. Value of the
   ``AccessModifierOffset`` is ignored.
 
@@ -3040,43 +3042,72 @@ the configuration (without a prefix: ``Auto``).
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
       * information */
 
+**ShortNamespaceLines** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.
+  Defaults to 1.
+
+  This determines the maximum length of short namespaces by counting
+  unwrapped lines (i.e. containing neither opening nor closing
+  namespace brace) and makes "FixNamespaceComments" omit adding
+  end comments for those.
+
+  .. code-block:: c++
+
+     ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+     namespace a {                      namespace a {
+       int foo;                           int foo;
+     }                                  } // namespace a
+
+     ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+     namespace b {                      namespace b {
+       int foo;                           int foo;
+       int bar;                           int bar;
+     } // namespace b                   } // namespace b
+
 **SortIncludes** (``SortIncludesOptions``)
   Controls if and how clang-format will sort ``#includes``.
+  If ``Never``, includes are never sorted.
+  If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
+  insensitive fashion.
+  If ``CaseSensitive``, includes are sorted in an alphabetical or case
+  sensitive fashion.
 
-  Possible Values:
+  Possible values:
 
-  * ``SI_Never`` (in configuration ``Never``)
+  * ``SI_Never`` (in configuration: ``Never``)
     Includes are never sorted.
 
     .. code-block:: c++
 
-      #include "B/A.h"
+       #include "B/A.h"
-      #include "A/B.h"
+       #include "A/B.h"
-      #include "a/b.h"
+       #include "a/b.h"
-      #include "A/b.h"
+       #include "A/b.h"
-      #include "B/a.h"
+       #include "B/a.h"
 
-  * ``SI_CaseInsensitive`` (in configuration ``CaseInsensitive``)
+  * ``SI_CaseInsensitive`` (in configuration: ``CaseInsensitive``)
     Includes are sorted in an ASCIIbetical or case insensitive fashion.
 
     .. code-block:: c++
 
-      #include "A/B.h"
+       #include "A/B.h"
-      #include "A/b.h"
+       #include "A/b.h"
-      #include "B/A.h"
+       #include "B/A.h"
-      #include "B/a.h"
+       #include "B/a.h"
-      #include "a/b.h"
+       #include "a/b.h"
 
-  * ``SI_CaseSensitive`` (in configuration ``CaseSensitive``)
+  * ``SI_CaseSensitive`` (in configuration: ``CaseSensitive``)
     Includes are sorted in an alphabetical or case sensitive fashion.
 
     .. code-block:: c++
 
-      #include "A/B.h"
+       #include "A/B.h"
-      #include "A/b.h"
+       #include "A/b.h"
-      #include "a/b.h"
+       #include "a/b.h"
-      #include "B/A.h"
+       #include "B/A.h"
-      #include "B/a.h"
+       #include "B/a.h"
+
+
 
 **SortJavaStaticImport** (``SortJavaStaticImportOptions``)
   When sorting Java imports, by default static imports are placed before

clang/docs/ReleaseNotes.rst

@@ -203,6 +203,9 @@ clang-format
 - Option ``IndentAccessModifiers`` has been added to be able to give access
   modifiers their own indentation level inside records.
 
+- Option ``ShortNamespaceLines`` has been added to give better control
+  over ``FixNamespaceComments`` when determining a namespace length.
+
 libclang
 --------
 

clang/include/clang/Format/Format.h

@@ -1964,12 +1964,14 @@ struct FormatStyle {
   /// not use this in config files, etc. Use at your own risk.
   bool ExperimentalAutoDetectBinPacking;
 
-  /// If ``true``, clang-format adds missing namespace end comments and
+  /// If ``true``, clang-format adds missing namespace end comments for
-  /// fixes invalid existing ones.
+  /// short namespaces and fixes invalid existing ones. Short ones are
+  /// controlled by "ShortNamespaceLines".
   /// \code
   ///    true:                                  false:
   ///    namespace a {                  vs.     namespace a {
   ///    foo();                                 foo();
+  ///    bar();                                 bar();
   ///    } // namespace a                       }
   /// \endcode
   bool FixNamespaceComments;
@@ -2644,6 +2646,27 @@ struct FormatStyle {
   bool ReflowComments;
   // clang-format on
 
+  /// The maximal number of unwrapped lines that a short namespace spans.
+  /// Defaults to 1.
+  ///
+  /// This determines the maximum length of short namespaces by counting
+  /// unwrapped lines (i.e. containing neither opening nor closing
+  /// namespace brace) and makes "FixNamespaceComments" omit adding
+  /// end comments for those.
+  /// \code
+  ///    ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+  ///    namespace a {                      namespace a {
+  ///      int foo;                           int foo;
+  ///    }                                  } // namespace a
+  ///
+  ///    ShortNamespaceLines: 1     vs.     ShortNamespaceLines: 0
+  ///    namespace b {                      namespace b {
+  ///      int foo;                           int foo;
+  ///      int bar;                           int bar;
+  ///    } // namespace b                   } // namespace b
+  /// \endcode
+  unsigned ShortNamespaceLines;
+
   /// Include sorting options.
   enum SortIncludesOptions : unsigned char {
     /// Includes are never sorted.
@@ -3224,6 +3247,7 @@ struct FormatStyle {
                R.PenaltyBreakTemplateDeclaration &&
            PointerAlignment == R.PointerAlignment &&
            RawStringFormats == R.RawStringFormats &&
+           ShortNamespaceLines == R.ShortNamespaceLines &&
            SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&

clang/lib/Format/Format.cpp

@@ -642,6 +642,7 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("PointerAlignment", Style.PointerAlignment);
     IO.mapOptional("RawStringFormats", Style.RawStringFormats);
     IO.mapOptional("ReflowComments", Style.ReflowComments);
+    IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1006,6 +1007,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
+  LLVMStyle.ShortNamespaceLines = 1;
   LLVMStyle.SpacesBeforeTrailingComments = 1;
   LLVMStyle.Standard = FormatStyle::LS_Latest;
   LLVMStyle.UseCRLF = false;

clang/lib/Format/NamespaceEndCommentsFixer.cpp

@@ -22,10 +22,6 @@ namespace clang {
 namespace format {
 
 namespace {
-// The maximal number of unwrapped lines that a short namespace spans.
-// Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
-
 // Computes the name of a namespace given the namespace token.
 // Returns "" for anonymous namespace.
 std::string computeName(const FormatToken *NamespaceTok) {
@@ -283,7 +279,7 @@ std::pair<tooling::Replacements, unsigned> NamespaceEndCommentsFixer::analyze(
         computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
                               Style.SpacesInLineCommentPrefix.Minimum);
     if (!hasEndComment(EndCommentPrevTok)) {
-      bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+      bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
       if (!isShort)
         addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
     } else if (!validEndComment(EndCommentPrevTok, NamespaceName,

clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

@@ -816,7 +816,7 @@ TEST_F(NamespaceEndCommentsFixerTest,
                                     "}\n"));
 }
 
-TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+TEST_F(NamespaceEndCommentsFixerTest, AddsEndCommentForNamespacesAroundMacros) {
   // Conditional blocks around are fine
   EXPECT_EQ("namespace A {\n"
             "#if 1\n"
@@ -1116,6 +1116,75 @@ TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
                                     "}\n"
                                     "}\n"));
 }
+
+using ShortNamespaceLinesTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(ShortNamespaceLinesTest, ZeroUnwrappedLines) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 0u;
+
+  EXPECT_EQ("namespace OneLinerNamespace {}\n",
+            fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+  EXPECT_EQ("namespace ShortNamespace {\n"
+            "}\n",
+            fixNamespaceEndComments("namespace ShortNamespace {\n"
+                                    "}\n",
+                                    Style));
+  EXPECT_EQ("namespace LongNamespace {\n"
+            "int i;\n"
+            "}// namespace LongNamespace\n",
+            fixNamespaceEndComments("namespace LongNamespace {\n"
+                                    "int i;\n"
+                                    "}\n",
+                                    Style));
+}
+
+TEST_F(ShortNamespaceLinesTest, OneUnwrappedLine) {
+  constexpr auto DefaultUnwrappedLines = 1u;
+  auto const Style = getLLVMStyle();
+
+  EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+  EXPECT_EQ("namespace ShortNamespace {\n"
+            "int i;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace ShortNamespace {\n"
+                                    "int i;\n"
+                                    "}\n"));
+  EXPECT_EQ("namespace LongNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "}// namespace LongNamespace\n",
+            fixNamespaceEndComments("namespace LongNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest, MultipleUnwrappedLine) {
+  auto Style = getLLVMStyle();
+  Style.ShortNamespaceLines = 2u;
+
+  EXPECT_EQ("namespace ShortNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "}\n",
+            fixNamespaceEndComments("namespace ShortNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "}\n",
+                                    Style));
+  EXPECT_EQ("namespace LongNamespace {\n"
+            "int i;\n"
+            "int j;\n"
+            "int k;\n"
+            "}// namespace LongNamespace\n",
+            fixNamespaceEndComments("namespace LongNamespace {\n"
+                                    "int i;\n"
+                                    "int j;\n"
+                                    "int k;\n"
+                                    "}\n",
+                                    Style));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang