[clang-tools-extra] cc175c2 - Support ObjC in IncludeInserter
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 15 19:14:59 PDT 2020
Author: Alexander Kornienko
Date: 2020-10-16T04:12:32+02:00
New Revision: cc175c2cc8e638462bab74e0781e06f9b6eb5017
URL: https://github.com/llvm/llvm-project/commit/cc175c2cc8e638462bab74e0781e06f9b6eb5017
DIFF: https://github.com/llvm/llvm-project/commit/cc175c2cc8e638462bab74e0781e06f9b6eb5017.diff
LOG: Support ObjC in IncludeInserter
Update IncludeSorter/IncludeInserter to support objective-c google style (part 1):
1) Correctly consider .mm/.m extensions
2) Correctly categorize category headers.
3) Add support for generated files to go in a separate section of imports
Reviewed By: alexfh, gribozavr2
Patch by Joe Turner.
Differential Revision: https://reviews.llvm.org/D89276
Added:
Modified:
clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
clang-tools-extra/clang-tidy/utils/IncludeSorter.h
clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp b/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
index c9d018f076e7..f0e56a0af448 100644
--- 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 <algorithm>
namespace clang {
namespace tidy {
@@ -36,6 +37,17 @@ StringRef MakeCanonicalName(StringRef Str, IncludeSorter::IncludeStyle Style) {
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 @@ DetermineIncludeKind(StringRef CanonicalFile, StringRef IncludeFile,
|| CanonicalInclude.endswith(CanonicalFile)) {
return IncludeSorter::IK_MainTUInclude;
}
- if (Style == IncludeSorter::IS_Google) {
+ if ((Style == IncludeSorter::IS_Google) ||
+ (Style == IncludeSorter::IS_Google_ObjC)) {
std::pair<StringRef, StringRef> Parts = CanonicalInclude.split("/public/");
std::string AltCanonicalInclude =
Parts.first.str() + "/internal/" + Parts.second.str();
@@ -78,9 +91,32 @@ DetermineIncludeKind(StringRef CanonicalFile, StringRef IncludeFile,
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<const char *, const char *> &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 @@ void IncludeSorter::AddInclude(StringRef FileName, bool IsAngled,
Optional<FixItHint> 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 @@ Optional<FixItHint> IncludeSorter::CreateIncludeInsertion(StringRef FileName,
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 @@ llvm::ArrayRef<std::pair<utils::IncludeSorter::IncludeStyle, StringRef>>
OptionEnumMapping<utils::IncludeSorter::IncludeStyle>::getEnumMapping() {
static constexpr std::pair<utils::IncludeSorter::IncludeStyle, StringRef>
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/clang-tidy/utils/IncludeSorter.h b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h
index 1d8997364e5c..a8cf18ca8625 100644
--- a/clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -23,7 +23,7 @@ namespace utils {
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 @@ class IncludeSorter {
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 ``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/unittests/clang-tidy/IncludeInserterTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
index dc213ea637ba..f1c9ba743542 100644
--- a/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@ namespace {
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 @@ class IncludeInserterCheckBase : public ClangTidyCheck {
virtual std::vector<StringRef> headersToInclude() const = 0;
- utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+ utils::IncludeInserter Inserter;
};
class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,42 @@ class InvalidIncludeInserterCheck : public IncludeInserterCheckBase {
}
};
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+ ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+ : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+ std::vector<StringRef> 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<StringRef> 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<StringRef> headersToInclude() const override {
+ return {"clang_tidy/tests/generated_file.proto.h"};
+ }
+};
+
template <typename Check>
std::string runCheckOnCode(StringRef Code, StringRef Filename) {
std::vector<ClangTidyError> Errors;
@@ -120,12 +158,20 @@ std::string runCheckOnCode(StringRef Code, StringRef Filename) {
{"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 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_input2.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
}
TEST(IncludeInserterTest, InsertMultipleIncludesAndDeduplicate) {
@@ -191,9 +237,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<MultipleHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_input2.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<MultipleHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
}
TEST(IncludeInserterTest, InsertBeforeFirstNonSystemInclude) {
@@ -221,9 +267,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_input2.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
}
TEST(IncludeInserterTest, InsertBetweenNonSystemIncludes) {
@@ -253,9 +299,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_input2.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
}
TEST(IncludeInserterTest, NonSystemIncludeAlreadyIncluded) {
@@ -272,9 +318,9 @@ TEST(IncludeInserterTest, NonSystemIncludeAlreadyIncluded) {
void foo() {
int a = 0;
})";
- EXPECT_EQ(PreCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_input2.cc"));
+ EXPECT_EQ(PreCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
}
TEST(IncludeInserterTest, InsertNonSystemIncludeAfterLastCXXSystemInclude) {
@@ -299,9 +345,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertNonSystemIncludeAfterMainFileInclude) {
@@ -320,9 +366,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<NonSystemHeaderInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<NonSystemHeaderInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterLastCXXSystemInclude) {
@@ -350,9 +396,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeFirstCXXSystemInclude) {
@@ -378,9 +424,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeBetweenCXXSystemIncludes) {
@@ -408,9 +454,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterMainFileInclude) {
@@ -433,9 +479,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeAfterCSystemInclude) {
@@ -462,9 +508,9 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeNonSystemInclude) {
@@ -483,9 +529,10 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "devtools/cymbal/clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(
+ PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertCSystemIncludeBeforeCXXSystemInclude) {
@@ -508,9 +555,10 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CSystemIncludeInserterCheck>(
- PreCode, "devtools/cymbal/clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(
+ PostCode,
+ runCheckOnCode<CSystemIncludeInserterCheck>(
+ PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, InsertIncludeIfThereWasNoneBefore) {
@@ -525,9 +573,10 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
- PreCode, "devtools/cymbal/clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(
+ PostCode,
+ runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "repo/clang_tidy/tests/insert_includes_test_header.cc"));
}
TEST(IncludeInserterTest, DontInsertDuplicateIncludeEvenIfMiscategorized) {
@@ -630,9 +679,84 @@ void foo() {
int a = 0;
})";
- EXPECT_EQ(PostCode, runCheckOnCode<InvalidIncludeInserterCheck>(
- PreCode, "clang_tidy/tests/"
- "insert_includes_test_header.cc"));
+ EXPECT_EQ(PostCode,
+ runCheckOnCode<InvalidIncludeInserterCheck>(
+ 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<ObjCEarlyInAlphabetHeaderInserterCheck>(
+ 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<ObjCCategoryHeaderInserterCheck>(
+ 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 <list>
+#include <map>
+
+#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 <list>
+#include <map>
+
+#include "path/to/a/header.h"
+
+#import "clang_tidy/tests/generated_file.proto.h"
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(
+ PostCode,
+ runCheckOnCode<ObjCGeneratedHeaderInserterCheck>(
+ PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
}
} // anonymous namespace
More information about the cfe-commits
mailing list