This is an archive of the discontinued LLVM Phabricator instance.

Differential D30688

[clang-format] Support namespaces ending in semicolon
ClosedPublic

Authored by krasimir on Mar 7 2017, 2:26 AM.

Details

Reviewers
djasper
Commits
rGeb62118e6d8f: [clang-format] Support namespaces ending in semicolon
rC297140: [clang-format] Support namespaces ending in semicolon
rL297140: [clang-format] Support namespaces ending in semicolon
Summary

This patch adds support for namespaces ending in semicolon to the namespace comment fixer.
source:

namespace A {
  int i;
  int j;
};

clang-format before:

namespace A {
  int i;
  int j;
} // namespace A;

clang-format after:

namespace A {
  int i;
  int j;
}; // namespace A

Diff Detail

Event Timeline

krasimir created this revision.Mar 7 2017, 2:26 AM
Harbormaster completed remote builds in B4574: Diff 90826.Mar 7 2017, 2:26 AM
Herald added a subscriber: klimek. · View Herald TranscriptMar 7 2017, 2:26 AM
krasimir edited the summary of this revision. (Show Details)Mar 7 2017, 2:28 AM
krasimir added a reviewer: djasper.
krasimir added a subscriber: cfe-commits.
djasper accepted this revision.Mar 7 2017, 6:12 AM

Minor nit, otherwise looks good.

lib/Format/NamespaceEndCommentsFixer.cpp
152

No braces around single-statement ifs.

This revision is now accepted and ready to land.Mar 7 2017, 6:12 AM
krasimir updated this revision to Diff 90839.Mar 7 2017, 6:18 AM
  • Remove braces
Closed by commit rL297140: [clang-format] Support namespaces ending in semicolon (authored by krasimir). · Explain WhyMar 7 2017, 6:19 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
Format/
30 lines
unittests/
Format/
85 lines

Diff 90826

lib/Format/NamespaceEndCommentsFixer.cpp

Show First 20 Lines • Show All 133 Lines • ▼ Show 20 Lines for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
if (NamespaceTok->is(tok::kw_inline)) if (NamespaceTok->is(tok::kw_inline))
NamespaceTok = NamespaceTok->getNextNonComment(); NamespaceTok = NamespaceTok->getNextNonComment();
if (NamespaceTok->isNot(tok::kw_namespace)) if (NamespaceTok->isNot(tok::kw_namespace))
continue; continue;
FormatToken *RBraceTok = EndLine->First; FormatToken *RBraceTok = EndLine->First;
if (RBraceTok->Finalized) if (RBraceTok->Finalized)
continue; continue;
RBraceTok->Finalized = true; RBraceTok->Finalized = true;
const FormatToken *EndCommentPrevTok = RBraceTok;
// Namespaces often end with '};'. In that case, attach namespace end
// comments to the semicolon tokens.
if (RBraceTok->Next && RBraceTok->Next->is(tok::semi)) {
EndCommentPrevTok = RBraceTok->Next;
}
// The next token in the token stream after the place where the end comment
// token must be. This is either the next token on the current line or the
// first token on the next line.
const FormatToken *EndCommentNextTok = EndCommentPrevTok->Next;
if (EndCommentNextTok && EndCommentNextTok->is(tok::comment)) {
djasperUnsubmitted
Not Done
ReplyInline Actions

No braces around single-statement ifs.

djasper: No braces around single-statement ifs.
EndCommentNextTok = EndCommentNextTok->Next;
}
if (!EndCommentNextTok && I + 1 < E) {
EndCommentNextTok = AnnotatedLines[I + 1]->First;
}
bool AddNewline = EndCommentNextTok &&
EndCommentNextTok->NewlinesBefore == 0 &&
EndCommentNextTok->isNot(tok::eof);
const std::string NamespaceName = computeName(NamespaceTok); const std::string NamespaceName = computeName(NamespaceTok);
bool AddNewline = (I + 1 < E) &&
AnnotatedLines[I + 1]->First->NewlinesBefore == 0 &&
AnnotatedLines[I + 1]->First->isNot(tok::eof);
const std::string EndCommentText = const std::string EndCommentText =
computeEndCommentText(NamespaceName, AddNewline); computeEndCommentText(NamespaceName, AddNewline);
if (!hasEndComment(RBraceTok)) { if (!hasEndComment(EndCommentPrevTok)) {
bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1; bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
if (!isShort) if (!isShort)
addEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes); addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
continue; continue;
} }
if (!validEndComment(RBraceTok, NamespaceName)) if (!validEndComment(EndCommentPrevTok, NamespaceName))
updateEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes); updateEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
} }
return Fixes; return Fixes;
} }
} // namespace format } // namespace format
} // namespace clang } // namespace clang

