Hi alexfh, djasper,
There are still a couple of rough edges in here but it is working fine
on LLVM and generates the same results as sort_includes.py if there are
no blank lines involved.
http://reviews.llvm.org/D4741
Files:
clang-tidy/llvm/IncludeOrderCheck.cpp
clang-tidy/llvm/IncludeOrderCheck.h
test/clang-tidy/llvm-include-order.cpp
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===================================================================
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "IncludeOrderCheck.h"
+#include "clang/AST/ASTContext.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
@@ -18,26 +19,172 @@
namespace {
class IncludeOrderPPCallbacks : public PPCallbacks {
public:
- explicit IncludeOrderPPCallbacks(IncludeOrderCheck &Check) : Check(Check) {}
+ explicit IncludeOrderPPCallbacks(IncludeOrderCheck &Check, SourceManager &SM)
+ : Check(Check), SM(SM) {}
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, const FileEntry *File,
StringRef SearchPath, StringRef RelativePath,
const Module *Imported) override {
- // FIXME: This is a dummy implementation to show how to get at preprocessor
- // information. Implement a real include order check.
- Check.diag(HashLoc, "This is an include");
+ // Report all includes in the main file.
+ if (SM.isInMainFile(HashLoc))
+ Check.addIncludeDirective(HashLoc, FilenameRange, FileName, IsAngled);
}
private:
IncludeOrderCheck &Check;
+ SourceManager &SM;
};
} // namespace
+void IncludeOrderCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ // FIXME: This matcher does nothing at all but without a matcher
+ // onEndOfTranslationUnit is not called.
+ Finder->addMatcher(ast_matchers::decl(), this);
+}
+
void IncludeOrderCheck::registerPPCallbacks(CompilerInstance &Compiler) {
- Compiler.getPreprocessor()
- .addPPCallbacks(new IncludeOrderPPCallbacks(*this));
+ Compiler.getPreprocessor().addPPCallbacks(
+ new IncludeOrderPPCallbacks(*this, Compiler.getSourceManager()));
+ AST = &Compiler.getASTContext();
+ SM = &Compiler.getSourceManager();
+}
+
+static int getCategory(StringRef Filename, bool IsAngled, bool IsMainModule) {
+ // We leave the main module header at the top.
+ if (IsMainModule)
+ return 0;
+
+ // LLVM and clang headers are in the penultimate position.
+ if (Filename.startswith("llvm/") || Filename.startswith("llvm-c/") ||
+ Filename.startswith("clang/") || Filename.startswith("clang-c/"))
+ return 2;
+
+ // System headers are sorted to the end.
+ if (IsAngled || Filename.startswith("gtest/"))
+ return 3;
+
+ // Other headers are inserted between the main module header and LLVM headers.
+ return 1;
+}
+
+/// \brief Find the offset of the next end of a line.
+static int findEndOfLine(const char *Text) {
+ int Offset = 0;
+ while (Text[Offset] != '\0') {
+ // The end of a line can be of three forms: '\r' (old MacOS), '\n' (UNIX),
+ // or '\r\n' (DOS). Check for '\n' to cover UNIX and DOS style. or '\r'
+ // not followed by '\n' to cover the old MacOS style of line ending.
+ if (Text[Offset] == '\n' ||
+ (Text[Offset] == '\r' && Text[Offset + 1] != '\n')) {
+ break; // Stop looking for the end of the line.
+ }
+ ++Offset;
+ }
+ return Offset;
+}
+
+void IncludeOrderCheck::onEndOfTranslationUnit() {
+ LookForMainModule = true;
+ if (IncludeDirectives.empty())
+ return;
+
+ SourceLocation FirstTopLevelDecl;
+ for (const Decl *D : AST->getTranslationUnitDecl()->decls()) {
+ if (SM->isInMainFile(D->getLocation())) {
+ FirstTopLevelDecl = D->getLocation();
+ break;
+ }
+ }
+
+ if (FirstTopLevelDecl.isInvalid())
+ return;
+
+ // Purge includes after the first decl in the translation unit.
+ IncludeDirectives.erase(
+ std::find_if(IncludeDirectives.begin(), IncludeDirectives.end(),
+ [FirstTopLevelDecl, this](const IncludeDirective &I) {
+ return !SM->isBeforeInTranslationUnit(I.Loc, FirstTopLevelDecl);
+ }),
+ IncludeDirectives.end());
+
+ if (IncludeDirectives.empty())
+ return;
+
+ // TODO: find duplicated includes.
+
+ // Form blocks of includes. We don't want to sort across blocks. This also
+ // implicitly makes #defines and #if block include sorting.
+ // FIXME: We should be more careful about sorting below comments as we don't
+ // know if the comment refers to the next include or the whole block that
+ // follows.
+ std::vector<unsigned> Blocks(1, 0);
+ for (unsigned I = 1, E = IncludeDirectives.size(); I != E; ++I)
+ if (SM->getPresumedLineNumber(IncludeDirectives[I].Loc) !=
+ SM->getPresumedLineNumber(IncludeDirectives[I - 1].Loc) + 1)
+ Blocks.push_back(I);
+ Blocks.push_back(IncludeDirectives.size()); // Sentinel value.
+
+ // Get a vector of indices.
+ std::vector<unsigned> IncludeIndices;
+ for (unsigned I = 0, E = IncludeDirectives.size(); I != E; ++I)
+ IncludeIndices.push_back(I);
+
+ // Sort the includes. We first sort by category, then lexicographically.
+ for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI)
+ std::sort(&IncludeIndices[Blocks[BI]], &IncludeIndices[Blocks[BI + 1]],
+ [this](unsigned LHSI, unsigned RHSI) {
+ IncludeDirective &LHS = IncludeDirectives[LHSI];
+ IncludeDirective &RHS = IncludeDirectives[RHSI];
+
+ int CategoryLHS =
+ getCategory(LHS.Filename, LHS.IsAngled, LHS.IsMainModule);
+ int CategoryRHS =
+ getCategory(RHS.Filename, RHS.IsAngled, RHS.IsMainModule);
+
+ return std::tie(CategoryLHS, LHS.Filename) <
+ std::tie(CategoryRHS, RHS.Filename);
+ });
+
+ // Emit a warning for each block but and fixits for all changes within that
+ // block.
+ for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI) {
+ // Find the first include that's not in the right position.
+ unsigned I, E;
+ for (I = Blocks[BI], E = Blocks[BI + 1]; I != E; ++I)
+ if (IncludeIndices[I] != I)
+ break;
+
+ if (I == E)
+ continue;
+
+ // Emit a warning.
+ auto D = diag(IncludeDirectives[I].Loc, "#includes not sorted properly");
+
+ // Emit fix-its for all following includes in this block.
+ for (; I != E; ++I) {
+ if (IncludeIndices[I] == I)
+ continue;
+ const IncludeDirective &CopyFrom = IncludeDirectives[IncludeIndices[I]];
+
+ SourceLocation FromLoc = CopyFrom.Range.getBegin();
+ const char *FromData = SM->getCharacterData(FromLoc);
+ unsigned FromLen = findEndOfLine(FromData);
+
+ StringRef FixedName(FromData, FromLen);
+
+ SourceLocation ToLoc = IncludeDirectives[I].Range.getBegin();
+ const char *ToData = SM->getCharacterData(ToLoc);
+ unsigned ToLen = findEndOfLine(ToData);
+ auto ToRange =
+ CharSourceRange::getCharRange(ToLoc, ToLoc.getLocWithOffset(ToLen));
+
+ D << FixItHint::CreateReplacement(ToRange, FixedName);
+ }
+ }
+
+ IncludeDirectives.clear();
}
} // namespace tidy
Index: clang-tidy/llvm/IncludeOrderCheck.h
===================================================================
--- clang-tidy/llvm/IncludeOrderCheck.h
+++ clang-tidy/llvm/IncludeOrderCheck.h
@@ -20,7 +20,37 @@
/// see: http://llvm.org/docs/CodingStandards.html#include-style
class IncludeOrderCheck : public ClangTidyCheck {
public:
+ IncludeOrderCheck() : LookForMainModule(true) {}
+
void registerPPCallbacks(CompilerInstance &Compiler) override;
+ void onEndOfTranslationUnit() override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+
+ void addIncludeDirective(SourceLocation Loc, CharSourceRange Range,
+ StringRef Filename, bool IsAngled) {
+ // We recognize the first include as a special main module header and want
+ // to leave it in the top position.
+ IncludeDirective ID = {Loc, Range, Filename, IsAngled, false};
+ if (LookForMainModule && !IsAngled) {
+ ID.IsMainModule = true;
+ LookForMainModule = false;
+ }
+ IncludeDirectives.push_back(std::move(ID));
+ }
+
+private:
+ struct IncludeDirective {
+ SourceLocation Loc; ///< '#' location in the include directive
+ CharSourceRange Range; ///< SourceRange for the file name
+ StringRef Filename; ///< Filename as a string
+ bool IsAngled; ///< true if this was an include with angle brackets
+ bool IsMainModule; ///< true if this was the first include in a file
+ };
+ std::vector<IncludeDirective> IncludeDirectives;
+ bool LookForMainModule;
+
+ ASTContext *AST;
+ SourceManager *SM;
};
} // namespace tidy
Index: test/clang-tidy/llvm-include-order.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/llvm-include-order.cpp
@@ -0,0 +1,45 @@
+// RUN: sed 's#// *[A-Z-]\+:.*#//#' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -fix --checks="-*,llvm-include-order" -- -I %S/Inputs > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES -strict-whitespace
+// RUN: FileCheck -input-file=%t.msg %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning}}:"
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: #includes not sorted
+#include "j.h"
+#include "gtest/foo.h"
+#include "i.h"
+#include <s.h>
+#include "llvm/a.h"
+#include "clang/b.h"
+#include "clang-c/c.h" // hi
+#include "llvm-c/d.h" // -c
+
+// CHECK-FIXES: #include "j.h"
+// CHECK-FIXES-NEXT: #include "i.h"
+// CHECK-FIXES-NEXT: #include "clang-c/c.h" // hi
+// CHECK-FIXES-NEXT: #include "clang/b.h"
+// CHECK-FIXES-NEXT: #include "llvm-c/d.h" // -c
+// CHECK-FIXES-NEXT: #include "llvm/a.h"
+// CHECK-FIXES-NEXT: #include "gtest/foo.h"
+// CHECK-FIXES-NEXT: #include <s.h>
+
+#include "b.h"
+#ifdef FOO
+#include "a.h"
+#endif
+
+// CHECK-FIXES: #include "b.h"
+// CHECK-FIXES-NEXT: #ifdef FOO
+// CHECK-FIXES-NEXT: #include "a.h"
+// CHECK-FIXES-NEXT: #endif
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: #includes not sorted
+#include "b.h"
+#include "a.h"
+
+// CHECK-FIXES: #include "a.h"
+// CHECK-FIXES-NEXT: #include "b.h"
+
+int i;
+
+#include "b.h"
+#include "a.h"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
