- Moved everything into the PPCallback. Required adding some accessors to
ClangTidyCheck.
- Removed check for "first decl", don't report errors from the main file.
- Made test use check_clang_tidy_fix.sh. Add dummy headers and make the script
forward arguments.
http://reviews.llvm.org/D4741
Files:
clang-tidy/ClangTidy.h
clang-tidy/llvm/IncludeOrderCheck.cpp
test/clang-tidy/Inputs/Headers/a.h
test/clang-tidy/Inputs/Headers/b.h
test/clang-tidy/Inputs/Headers/clang-c/c.h
test/clang-tidy/Inputs/Headers/clang/b.h
test/clang-tidy/Inputs/Headers/gtest/foo.h
test/clang-tidy/Inputs/Headers/i.h
test/clang-tidy/Inputs/Headers/j.h
test/clang-tidy/Inputs/Headers/llvm-c/d.h
test/clang-tidy/Inputs/Headers/llvm/a.h
test/clang-tidy/Inputs/Headers/s.h
test/clang-tidy/Inputs/empty.h
test/clang-tidy/check_clang_tidy_fix.sh
test/clang-tidy/llvm-include-order.cpp
Index: clang-tidy/ClangTidy.h
===================================================================
--- clang-tidy/ClangTidy.h
+++ clang-tidy/ClangTidy.h
@@ -78,14 +78,20 @@
/// \brief The infrastructure sets the context to \p Ctx with this function.
void setContext(ClangTidyContext *Ctx) { Context = Ctx; }
+ /// \brief Retrieve the context associated with this checker.
+ ClangTidyContext *getContext() const { return Context; }
+
/// \brief Add a diagnostic with the check's name.
DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,
DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
/// \brief Sets the check name. Intended to be used by the clang-tidy
/// framework. Can be called only once.
void setName(StringRef Name);
+ /// \brief Gets the name of this checker.
+ const std::string &getName() const { return CheckName; }
+
private:
void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
ClangTidyContext *Context;
Index: clang-tidy/llvm/IncludeOrderCheck.cpp
===================================================================
--- clang-tidy/llvm/IncludeOrderCheck.cpp
+++ clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -18,26 +18,167 @@
namespace {
class IncludeOrderPPCallbacks : public PPCallbacks {
public:
- explicit IncludeOrderPPCallbacks(IncludeOrderCheck &Check) : Check(Check) {}
+ explicit IncludeOrderPPCallbacks(std::string CheckName,
+ ClangTidyContext &Context, SourceManager &SM)
+ : CheckName(std::move(CheckName)), Context(Context), 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");
- }
+ const Module *Imported) override;
+ void EndOfMainFile() override;
private:
- IncludeOrderCheck &Check;
+ 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;
+
+ std::string CheckName;
+ ClangTidyContext &Context;
+ SourceManager &SM;
};
} // namespace
void IncludeOrderCheck::registerPPCallbacks(CompilerInstance &Compiler) {
- Compiler.getPreprocessor()
- .addPPCallbacks(new IncludeOrderPPCallbacks(*this));
+ // We store a copy of the name and context pointer because clang checks die
+ // before PPCallbacks get an EndOfMainFile call.
+ Compiler.getPreprocessor().addPPCallbacks(new IncludeOrderPPCallbacks(
+ getName(), *getContext(), Compiler.getSourceManager()));
+}
+
+static int getPriority(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 IncludeOrderPPCallbacks::InclusionDirective(
+ SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
+ bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
+ StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+ // We recognize the first include as a special main module header and want
+ // to leave it in the top position.
+ IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};
+ if (LookForMainModule && !IsAngled) {
+ ID.IsMainModule = true;
+ LookForMainModule = false;
+ }
+ IncludeDirectives.push_back(std::move(ID));
+}
+
+void IncludeOrderPPCallbacks::EndOfMainFile() {
+ LookForMainModule = true;
+ 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 priority, 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 PriorityLHS =
+ getPriority(LHS.Filename, LHS.IsAngled, LHS.IsMainModule);
+ int PriorityRHS =
+ getPriority(RHS.Filename, RHS.IsAngled, RHS.IsMainModule);
+
+ return std::tie(PriorityLHS, LHS.Filename) <
+ std::tie(PriorityRHS, RHS.Filename);
+ });
+
+ // Emit a warning for each block 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 = Context.diag(CheckName, 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/check_clang_tidy_fix.sh
===================================================================
--- test/clang-tidy/check_clang_tidy_fix.sh
+++ test/clang-tidy/check_clang_tidy_fix.sh
@@ -5,14 +5,15 @@
INPUT_FILE=$1
CHECK_TO_RUN=$2
TEMPORARY_FILE=$3.cpp
+shift 3
# Remove the contents of the CHECK lines to avoid CHECKs matching on themselves.
# We need to keep the comments to preserve line numbers while avoiding empty
# lines which could potentially trigger formatting-related checks.
sed 's#// *[A-Z-]\+:.*#//#' ${INPUT_FILE} > ${TEMPORARY_FILE}
clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" -- --std=c++11 \
- > ${TEMPORARY_FILE}.msg 2>&1 || exit $?
+ $* > ${TEMPORARY_FILE}.msg 2>&1 || exit $?
FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} \
-check-prefix=CHECK-FIXES -strict-whitespace || exit $?
Index: test/clang-tidy/llvm-include-order.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/llvm-include-order.cpp
@@ -0,0 +1,38 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s llvm-include-order %t -isystem %S/Inputs/Headers
+// REQUIRES: shell
+
+// 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"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