unittests/Format/NamespaceEndCommentsFixerTest.cpp

Show First 20 Lines • Show All 178 Lines • ▼ Show 20 Lines EXPECT_EQ("namespace A {\n"
fixNamespaceEndComments("namespace A {\n" fixNamespaceEndComments("namespace A {\n"
"namespace B {\n" "namespace B {\n"
"namespace C {\n" "namespace C {\n"
"namespace D {\n" "namespace D {\n"
"}\n" "}\n"
"}\n" "}\n"
"}\n" "}\n"
"}")); "}"));
// Adds an end comment after a semicolon.
EXPECT_EQ("namespace {\n"
" int i;\n"
" int j;\n"
"};// namespace",
fixNamespaceEndComments("namespace {\n"
" int i;\n"
" int j;\n"
"};"));
EXPECT_EQ("namespace A {\n"
" int i;\n"
" int j;\n"
"};// namespace A",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
" int j;\n"
"};"));
EXPECT_EQ("namespace A {\n"
" int i;\n"
" int j;\n"
"};// namespace A\n"
"// unrelated",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
" int j;\n"
"};\n"
"// unrelated"));
} }
TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) { TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
EXPECT_EQ("namespace A {\n" EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
" int j;\n" " int j;\n"
"}// namespace A\n" "}// namespace A\n"
" int k;", " int k;",
Show All 20 Lines EXPECT_EQ("namespace A {\n"
"}// namespace B", "}// namespace B",
fixNamespaceEndComments("namespace A {\n" fixNamespaceEndComments("namespace A {\n"
" int i;\n" " int i;\n"
" int j;\n" " int j;\n"
"} namespace B {\n" "} namespace B {\n"
" int j;\n" " int j;\n"
" int k;\n" " int k;\n"
"}")); "}"));
EXPECT_EQ("namespace {\n"
" int i;\n"
" int j;\n"
"};// namespace\n"
"int k;",
fixNamespaceEndComments("namespace {\n"
" int i;\n"
" int j;\n"
"};int k;"));
EXPECT_EQ("namespace {\n"
" int i;\n"
" int j;\n"
"};// namespace\n"
";",
fixNamespaceEndComments("namespace {\n"
" int i;\n"
" int j;\n"
"};;"));
} }
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) { TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}")); EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}")); EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
EXPECT_EQ("namespace A { a }", EXPECT_EQ("namespace A { a }",
fixNamespaceEndComments("namespace A { a }")); fixNamespaceEndComments("namespace A { a }"));
EXPECT_EQ("namespace A { a };",
fixNamespaceEndComments("namespace A { a };"));
} }
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterUnaffectedRBrace) { TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterUnaffectedRBrace) {
EXPECT_EQ("namespace A {\n" EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"}", "}",
fixNamespaceEndComments("namespace A {\n" fixNamespaceEndComments("namespace A {\n"
" int i;\n" " int i;\n"
"}", "}",
// The range (16, 3) spans the 'int' above. // The range (16, 3) spans the 'int' above.
/*Ranges=*/{1, tooling::Range(16, 3)})); /*Ranges=*/{1, tooling::Range(16, 3)}));
EXPECT_EQ("namespace A {\n"
" int i;\n"
"};",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
"};",
// The range (16, 3) spans the 'int' above.
/*Ranges=*/{1, tooling::Range(16, 3)}));
} }
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterRBraceInPPDirective) { TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterRBraceInPPDirective) {
EXPECT_EQ("#define SAD \\\n" EXPECT_EQ("#define SAD \\\n"
"namespace A { \\\n" "namespace A { \\\n"
" int i; \\\n" " int i; \\\n"
"}", "}",
fixNamespaceEndComments("#define SAD \\\n" fixNamespaceEndComments("#define SAD \\\n"
Show All 22 Lines EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"} // namespace A")); "} // namespace A"));
EXPECT_EQ("namespace A::B {\n" EXPECT_EQ("namespace A::B {\n"
" int i;\n" " int i;\n"
"} // end namespace A::B", "} // end namespace A::B",
fixNamespaceEndComments("namespace A::B {\n" fixNamespaceEndComments("namespace A::B {\n"
" int i;\n" " int i;\n"
"} // end namespace A::B")); "} // end namespace A::B"));
EXPECT_EQ("namespace A {\n"
" int i;\n"
"}; // end namespace A",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
"}; // end namespace A"));
EXPECT_EQ("namespace {\n"
" int i;\n"
"}; /* unnamed namespace */",
fixNamespaceEndComments("namespace {\n"
" int i;\n"
"}; /* unnamed namespace */"));
} }
TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) { TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
EXPECT_EQ("namespace {\n" EXPECT_EQ("namespace {\n"
" int i;\n" " int i;\n"
"} // namespace", "} // namespace",
fixNamespaceEndComments("namespace {\n" fixNamespaceEndComments("namespace {\n"
" int i;\n" " int i;\n"
Show All 17 Lines EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"} //")); "} //"));
EXPECT_EQ("namespace A {\n" EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"} // namespace A", "} // namespace A",
fixNamespaceEndComments("namespace A {\n" fixNamespaceEndComments("namespace A {\n"
" int i;\n" " int i;\n"
"} // banamespace A")); "} // banamespace A"));
EXPECT_EQ("namespace A {\n"
" int i;\n"
"}; // namespace A",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
"}; // banamespace A"));
// Updates invalid line comments even for short namespaces. // Updates invalid line comments even for short namespaces.
EXPECT_EQ("namespace A {} // namespace A", EXPECT_EQ("namespace A {} // namespace A",
fixNamespaceEndComments("namespace A {} // namespace")); fixNamespaceEndComments("namespace A {} // namespace"));
EXPECT_EQ("namespace A {}; // namespace A",
fixNamespaceEndComments("namespace A {}; // namespace"));
} }
TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndBlockComment) { TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndBlockComment) {
EXPECT_EQ("namespace {\n" EXPECT_EQ("namespace {\n"
" int i;\n" " int i;\n"
"} // namespace", "} // namespace",
fixNamespaceEndComments("namespace {\n" fixNamespaceEndComments("namespace {\n"
" int i;\n" " int i;\n"
Show All 17 Lines EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"} /* end unnamed namespace */")); "} /* end unnamed namespace */"));
EXPECT_EQ("namespace A {\n" EXPECT_EQ("namespace A {\n"
" int i;\n" " int i;\n"
"} // namespace A", "} // namespace A",
fixNamespaceEndComments("namespace A {\n" fixNamespaceEndComments("namespace A {\n"
" int i;\n" " int i;\n"
"} /* banamespace A */")); "} /* banamespace A */"));
EXPECT_EQ("namespace A {\n"
" int i;\n"
"}; // namespace A",
fixNamespaceEndComments("namespace A {\n"
" int i;\n"
"}; /* banamespace A */"));
EXPECT_EQ("namespace A {} // namespace A", EXPECT_EQ("namespace A {} // namespace A",
fixNamespaceEndComments("namespace A {} /**/")); fixNamespaceEndComments("namespace A {} /**/"));
EXPECT_EQ("namespace A {}; // namespace A",
fixNamespaceEndComments("namespace A {}; /**/"));
} }
TEST_F(NamespaceEndCommentsFixerTest, TEST_F(NamespaceEndCommentsFixerTest,
DoesNotAddEndCommentForNamespacesControlledByMacros) { DoesNotAddEndCommentForNamespacesControlledByMacros) {
EXPECT_EQ("#ifdef 1\n" EXPECT_EQ("#ifdef 1\n"
"namespace A {\n" "namespace A {\n"
"#elseif\n" "#elseif\n"
"namespace B {\n" "namespace B {\n"
▲ Show 20 Lines • Show All 146 Lines • Show Last 20 Lines