Skip to content

Commit 303baf5

Browse files
author
Eric Liu
committed
[clang-format] skip empty lines and comments in the top of the code when inserting new headers.
Summary: [clang-format] skip empty lines and comments in the top of the code when inserting new headers. Pair-programmed with @hokein Reviewers: djasper Subscribers: ioeric, cfe-commits, hokein, klimek Differential Revision: http://reviews.llvm.org/D20898 llvm-svn: 271664
1 parent 0f7e949 commit 303baf5

File tree

2 files changed

+101
-15
lines changed

2 files changed

+101
-15
lines changed

‎clang/lib/Format/Format.cpp

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,20 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
14691469
StringRef FileName = Replaces.begin()->getFilePath();
14701470
IncludeCategoryManager Categories(Style, FileName);
14711471

1472+
std::unique_ptr<Environment> Env =
1473+
Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
1474+
const SourceManager &SourceMgr = Env->getSourceManager();
1475+
Lexer Lex(Env->getFileID(), SourceMgr.getBuffer(Env->getFileID()), SourceMgr,
1476+
getFormattingLangOpts(Style));
1477+
Token Tok;
1478+
// All new headers should be inserted after this offset.
1479+
int MinInsertOffset = Code.size();
1480+
while (!Lex.LexFromRawLexer(Tok)) {
1481+
if (Tok.isNot(tok::comment)) {
1482+
MinInsertOffset = SourceMgr.getFileOffset(Tok.getLocation());
1483+
break;
1484+
}
1485+
}
14721486
// Record the offset of the end of the last include in each category.
14731487
std::map<int, int> CategoryEndOffsets;
14741488
// All possible priorities.
@@ -1477,10 +1491,11 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
14771491
for (const auto &Category : Style.IncludeCategories)
14781492
Priorities.insert(Category.Priority);
14791493
int FirstIncludeOffset = -1;
1480-
int Offset = 0;
1481-
int AfterHeaderGuard = 0;
1494+
bool HeaderGuardFound = false;
1495+
StringRef TrimmedCode = Code.drop_front(MinInsertOffset);
14821496
SmallVector<StringRef, 32> Lines;
1483-
Code.split(Lines, '\n');
1497+
TrimmedCode.split(Lines, '\n');
1498+
int Offset = MinInsertOffset;
14841499
for (auto Line : Lines) {
14851500
if (IncludeRegex.match(Line, &Matches)) {
14861501
StringRef IncludeName = Matches[2];
@@ -1492,22 +1507,22 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
14921507
}
14931508
Offset += Line.size() + 1;
14941509
// FIXME: make header guard matching stricter, e.g. consider #ifndef.
1495-
if (AfterHeaderGuard == 0 && DefineRegex.match(Line))
1496-
AfterHeaderGuard = Offset;
1510+
if (!HeaderGuardFound && DefineRegex.match(Line)) {
1511+
HeaderGuardFound = true;
1512+
MinInsertOffset = Offset;
1513+
}
14971514
}
14981515

14991516
// Populate CategoryEndOfssets:
15001517
// - Ensure that CategoryEndOffset[Highest] is always populated.
15011518
// - If CategoryEndOffset[Priority] isn't set, use the next higher value that
15021519
// is set, up to CategoryEndOffset[Highest].
1503-
// FIXME: skip comment section in the beginning of the code if there is no
1504-
// existing #include and #define.
15051520
auto Highest = Priorities.begin();
15061521
if (CategoryEndOffsets.find(*Highest) == CategoryEndOffsets.end()) {
15071522
if (FirstIncludeOffset >= 0)
15081523
CategoryEndOffsets[*Highest] = FirstIncludeOffset;
15091524
else
1510-
CategoryEndOffsets[*Highest] = AfterHeaderGuard;
1525+
CategoryEndOffsets[*Highest] = MinInsertOffset;
15111526
}
15121527
// By this point, CategoryEndOffset[Highest] is always set appropriately:
15131528
// - to an appropriate location before/after existing #includes, or

