diff --git a/clang-tools-extra/clang-tidy/utils/IncludeSorter.h b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h --- a/clang-tools-extra/clang-tidy/utils/IncludeSorter.h +++ b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h @@ -23,7 +23,7 @@ class IncludeSorter { public: /// Supported include styles. - enum IncludeStyle { IS_LLVM = 0, IS_Google = 1 }; + enum IncludeStyle { IS_LLVM = 0, IS_Google = 1, IS_Google_ObjC }; /// The classifications of inclusions, in the order they should be sorted. enum IncludeKinds { @@ -31,7 +31,8 @@ IK_CSystemInclude = 1, ///< e.g. ``#include `` IK_CXXSystemInclude = 2, ///< e.g. ``#include `` IK_NonSystemInclude = 3, ///< e.g. ``#include "bar.h"`` - IK_InvalidInclude = 4 ///< total number of valid ``IncludeKind``s + IK_GeneratedInclude = 4, ///< e.g. ``#include "bar.proto.h"`` + IK_InvalidInclude = 5 ///< total number of valid ``IncludeKind``s }; /// ``IncludeSorter`` constructor; takes the FileID and name of the file to be diff --git a/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp b/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp --- a/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp +++ b/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp @@ -9,6 +9,7 @@ #include "IncludeSorter.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" +#include namespace clang { namespace tidy { @@ -36,6 +37,17 @@ return RemoveFirstSuffix( RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}), {"Test"}); } + if (Style == IncludeSorter::IS_Google_ObjC) { + StringRef Canonical = + RemoveFirstSuffix(RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", + ".hpp", ".mm", ".m"}), + {"_unittest", "_regtest", "_test", "Test"}); + + // Objective-C categories have a `+suffix` format, but should be grouped + // with the file they are a category of. + return Canonical.substr( + 0, Canonical.find_first_of('+', Canonical.find_last_of('/'))); + } return RemoveFirstSuffix( RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}), {"_unittest", "_regtest", "_test"}); @@ -65,7 +77,8 @@ || CanonicalInclude.endswith(CanonicalFile)) { return IncludeSorter::IK_MainTUInclude; } - if (Style == IncludeSorter::IS_Google) { + if ((Style == IncludeSorter::IS_Google) || + (Style == IncludeSorter::IS_Google_ObjC)) { std::pair Parts = CanonicalInclude.split("/public/"); std::string AltCanonicalInclude = Parts.first.str() + "/internal/" + Parts.second.str(); @@ -78,9 +91,32 @@ return IncludeSorter::IK_MainTUInclude; } } + if (Style == IncludeSorter::IS_Google_ObjC) { + if (IncludeFile.endswith(".generated.h") || + IncludeFile.endswith(".proto.h") || IncludeFile.endswith(".pbobjc.h")) { + return IncludeSorter::IK_GeneratedInclude; + } + } return IncludeSorter::IK_NonSystemInclude; } +int compareHeaders(StringRef LHS, StringRef RHS, + IncludeSorter::IncludeStyle Style) { + if (Style == IncludeSorter::IncludeStyle::IS_Google_ObjC) { + const std::pair &Mismatch = + std::mismatch(LHS.begin(), LHS.end(), RHS.begin()); + if ((Mismatch.first != LHS.end()) && (Mismatch.second != RHS.end())) { + if ((*Mismatch.first == '.') && (*Mismatch.second == '+')) { + return -1; + } + if ((*Mismatch.first == '+') && (*Mismatch.second == '.')) { + return 1; + } + } + } + return LHS.compare(RHS); +} + } // namespace IncludeSorter::IncludeSorter(const SourceManager *SourceMgr, @@ -112,9 +148,16 @@ Optional IncludeSorter::CreateIncludeInsertion(StringRef FileName, bool IsAngled) { - std::string IncludeStmt = - IsAngled ? llvm::Twine("#include <" + FileName + ">\n").str() - : llvm::Twine("#include \"" + FileName + "\"\n").str(); + std::string IncludeStmt; + if (Style == IncludeStyle::IS_Google_ObjC) { + IncludeStmt = IsAngled + ? llvm::Twine("#import <" + FileName + ">\n").str() + : llvm::Twine("#import \"" + FileName + "\"\n").str(); + } else { + 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. @@ -128,7 +171,7 @@ if (!IncludeBucket[IncludeKind].empty()) { for (const std::string &IncludeEntry : IncludeBucket[IncludeKind]) { - if (FileName < IncludeEntry) { + if (compareHeaders(FileName, IncludeEntry, Style) < 0) { const auto &Location = IncludeLocations[IncludeEntry][0]; return FixItHint::CreateInsertion(Location.getBegin(), IncludeStmt); } else if (FileName == IncludeEntry) { @@ -181,7 +224,8 @@ OptionEnumMapping::getEnumMapping() { static constexpr std::pair Mapping[] = {{utils::IncludeSorter::IS_LLVM, "llvm"}, - {utils::IncludeSorter::IS_Google, "google"}}; + {utils::IncludeSorter::IS_Google, "google"}, + {utils::IncludeSorter::IS_Google_ObjC, "google-objc"}}; return makeArrayRef(Mapping); } } // namespace tidy diff --git a/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp @@ -28,8 +28,10 @@ class IncludeInserterCheckBase : public ClangTidyCheck { public: - IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context) - : ClangTidyCheck(CheckName, Context) {} + IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context, + utils::IncludeSorter::IncludeStyle Style = + utils::IncludeSorter::IS_Google) + : ClangTidyCheck(CheckName, Context), Inserter(Style) {} void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override { @@ -50,7 +52,7 @@ virtual std::vector headersToInclude() const = 0; - utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google}; + utils::IncludeInserter Inserter; }; class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase { @@ -111,6 +113,42 @@ } }; +class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase { +public: + ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName, + ClangTidyContext *Context) + : IncludeInserterCheckBase(CheckName, Context, + utils::IncludeSorter::IS_Google_ObjC) {} + + std::vector headersToInclude() const override { + return {"a/header.h"}; + } +}; + +class ObjCCategoryHeaderInserterCheck : public IncludeInserterCheckBase { +public: + ObjCCategoryHeaderInserterCheck(StringRef CheckName, + ClangTidyContext *Context) + : IncludeInserterCheckBase(CheckName, Context, + utils::IncludeSorter::IS_Google_ObjC) {} + + std::vector headersToInclude() const override { + return {"clang_tidy/tests/insert_includes_test_header+foo.h"}; + } +}; + +class ObjCGeneratedHeaderInserterCheck : public IncludeInserterCheckBase { +public: + ObjCGeneratedHeaderInserterCheck(StringRef CheckName, + ClangTidyContext *Context) + : IncludeInserterCheckBase(CheckName, Context, + utils::IncludeSorter::IS_Google_ObjC) {} + + std::vector headersToInclude() const override { + return {"clang_tidy/tests/generated_file.proto.h"}; + } +}; + template std::string runCheckOnCode(StringRef Code, StringRef Filename) { std::vector Errors; @@ -120,12 +158,20 @@ {"clang_tidy/tests/" "insert_includes_test_header.h", "\n"}, + // ObjC category. + {"clang_tidy/tests/" + "insert_includes_test_header+foo.h", + "\n"}, // Non system headers {"a/header.h", "\n"}, {"path/to/a/header.h", "\n"}, {"path/to/z/header.h", "\n"}, {"path/to/header.h", "\n"}, {"path/to/header2.h", "\n"}, + // Generated headers + {"clang_tidy/tests/" + "generated_file.proto.h", + "\n"}, // Fake system headers. {"stdlib.h", "\n"}, {"unistd.h", "\n"}, @@ -160,9 +206,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_input2.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } TEST(IncludeInserterTest, InsertMultipleIncludesAndDeduplicate) { @@ -191,9 +237,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_input2.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } TEST(IncludeInserterTest, InsertBeforeFirstNonSystemInclude) { @@ -221,9 +267,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_input2.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } TEST(IncludeInserterTest, InsertBetweenNonSystemIncludes) { @@ -253,9 +299,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_input2.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } TEST(IncludeInserterTest, NonSystemIncludeAlreadyIncluded) { @@ -272,9 +318,9 @@ void foo() { int a = 0; })"; - EXPECT_EQ(PreCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_input2.cc")); + EXPECT_EQ(PreCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } TEST(IncludeInserterTest, InsertNonSystemIncludeAfterLastCXXSystemInclude) { @@ -299,9 +345,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertNonSystemIncludeAfterMainFileInclude) { @@ -320,9 +366,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterLastCXXSystemInclude) { @@ -350,9 +396,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeFirstCXXSystemInclude) { @@ -378,9 +424,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeBetweenCXXSystemIncludes) { @@ -408,9 +454,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterMainFileInclude) { @@ -433,9 +479,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterCSystemInclude) { @@ -462,9 +508,9 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeNonSystemInclude) { @@ -483,9 +529,10 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "devtools/cymbal/clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertCSystemIncludeBeforeCXXSystemInclude) { @@ -508,9 +555,10 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "devtools/cymbal/clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, InsertIncludeIfThereWasNoneBefore) { @@ -525,9 +573,10 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "devtools/cymbal/clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc")); } TEST(IncludeInserterTest, DontInsertDuplicateIncludeEvenIfMiscategorized) { @@ -630,9 +679,84 @@ int a = 0; })"; - EXPECT_EQ(PostCode, runCheckOnCode( - PreCode, "clang_tidy/tests/" - "insert_includes_test_header.cc")); + EXPECT_EQ(PostCode, + runCheckOnCode( + PreCode, "clang_tidy/tests/insert_includes_test_header.cc")); +} + +TEST(IncludeInserterTest, InsertHeaderObjectiveC) { + const char *PreCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" + +void foo() { + int a = 0; +})"; + const char *PostCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" + +#import "a/header.h" + +void foo() { + int a = 0; +})"; + + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm")); +} + +TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) { + const char *PreCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" + +void foo() { + int a = 0; +})"; + const char *PostCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" +#import "clang_tidy/tests/insert_includes_test_header+foo.h" + +void foo() { + int a = 0; +})"; + + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm")); +} + +TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) { + const char *PreCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" + +#include +#include + +#include "path/to/a/header.h" + +void foo() { + int a = 0; +})"; + const char *PostCode = R"( +#import "clang_tidy/tests/insert_includes_test_header.h" + +#include +#include + +#include "path/to/a/header.h" + +#import "clang_tidy/tests/generated_file.proto.h" + +void foo() { + int a = 0; +})"; + + EXPECT_EQ( + PostCode, + runCheckOnCode( + PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm")); } } // anonymous namespace