Correctly classify main file includes if there is a prefix added

[jbangert]

Summary

Prevents misclassifying includes based on the command-line filename (e.g. if a project is in a subdirectory).

This is slightly more robust than the additional duplicate detection, however the current classification scheme is still kind of brittle for a lot of code.

[alexfh]

Julian, I would appreciate if you could produce a test case for this fix, since otherwise it would be hard to track, if this regresses.

[jbangert]

Instead modify IncludeInserter to classify correctly.

[jbangert]

I slightly changed the approach of this CL, and added a test. This now also works in clang_tidy is called from a different working directory.

[alexfh]

LG. Thank you for the fix!

[alexfh]

Tests don't pass with this patch applied:

$ ninja check-clang-tools                                                                              
[22/23] Running the Clang extra tools' regression tests
FAIL: Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized (427 of 489)
******************** TEST 'Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized' FAILED ********************
Note: Google Test filter = IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from IncludeInserterTest
[ RUN      ] IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized
1 warning and 1 error generated.
LLVM ERROR: 'a/header.h' file not found
foo, bar


********************
Testing Time: 5.32s
********************
Failing Tests (1):
    Extra Tools Unit Tests :: clang-tidy/ClangTidyTests/IncludeInserterTest.DontInsertDuplicateIncludeEvenIfMiscategorized

  Expected Passes    : 487
  Expected Failures  : 1
  Unexpected Failures: 1
FAILED: tools/clang/tools/extra/test/CMakeFiles/check-clang-tools 
cd /build/tools/clang/tools/extra/test && /usr/bin/python2.7 /src/utils/lit/lit.py -sv /build/tools/clang/tools/extra/test
ninja: build stopped: subcommand failed.

[jbangert]

• Add header to unit test fixture.

[alexfh]

LG, committed as r291767.

clang-tools-extra/trunk/clang-tidy/utils/IncludeSorter.cpp

DetermineIncludeKind(StringRef CanonicalFile, StringRef IncludeFile,
   // replaced by "/internal/".
   if (IsAngled) {
     // If the system include (<foo>) ends with ".h", then it is a normal C-style
     // include. Otherwise assume it is a C++-style extensionless include.
     return IncludeFile.endswith(".h") ? IncludeSorter::IK_CSystemInclude
                                       : IncludeSorter::IK_CXXSystemInclude;
   }
   StringRef CanonicalInclude = MakeCanonicalName(IncludeFile, Style);
-  if (CanonicalFile.equals(CanonicalInclude)) {
+  if (CanonicalFile.endswith(CanonicalInclude)
+      || CanonicalInclude.endswith(CanonicalFile)) {
     return IncludeSorter::IK_MainTUInclude;
   }
   if (Style == IncludeSorter::IS_Google) {
     std::pair<StringRef, StringRef> Parts = CanonicalInclude.split("/public/");
     std::string AltCanonicalInclude =
         Parts.first.str() + "/internal/" + Parts.second.str();
     std::string ProtoCanonicalInclude =
         Parts.first.str() + "/proto/" + Parts.second.str();

clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp

NonSystemHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
   std::vector<StringRef> HeadersToInclude() const override {
     return {"path/to/header.h"};
   }
   bool IsAngledInclude() const override { return false; }
 };
 
+class EarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  EarlyInAlphabetHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
+      : IncludeInserterCheckBase(CheckName, Context) {}
+
+  std::vector<StringRef> HeadersToInclude() const override {
+    return {"a/header.h"};
+  }
+  bool IsAngledInclude() const override { return false; }
+};
+
 class MultipleHeaderInserterCheck : public IncludeInserterCheckBase {
 public:
   MultipleHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
   std::vector<StringRef> HeadersToInclude() const override {
     return {"path/to/header.h", "path/to/header2.h", "path/to/header.h"};
   }
std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector<ClangTidyError> Errors;
   return test::runCheckOnCode<Check>(Code, &Errors, Filename, None,
                                      ClangTidyOptions(),
                                      {// Main file include
                                       {"clang_tidy/tests/"
                                        "insert_includes_test_header.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"},
                                       // Fake system headers.
                                       {"stdlib.h", "\n"},
                                       {"unistd.h", "\n"},
                                       {"list", "\n"},
void foo() {
   int a = 0;
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
                           PreCode, "devtools/cymbal/clang_tidy/tests/"
                                    "insert_includes_test_header.cc"));
 }
 
+TEST(IncludeInserterTest, DontInsertDuplicateIncludeEvenIfMiscategorized) {
+  const char *PreCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "a/header.h"
+#include "path/to/a/header.h"
+#include "path/to/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  const char *PostCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "a/header.h"
+#include "path/to/a/header.h"
+#include "path/to/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode<EarlyInAlphabetHeaderInserterCheck>(
+                          PreCode, "workspace_folder/clang_tidy/tests/"
+                                   "insert_includes_test_header.cc"));
+}
+
+TEST(IncludeInserterTest, HandleOrderInSubdirectory) {
+  const char *PreCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "path/to/a/header.h"
+#include "path/to/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  const char *PostCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "a/header.h"
+#include "path/to/a/header.h"
+#include "path/to/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode<EarlyInAlphabetHeaderInserterCheck>(
+                          PreCode, "workspace_folder/clang_tidy/tests/"
+                                   "insert_includes_test_header.cc"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang
 
 #endif