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 IncludeDirectives; + bool LookForMainModule; + + ASTContext *AST; + SourceManager *SM; }; } // namespace tidy 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 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 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: 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 +#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 + +#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"