Skip to content

Commit 9163fe2

Browse files
krasimirggkrasimirgg
committed
[clang-format] Use number of unwrapped lines for short namespace
Summary: This patch makes the namespace comment fixer use the number of unwrapped lines that a namespace spans to detect it that namespace is short, thus not needing end comments to be added. This is needed to ensure clang-format is idempotent. Previously, a short namespace was detected by the original source code lines. This has the effect of requiring two runs for this example: ``` namespace { class A; } ``` after first run: ``` namespace { class A; } ``` after second run: ``` namespace { class A; } // namespace ``` Reviewers: djasper Reviewed By: djasper Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D30528 llvm-svn: 296736
1 parent fb0dc62 commit 9163fe2

File tree

3 files changed

+60
-24
lines changed

3 files changed

+60
-24
lines changed

‎clang/lib/Format/NamespaceEndCommentsFixer.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace clang {
2323
namespace format {
2424

2525
namespace {
26-
// The maximal number of lines that a short namespace spans.
26+
// The maximal number of unwrapped lines that a short namespace spans.
2727
// Short namespaces don't need an end comment.
2828
static const int kShortNamespaceMaxLines = 1;
2929

@@ -60,14 +60,6 @@ std::string computeEndCommentText(StringRef NamespaceName, bool AddNewline) {
6060
return text;
6161
}
6262

63-
bool isShort(const FormatToken *NamespaceTok, const FormatToken *RBraceTok,
64-
const SourceManager &SourceMgr) {
65-
int StartLine =
66-
SourceMgr.getSpellingLineNumber(NamespaceTok->Tok.getLocation());
67-
int EndLine = SourceMgr.getSpellingLineNumber(RBraceTok->Tok.getLocation());
68-
return EndLine - StartLine + 1 <= kShortNamespaceMaxLines;
69-
}
70-
7163
bool hasEndComment(const FormatToken *RBraceTok) {
7264
return RBraceTok->Next && RBraceTok->Next->is(tok::comment);
7365
}
@@ -151,7 +143,8 @@ tooling::Replacements NamespaceEndCommentsFixer::analyze(
151143
const std::string EndCommentText =
152144
computeEndCommentText(NamespaceName, AddNewline);
153145
if (!hasEndComment(RBraceTok)) {
154-
if (!isShort(NamespaceTok, RBraceTok, SourceMgr))
146+
bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
147+
if (!isShort)
155148
addEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes);
156149
continue;
157150
}

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ TEST_F(FormatTest, RemovesEmptyLines) {
185185
EXPECT_EQ("namespace N {\n"
186186
"\n"
187187
"int i;\n"
188-
"} // namespace N",
188+
"}",
189189
format("namespace N {\n"
190190
"\n"
191191
"int i;\n"
@@ -281,8 +281,7 @@ TEST_F(FormatTest, RemovesEmptyLines) {
281281
"}", LLVMWithNoNamespaceFix));
282282
EXPECT_EQ("namespace {\n"
283283
"int i;\n"
284-
"\n"
285-
"} // namespace",
284+
"}",
286285
format("namespace {\n"
287286
"int i;\n"
288287
"\n"
@@ -5460,7 +5459,7 @@ TEST_F(FormatTest, IncorrectCodeMissingSemicolon) {
54605459
EXPECT_EQ("namespace N {\n"
54615460
"void f() {}\n"
54625461
"void g()\n"
5463-
"}",
5462+
"} // namespace N",
54645463
format("namespace N { void f( ) { } void g( ) }"));
54655464
}
54665465

@@ -6140,8 +6139,8 @@ TEST_F(FormatTest, FormatStarDependingOnContext) {
61406139
" void f() {}\n"
61416140
" int *a;\n"
61426141
"};\n"
6143-
"}\n"
6144-
"}");
6142+
"} // namespace b\n"
6143+
"} // namespace a");
61456144
}
61466145

61476146
TEST_F(FormatTest, SpecialTokensAtEndOfLine) {
@@ -7934,7 +7933,7 @@ TEST_F(FormatTest, StroustrupBraceBreaking) {
79347933
"struct B {\n"
79357934
" int x;\n"
79367935
"};\n"
7937-
"}\n",
7936+
"} // namespace a\n",
79387937
StroustrupBraceStyle);
79397938

79407939
verifyFormat("void foo()\n"

‎clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,87 +47,123 @@ class NamespaceEndCommentsFixerTest : public ::testing::Test {
4747
TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
4848
EXPECT_EQ("namespace {\n"
4949
" int i;\n"
50+
" int j;\n"
5051
"}// namespace",
5152
fixNamespaceEndComments("namespace {\n"
5253
" int i;\n"
54+
" int j;\n"
5355
"}"));
5456
EXPECT_EQ("namespace {\n"
5557
" int i;\n"
58+
" int j;\n"
5659
"}// namespace\n",
5760
fixNamespaceEndComments("namespace {\n"
5861
" int i;\n"
62+
" int j;\n"
5963
"}\n"));
6064
EXPECT_EQ("namespace A {\n"
6165
" int i;\n"
66+
" int j;\n"
6267
"}// namespace A",
6368
fixNamespaceEndComments("namespace A {\n"
6469
" int i;\n"
70+
" int j;\n"
6571
"}"));
6672
EXPECT_EQ("inline namespace A {\n"
6773
" int i;\n"
74+
" int j;\n"
6875
"}// namespace A",
6976
fixNamespaceEndComments("inline namespace A {\n"
7077
" int i;\n"
78+
" int j;\n"
7179
"}"));
7280
EXPECT_EQ("namespace ::A {\n"
7381
" int i;\n"
82+
" int j;\n"
7483
"}// namespace ::A",
7584
fixNamespaceEndComments("namespace ::A {\n"
7685
" int i;\n"
86+
" int j;\n"
7787
"}"));
7888
EXPECT_EQ("namespace ::A::B {\n"
7989
" int i;\n"
90+
" int j;\n"
8091
"}// namespace ::A::B",
8192
fixNamespaceEndComments("namespace ::A::B {\n"
8293
" int i;\n"
94+
" int j;\n"
8395
"}"));
8496
EXPECT_EQ("namespace /**/::/**/A/**/::/**/B/**/ {\n"
8597
" int i;\n"
98+
" int j;\n"
8699
"}// namespace ::A::B",
87100
fixNamespaceEndComments("namespace /**/::/**/A/**/::/**/B/**/ {\n"
88101
" int i;\n"
102+
" int j;\n"
89103
"}"));
90104
EXPECT_EQ("namespace A {\n"
91105
"namespace B {\n"
92106
" int i;\n"
107+
"}\n"
108+
"}// namespace A",
109+
fixNamespaceEndComments("namespace A {\n"
110+
"namespace B {\n"
111+
" int i;\n"
112+
"}\n"
113+
"}"));
114+
EXPECT_EQ("namespace A {\n"
115+
"namespace B {\n"
116+
" int i;\n"
117+
" int j;\n"
93118
"}// namespace B\n"
94119
"}// namespace A",
95120
fixNamespaceEndComments("namespace A {\n"
96121
"namespace B {\n"
97122
" int i;\n"
123+
" int j;\n"
98124
"}\n"
99125
"}"));
100126
EXPECT_EQ("namespace A {\n"
101127
" int a;\n"
128+
" int b;\n"
102129
"}// namespace A\n"
103130
"namespace B {\n"
104131
" int b;\n"
132+
" int a;\n"
105133
"}// namespace B",
106134
fixNamespaceEndComments("namespace A {\n"
107135
" int a;\n"
136+
" int b;\n"
108137
"}\n"
109138
"namespace B {\n"
110139
" int b;\n"
140+
" int a;\n"
111141
"}"));
112142
EXPECT_EQ("namespace A {\n"
113143
" int a1;\n"
144+
" int a2;\n"
114145
"}// namespace A\n"
115146
"namespace A {\n"
116147
" int a2;\n"
148+
" int a1;\n"
117149
"}// namespace A",
118150
fixNamespaceEndComments("namespace A {\n"
119151
" int a1;\n"
152+
" int a2;\n"
120153
"}\n"
121154
"namespace A {\n"
122155
" int a2;\n"
156+
" int a1;\n"
123157
"}"));
124158
EXPECT_EQ("namespace A {\n"
125159
" int a;\n"
160+
" int b;\n"
126161
"}// namespace A\n"
127162
"// comment about b\n"
128163
"int b;",
129164
fixNamespaceEndComments("namespace A {\n"
130165
" int a;\n"
166+
" int b;\n"
131167
"}\n"
132168
"// comment about b\n"
133169
"int b;"));
@@ -136,7 +172,7 @@ TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
136172
"namespace B {\n"
137173
"namespace C {\n"
138174
"namespace D {\n"
139-
"}// namespace D\n"
175+
"}\n"
140176
"}// namespace C\n"
141177
"}// namespace B\n"
142178
"}// namespace A",
@@ -153,36 +189,44 @@ TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
153189
TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
154190
EXPECT_EQ("namespace A {\n"
155191
" int i;\n"
192+
" int j;\n"
156193
"}// namespace A\n"
157-
" int j;",
194+
" int k;",
158195
fixNamespaceEndComments("namespace A {\n"
159196
" int i;\n"
160-
"} int j;"));
197+
" int j;\n"
198+
"} int k;"));
161199
EXPECT_EQ("namespace {\n"
162200
" int i;\n"
201+
" int j;\n"
163202
"}// namespace\n"
164-
" int j;",
203+
" int k;",
165204
fixNamespaceEndComments("namespace {\n"
166205
" int i;\n"
167-
"} int j;"));
206+
" int j;\n"
207+
"} int k;"));
168208
EXPECT_EQ("namespace A {\n"
169209
" int i;\n"
210+
" int j;\n"
170211
"}// namespace A\n"
171212
" namespace B {\n"
172213
" int j;\n"
214+
" int k;\n"
173215
"}// namespace B",
174216
fixNamespaceEndComments("namespace A {\n"
175217
" int i;\n"
218+
" int j;\n"
176219
"} namespace B {\n"
177220
" int j;\n"
221+
" int k;\n"
178222
"}"));
179223
}
180224

181225
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
182226
EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
183227
EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
184-
EXPECT_EQ("namespace A { int i; }",
185-
fixNamespaceEndComments("namespace A { int i; }"));
228+
EXPECT_EQ("namespace A { a }",
229+
fixNamespaceEndComments("namespace A { a }"));
186230
}
187231

188232
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterUnaffectedRBrace) {

0 commit comments

Comments
 (0)