‎clang/unittests/Format/CleanupTest.cpp

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -465,13 +465,13 @@ TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesGoogleStyle) {
465465

466466
TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortLLVM) {
467467
std::string Code = "\nint x;";
468-
std::string Expected = "#include \"fix.h\"\n"
468+
std::string Expected = "\n#include \"fix.h\"\n"
469469
"#include \"a.h\"\n"
470470
"#include \"b.h\"\n"
471471
"#include \"c.h\"\n"
472472
"#include <list>\n"
473473
"#include <vector>\n"
474-
"\nint x;";
474+
"int x;";
475475
tooling::Replacements Replaces = {createInsertion("#include \"a.h\""),
476476
createInsertion("#include \"c.h\""),
477477
createInsertion("#include \"b.h\""),
@@ -483,13 +483,13 @@ TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortLLVM) {
483483

484484
TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
485485
std::string Code = "\nint x;";
486-
std::string Expected = "#include \"fix.h\"\n"
486+
std::string Expected = "\n#include \"fix.h\"\n"
487487
"#include <list>\n"
488488
"#include <vector>\n"
489489
"#include \"a.h\"\n"
490490
"#include \"b.h\"\n"
491491
"#include \"c.h\"\n"
492-
"\nint x;";
492+
"int x;";
493493
tooling::Replacements Replaces = {createInsertion("#include \"a.h\""),
494494
createInsertion("#include \"c.h\""),
495495
createInsertion("#include \"b.h\""),
@@ -502,21 +502,22 @@ TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
502502

503503
TEST_F(CleanUpReplacementsTest, FormatCorrectLineWhenHeadersAreInserted) {
504504
std::string Code = "\n"
505+
"int x;\n"
505506
"int a;\n"
506507
"int a;\n"
507508
"int a;";
508509

509-
std::string Expected = "#include \"x.h\"\n"
510+
std::string Expected = "\n#include \"x.h\"\n"
510511
"#include \"y.h\"\n"
511512
"#include \"clang/x/x.h\"\n"
512513
"#include <list>\n"
513514
"#include <vector>\n"
514-
"\n"
515+
"int x;\n"
515516
"int a;\n"
516517
"int b;\n"
517518
"int a;";
518519
tooling::Replacements Replaces = {
519-
createReplacement(getOffset(Code, 3, 8), 1, "b"),
520+
createReplacement(getOffset(Code, 4, 8), 1, "b"),
520521
createInsertion("#include <vector>"),
521522
createInsertion("#include <list>"),
522523
createInsertion("#include \"clang/x/x.h\""),
@@ -537,6 +538,76 @@ TEST_F(CleanUpReplacementsTest, NotConfusedByDefine) {
537538
EXPECT_EQ(Expected, formatAndApply(Code, Replaces));
538539
}
539540

541+
TEST_F(CleanUpReplacementsTest, SkippedTopComment) {
542+
std::string Code = "// comment\n"
543+
"\n"
544+
" // comment\n";
545+
std::string Expected = "// comment\n"
546+
"\n"
547+
" // comment\n"
548+
"#include <vector>\n";
549+
tooling::Replacements Replaces = {createInsertion("#include <vector>")};
550+
EXPECT_EQ(Expected, apply(Code, Replaces));
551+
}
552+
553+
TEST_F(CleanUpReplacementsTest, SkippedMixedComments) {
554+
std::string Code = "// comment\n"
555+
"// comment \\\n"
556+
" comment continued\n"
557+
"/*\n"
558+
"* comment\n"
559+
"*/\n";
560+
std::string Expected = "// comment\n"
561+
"// comment \\\n"
562+
" comment continued\n"
563+
"/*\n"
564+
"* comment\n"
565+
"*/\n"
566+
"#include <vector>\n";
567+
tooling::Replacements Replaces = {createInsertion("#include <vector>")};
568+
EXPECT_EQ(Expected, apply(Code, Replaces));
569+
}
570+
571+
TEST_F(CleanUpReplacementsTest, MultipleBlockCommentsInOneLine) {
572+
std::string Code = "/*\n"
573+
"* comment\n"
574+
"*/ /* comment\n"
575+
"*/\n"
576+
"\n\n"
577+
"/* c1 */ /*c2 */\n";
578+
std::string Expected = "/*\n"
579+
"* comment\n"
580+
"*/ /* comment\n"
581+
"*/\n"
582+
"\n\n"
583+
"/* c1 */ /*c2 */\n"
584+
"#include <vector>\n";
585+
tooling::Replacements Replaces = {createInsertion("#include <vector>")};
586+
EXPECT_EQ(Expected, apply(Code, Replaces));
587+
}
588+
589+
TEST_F(CleanUpReplacementsTest, CodeAfterComments) {
590+
std::string Code = "/*\n"
591+
"* comment\n"
592+
"*/ /* comment\n"
593+
"*/\n"
594+
"\n\n"
595+
"/* c1 */ /*c2 */\n"
596+
"\n"
597+
"int x;\n";
598+
std::string Expected = "/*\n"
599+
"* comment\n"
600+
"*/ /* comment\n"
601+
"*/\n"
602+
"\n\n"
603+
"/* c1 */ /*c2 */\n"
604+
"\n"
605+
"#include <vector>\n"
606+
"int x;\n";
607+
tooling::Replacements Replaces = {createInsertion("#include <vector>")};
608+
EXPECT_EQ(Expected, apply(Code, Replaces));
609+
}
610+
540611
} // end namespace
541612
} // end namespace format
542613
} // end namespace clang

0 commit comments

Comments
 (0)