angelgarcia updated this revision to Diff 32146.
angelgarcia added a comment.
Add comments.
http://reviews.llvm.org/D12017
Files:
clang-tidy/IncludeInserter.cpp
clang-tidy/IncludeSorter.cpp
clang-tidy/IncludeSorter.h
unittests/clang-tidy/IncludeInserterTest.cpp
Index: unittests/clang-tidy/IncludeInserterTest.cpp
===================================================================
--- unittests/clang-tidy/IncludeInserterTest.cpp
+++ unittests/clang-tidy/IncludeInserterTest.cpp
@@ -59,6 +59,13 @@
bool IsAngledInclude() const override { return false; }
};
+class CSystemIncludeInserterCheck : public IncludeInserterCheckBase {
+public:
+ using IncludeInserterCheckBase::IncludeInserterCheckBase;
+ StringRef HeaderToInclude() const override { return "stdlib.h"; }
+ bool IsAngledInclude() const override { return true; }
+};
+
class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
public:
using IncludeInserterCheckBase::IncludeInserterCheckBase;
@@ -402,6 +409,72 @@
1));
}
+TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeNonSystemInclude) {
+ const char *PreCode = R"(
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
+
+TEST(IncludeInserterTest, InsertCSystemIncludeBeforeCXXSystemInclude) {
+ const char *PreCode = R"(
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(
+#include <stdlib.h>
+
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
+
+TEST(IncludeInserterTest, InsertIncludeIfThereWasNoneBefore) {
+ const char *PreCode = R"(
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(#include <set>
+
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
+
} // namespace
} // namespace tidy
} // namespace clang
Index: clang-tidy/IncludeSorter.h
===================================================================
--- clang-tidy/IncludeSorter.h
+++ clang-tidy/IncludeSorter.h
@@ -21,27 +21,23 @@
// the necessary commands to sort the inclusions according to the precedence
// enocded in IncludeKinds.
class IncludeSorter {
- public:
+public:
// Supported include styles.
- enum IncludeStyle {
- IS_LLVM = 0,
- IS_Google = 1
- };
+ enum IncludeStyle { IS_LLVM = 0, IS_Google = 1 };
// The classifications of inclusions, in the order they should be sorted.
enum IncludeKinds {
- IK_MainTUInclude = 0, // e.g. #include "foo.h" when editing foo.cc
- IK_CSystemInclude = 1, // e.g. #include <stdio.h>
- IK_CXXSystemInclude = 2, // e.g. #include <vector>
- IK_NonSystemInclude = 3, // e.g. #include "bar.h"
- IK_InvalidInclude = 4 // total number of valid IncludeKinds
+ IK_MainTUInclude = 0, // e.g. #include "foo.h" when editing foo.cc
+ IK_CSystemInclude = 1, // e.g. #include <stdio.h>
+ IK_CXXSystemInclude = 2, // e.g. #include <vector>
+ IK_NonSystemInclude = 3, // e.g. #include "bar.h"
+ IK_InvalidInclude = 4 // total number of valid IncludeKinds
};
// IncludeSorter constructor; takes the FileID and name of the file to be
// processed by the sorter.
- IncludeSorter(const SourceManager* SourceMgr,
- const LangOptions* LangOpts, const FileID FileID,
- StringRef FileName, IncludeStyle Style);
+ IncludeSorter(const SourceManager *SourceMgr, const LangOptions *LangOpts,
+ const FileID FileID, StringRef FileName, IncludeStyle Style);
// Returns the SourceManager-specific file ID for the file being handled by
// the sorter.
@@ -58,18 +54,17 @@
// Creates a quoted inclusion directive in the right sort order. Returns None
// on error or if header inclusion directive for header already exists.
- Optional<FixItHint> CreateIncludeInsertion(StringRef FileName,
- bool IsAngled);
+ Optional<FixItHint> CreateIncludeInsertion(StringRef FileName, bool IsAngled);
- private:
+private:
typedef SmallVector<SourceRange, 1> SourceRangeVector;
// Creates a fix-it for the given replacements.
// Takes the the source location that will be replaced, and the new text.
- FixItHint CreateFixIt(SourceRange EditRange, const std::string& NewText);
+ FixItHint CreateFixIt(SourceRange EditRange, const std::string &NewText);
- const SourceManager* SourceMgr;
- const LangOptions* LangOpts;
+ const SourceManager *SourceMgr;
+ const LangOptions *LangOpts;
const IncludeStyle Style;
FileID CurrentFileID;
// The file name stripped of common suffixes.
@@ -82,6 +77,6 @@
SmallVector<std::string, 1> IncludeBucket[IK_InvalidInclude];
};
-} // namespace tidy
-} // namespace clang
+} // namespace tidy
+} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_INCLUDESORTER_H
Index: clang-tidy/IncludeSorter.cpp
===================================================================
--- clang-tidy/IncludeSorter.cpp
+++ clang-tidy/IncludeSorter.cpp
@@ -111,14 +111,20 @@
Optional<FixItHint> IncludeSorter::CreateIncludeInsertion(StringRef FileName,
bool IsAngled) {
- if (SourceLocations.empty()) {
- return None;
- }
std::string IncludeStmt =
IsAngled ? llvm::Twine("#include <" + FileName + ">\n").str()
: llvm::Twine("#include \"" + FileName + "\"\n").str();
+ if (SourceLocations.empty()) {
+ // If there are no includes in this file, add it in the first line.
+ // FIXME: insert after the file comment or the header guard, if present.
+ IncludeStmt.append("\n");
+ return FixItHint::CreateInsertion(
+ SourceMgr->getLocForStartOfFile(CurrentFileID), IncludeStmt);
+ }
+
auto IncludeKind =
DetermineIncludeKind(CanonicalFile, FileName, IsAngled, Style);
+
if (!IncludeBucket[IncludeKind].empty()) {
for (const std::string &IncludeEntry : IncludeBucket[IncludeKind]) {
if (FileName < IncludeEntry) {
@@ -135,21 +141,37 @@
return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
IncludeStmt);
}
+ // Find the non-empty include bucket to be sorted directly above
+ // 'IncludeKind'. If such a bucket exists, we'll want to sort the include
+ // after that bucket. If no such bucket exists, find the first non-empty
+ // include bucket in the file. In that case, we'll want to sort the include
+ // before that bucket.
IncludeKinds NonEmptyKind = IK_InvalidInclude;
- for (int i = IncludeKind - 1; i >= 0; --i) {
+ for (int i = IK_InvalidInclude - 1; i >= 0; --i) {
if (!IncludeBucket[i].empty()) {
NonEmptyKind = static_cast<IncludeKinds>(i);
- break;
+ if (NonEmptyKind < IncludeKind)
+ break;
}
}
if (NonEmptyKind == IK_InvalidInclude) {
return llvm::None;
}
- const std::string &LastInclude = IncludeBucket[NonEmptyKind].back();
- SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+
+ if (NonEmptyKind < IncludeKind) {
+ // Create a block after.
+ const std::string &LastInclude = IncludeBucket[NonEmptyKind].back();
+ SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+ IncludeStmt = '\n' + IncludeStmt;
+ return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
+ IncludeStmt);
+ }
+ // Create a block before.
+ const std::string &FirstInclude = IncludeBucket[NonEmptyKind][0];
+ SourceRange FirstIncludeLocation = IncludeLocations[FirstInclude].back();
IncludeStmt.append("\n");
- return FixItHint::CreateInsertion(
- LastIncludeLocation.getEnd().getLocWithOffset(1), IncludeStmt);
+ return FixItHint::CreateInsertion(FirstIncludeLocation.getBegin(),
+ IncludeStmt);
}
std::vector<FixItHint> IncludeSorter::GetEdits() {
Index: clang-tidy/IncludeInserter.cpp
===================================================================
--- clang-tidy/IncludeInserter.cpp
+++ clang-tidy/IncludeInserter.cpp
@@ -26,7 +26,7 @@
StringRef /*SearchPath*/, StringRef /*RelativePath*/,
const Module * /*ImportedModule*/) override {
Inserter->AddInclude(FileNameRef, IsAngled, HashLocation,
- FileNameRange.getEnd());
+ FileNameRange.getEnd());
}
private:
@@ -54,7 +54,14 @@
return llvm::None;
}
if (IncludeSorterByFile.find(FileID) == IncludeSorterByFile.end()) {
- return llvm::None;
+ // This may happen if there have been no preprocessor directives in this
+ // file.
+ IncludeSorterByFile.insert(std::make_pair(
+ FileID,
+ llvm::make_unique<IncludeSorter>(
+ &SourceMgr, &LangOpts, FileID,
+ SourceMgr.getFilename(SourceMgr.getLocForStartOfFile(FileID)),
+ Style)));
}
return IncludeSorterByFile[FileID]->CreateIncludeInsertion(Header, IsAngled);
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
