Skip to content

Commit

Permalink
[clang-format][PR47290] Add ShortNamespaceLines format option
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@kuzkry @HazardyKnusperkeks
kuzkry authored and HazardyKnusperkeks committed Mar 1, 2021
1 parent 418b4a7 commit 6ca5281
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 30 deletions.
75 changes: 53 additions & 22 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
fixes invalid existing ones.
If ``true``, clang-format adds missing namespace end comments for
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>``)
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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 "A/B.h"
#include "a/b.h"
#include "A/b.h"
#include "B/a.h"
#include "B/A.h"
#include "A/B.h"
#include "a/b.h"
#include "A/b.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 "B/A.h"
#include "B/a.h"
#include "a/b.h"
#include "A/B.h"
#include "A/b.h"
#include "B/A.h"
#include "B/a.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 "B/A.h"
#include "B/a.h"
#include "A/B.h"
#include "A/b.h"
#include "a/b.h"
#include "B/A.h"
#include "B/a.h"



**SortJavaStaticImport** (``SortJavaStaticImportOptions``)
When sorting Java imports, by default static imports are placed before
Expand Down
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------

Expand Down
28 changes: 26 additions & 2 deletions clang/include/clang/Format/Format.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// fixes invalid existing ones.
/// If ``true``, clang-format adds missing namespace end comments for
/// 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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 &&
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Format/Format.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions clang/lib/Format/NamespaceEndCommentsFixer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
71 changes: 70 additions & 1 deletion clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

0 comments on commit 6ca5281

Please sign in to comment.