from:"Joe Turner via Phabricator via cfe\-commits"

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-26 Thread Joe Turner via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e076a8d8099: Make sure Objective-C category support in 
IncludeSorter handles top-level… (authored by compositeprimes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89608/new/

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,23 +712,21 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
 })";
 
-  EXPECT_EQ(
-  PostCode,
-  runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "top_level_test_header.mm"));
 }
 
 TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t StartIndex = Canonical.find_last_of('/');
+if (StartIndex == StringRef::npos) {
+  StartIndex = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', StartIndex));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,23 +712,21 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-26 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 300749.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89608/new/

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,23 +712,21 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
 })";
 
-  EXPECT_EQ(
-  PostCode,
-  runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "top_level_test_header.mm"));
 }
 
 TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t StartIndex = Canonical.find_last_of('/');
+if (StartIndex == StringRef::npos) {
+  StartIndex = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', StartIndex));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,23 +712,21 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
 })";
 
-  EXPECT_EQ(
-  PostCode,
-  runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  EXPECT_EQ(PostCode, runC

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-23 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 300402.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89608/new/

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,14 +712,14 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
@@ -724,7 +728,7 @@
   EXPECT_EQ(
   PostCode,
   runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  PreCode, "top_level_test_header.mm"));
 }
 
 TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -133,7 +133,7 @@
  utils::IncludeSorter::IS_Google_ObjC) {}
 
   std::vector headersToInclude() const override {
-return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+return {"top_level_test_header+foo.h"};
   }
 };
 
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,14 +712,14 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
@@ -724,7 +728,7 @@
   EXPECT_EQ(
   PostCode,
   runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  PreCode, "top_level_test_header.mm"));
 }

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-23 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes added a comment.

In D89608#2336596 , @gribozavr2 wrote:

> Any chance of adding a test?

Done!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89608/new/

https://reviews.llvm.org/D89608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-23 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 300380.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89608/new/

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,14 +712,14 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
@@ -724,7 +728,7 @@
   EXPECT_EQ(
   PostCode,
   runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  PreCode, "top_level_test_header.mm"));
 }
 
 TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -158,6 +158,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // Top-level main file include +
+  // category.
+  {"top_level_test_header.h", "\n"},
+  {"top_level_test_header+foo.h", "\n"},
   // ObjC category.
   {"clang_tidy/tests/"
"insert_includes_test_header+foo.h",
@@ -708,14 +712,14 @@
 
 TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
   const char *PreCode = R"(
-#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "top_level_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"
+#import "top_level_test_header.h"
+#import "top_level_test_header+foo.h"
 
 void foo() {
   int a = 0;
@@ -724,7 +728,7 @@
   EXPECT_EQ(
   PostCode,
   runCheckOnCode(
-  PreCode, "repo/clang_tidy/tests/insert_includes_test_header.mm"));
+  PreCode, "top_level_test_header.mm"));
 }
 
 TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

2020-10-16 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added reviewers: alexfh, gribozavr.
compositeprimes added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
compositeprimes requested review of this revision.

Currently, this would not correctly associate a category with the related 
include if it was top-level (i.e. no slashes in the path). This ensures that we 
explicitly think about that case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-14 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes added a comment.

clang-format is complaining about my ordering of headers in my test case in 
IncludeInserterTest (but that's correct objc formatting). Not sure if there's a 
way to disable that on those lines


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-14 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298264.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 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 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,42 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector 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 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 headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,12 +158,20 @@
   {"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 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
 }
 
 TEST(IncludeInserterTest, InsertMultipleIncludesAndDeduplicate) {
@@ -191,9 +237,9 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/i

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-14 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298241.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 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 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,42 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector 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 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 headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,12 +158,20 @@
   {"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 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
 }
 
 TEST(IncludeInserterTest, InsertMultipleIncludesAndDeduplicate) {
@@ -191,9 +237,9 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/i

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-14 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298203.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 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 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,42 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector 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 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 headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,12 +158,20 @@
   {"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 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/insert_includes_test_input2.cc"));
 }
 
 TEST(IncludeInserterTest, InsertMultipleIncludesAndDeduplicate) {
@@ -191,9 +237,9 @@
   int a = 0;
 })";
 
-  EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "clang_tidy/tests/"
-   "insert_includes_test_input2.cc"));
+  EXPECT_EQ(PostCode,
+runCheckOnCode(
+PreCode, "clang_tidy/tests/i

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-13 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298029.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 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 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,42 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector 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 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 headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,12 +158,19 @@
   {"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"},
@@ -635,6 +680,78 @@
"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(
+  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

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-13 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298027.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -717,7 +717,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
@@ -748,7 +748,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -717,7 +717,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
@@ -748,7 +748,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-13 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 298026.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:





___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-13 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 297980.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -717,7 +717,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
@@ -748,7 +748,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 


Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -717,7 +717,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
@@ -748,7 +748,7 @@
 })";
 
   EXPECT_EQ(PostCode, runCheckOnCode(
-  PreCode, "devtools/cymbal/clang_tidy/tests/"
+  PreCode, "repo/clang_tidy/tests/"
"insert_includes_test_header.mm"));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-12 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 297728.
compositeprimes edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89276/new/

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -137,6 +137,18 @@
   }
 };
 
+class ObjCGeneratedHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCGeneratedHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -156,6 +168,9 @@
   {"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"},
@@ -706,6 +721,37 @@
"insert_includes_test_header.mm"));
 }
 
+TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
+  const char *PreCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+#include 
+#include 
+
+#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 
+#include 
+
+#include "path/to/a/header.h"
+
+#import "clang_tidy/tests/generated_file.proto.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "devtools/cymbal/clang_tidy/tests/"
+   "insert_includes_test_header.mm"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -31,7 +31,8 @@
 IK_CSystemInclude = 1,   ///< e.g. ``#include ``
 IK_CXXSystemInclude = 2, ///< e.g. ``#include ``
 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
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -91,6 +91,12 @@
   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;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89276: Support ObjC in IncludeInserter

2020-10-12 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added a reviewer: alexfh.
compositeprimes added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
compositeprimes requested review of this revision.

Update IncludeSorter/IncludeInserter to support objective-c google style (part 
1):

1. Correctly consider .mm/.m extensions
2. Correctly categorize category headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 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 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,30 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector 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 headersToInclude() const override {
+return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,6 +146,10 @@
   {"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"},
@@ -635,6 +665,47 @@
"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(
+  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(
+  PreCode, "devtools/cymbal/clang_tidy/tests/"
+   "insert_includes_test_header.mm"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -23,7 +23,7

[PATCH] D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError

2020-02-21 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 245915.
compositeprimes edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68887/new/

https://reviews.llvm.org/D68887

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp


Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -62,6 +62,9 @@
 }
 assert(Error.Message.Message.empty() && "Overwriting a diagnostic 
message");
 Error.Message = TidyMessage;
+for (const CharSourceRange &SourceRange : Ranges) {
+  Error.Ranges.emplace_back(Loc.getManager(), SourceRange);
+}
   }
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,


Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -62,6 +62,9 @@
 }
 assert(Error.Message.Message.empty() && "Overwriting a diagnostic message");
 Error.Message = TidyMessage;
+for (const CharSourceRange &SourceRange : Ranges) {
+  Error.Ranges.emplace_back(Loc.getManager(), SourceRange);
+}
   }
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-21 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes added a comment.

I don't have commit rights (this is my first llvm commit!), so could someone 
help out? Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-20 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes added inline comments.



Comment at: include/clang/Tooling/Core/Diagnostic.h:51
+/// Represents a single source ranges to be associated with a diagnostic.
+struct DiagnosticAssociatedRange {
+  DiagnosticAssociatedRange() = default;

alexfh wrote:
> This comment was lost by Phabricator (or I just wanted to leave it without 
> actually doing it?).
> 
> The name assumes a specific usage. I'd try to make it more generic in case 
> there's another use case for the struct, for example: `ByteRange` (as opposed 
> to `TextRange`, which would be {start line, start column, end line, end 
> column}) or `FileByteRange` (if we want to stress that it contains a file 
> name).
Ah good idea, I like that name, Done


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-20 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 245717.
compositeprimes marked an inline comment as done.
compositeprimes added a comment.

Added tests for Yaml serialization


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782

Files:
  include/clang/Tooling/Core/Diagnostic.h
  include/clang/Tooling/DiagnosticsYaml.h
  lib/Tooling/Core/Diagnostic.cpp
  unittests/Tooling/DiagnosticsYamlTest.cpp

Index: unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- unittests/Tooling/DiagnosticsYamlTest.cpp
+++ unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/Tooling/DiagnosticsYaml.h"
+#include "third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/ReplacementsYaml.h"
 #include "gtest/gtest.h"
@@ -30,13 +31,24 @@
   return DiagMessage;
 }
 
+static FileByteRange makeByteRange(int FileOffset,
+   int Length,
+   const std::string &FilePath) {
+  FileByteRange Range;
+  Range.FileOffset = FileOffset;
+  Range.Length = Length;
+  Range.FilePath = FilePath;
+  return Range;
+}
+
 static Diagnostic makeDiagnostic(StringRef DiagnosticName,
  const std::string &Message, int FileOffset,
  const std::string &FilePath,
- const StringMap &Fix) {
+ const StringMap &Fix,
+ const SmallVector &Ranges) {
   return Diagnostic(DiagnosticName,
 makeMessage(Message, FileOffset, FilePath, Fix), {},
-Diagnostic::Warning, "path/to/build/directory");
+Diagnostic::Warning, "path/to/build/directory", Ranges);
 }
 
 static const char *YAMLContent =
@@ -63,6 +75,10 @@
 "  Offset:  62\n"
 "  Length:  2\n"
 "  ReplacementText: 'replacement #2'\n"
+"Ranges:\n"
+"  - FilePath:'path/to/source.cpp'\n"
+"FileOffset:  10\n"
+"Length:  10\n"
 "  - DiagnosticName:  'diagnostic#3'\n"
 "DiagnosticMessage:\n"
 "  Message: 'message #3'\n"
@@ -88,16 +104,18 @@
   {"path/to/source.cpp",
Replacements({"path/to/source.cpp", 100, 12, "replacement #1"})}};
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#1", "message #1", 55,
-   "path/to/source.cpp", Fix1));
+   "path/to/source.cpp", Fix1, {}));
 
   StringMap Fix2 = {
   {"path/to/header.h",
Replacements({"path/to/header.h", 62, 2, "replacement #2"})}};
+  SmallVector Ranges2 =
+  {makeByteRange(10, 10, "path/to/source.cpp")};
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#2", "message #2", 60,
-   "path/to/header.h", Fix2));
+   "path/to/header.h", Fix2, Ranges2));
 
   TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72,
-   "path/to/source2.cpp", {}));
+   "path/to/source2.cpp", {}, {}));
   TUD.Diagnostics.back().Notes.push_back(
   makeMessage("Note1", 88, "path/to/note1.cpp", {}));
   TUD.Diagnostics.back().Notes.push_back(
@@ -142,6 +160,7 @@
   EXPECT_EQ(100u, Fixes1[0].getOffset());
   EXPECT_EQ(12u, Fixes1[0].getLength());
   EXPECT_EQ("replacement #1", Fixes1[0].getReplacementText());
+  EXPECT_TRUE(D1.Ranges.empty());
 
   Diagnostic D2 = TUDActual.Diagnostics[1];
   EXPECT_EQ("diagnostic#2", D2.DiagnosticName);
@@ -154,6 +173,10 @@
   EXPECT_EQ(62u, Fixes2[0].getOffset());
   EXPECT_EQ(2u, Fixes2[0].getLength());
   EXPECT_EQ("replacement #2", Fixes2[0].getReplacementText());
+  EXPECT_EQ(1, D2.Ranges.size());
+  EXPECT_EQ("path/to/source.cpp", D2.Ranges[0].FilePath);
+  EXPECT_EQ(10, D2.Ranges[0].FileOffset);
+  EXPECT_EQ(10, D2.Ranges[0].Length);
 
   Diagnostic D3 = TUDActual.Diagnostics[2];
   EXPECT_EQ("diagnostic#3", D3.DiagnosticName);
@@ -169,4 +192,5 @@
   EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath);
   std::vector Fixes3 = getFixes(D3.Message.Fix);
   EXPECT_TRUE(Fixes3.empty());
+  EXPECT_TRUE(D3.Ranges.empty());
 }
Index: lib/Tooling/Core/Diagnostic.cpp
===
--- lib/Tooling/Core/Diagnostic.cpp
+++ lib/Tooling/Core/Diagnostic.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/Tooling/Core/Diagnostic.h"
+#include "third_party/llvm/llvm-project/clang/include/clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #includ

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-18 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes added a comment.

Sorry about the lack of context on the last upload, this one should have it all




Comment at: include/clang/Tooling/Core/Diagnostic.h:62-70
+/// Represents extra source ranges to be associated with a diagnostic.
+struct DiagnosticAssociatedRanges {
+  DiagnosticAssociatedRanges() = default;
+
+  DiagnosticAssociatedRanges(const SourceManager &Sources,
+ ArrayRef SourceRanges);
+

alexfh wrote:
> Why is this needed? Shouldn't `LLVM_YAML_IS_SEQUENCE_VECTOR` be enough to 
> allow for SmallVector to be yaml 
> serializable? Seems to work with `DiagnosticMessage` and `Diagnostic::Notes`.
Yeah, you're right it's not really needed. I had been trying to make it easier 
to convert a vector to vector, but it's 
really not that hard as is.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-18 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 245207.
compositeprimes marked 2 inline comments as done.
compositeprimes added a comment.

Removed unnecessary DiagnosticAssociatedRanges struct


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782

Files:
  include/clang/Tooling/Core/Diagnostic.h
  include/clang/Tooling/DiagnosticsYaml.h
  lib/Tooling/Core/Diagnostic.cpp

Index: lib/Tooling/Core/Diagnostic.cpp
===
--- lib/Tooling/Core/Diagnostic.cpp
+++ lib/Tooling/Core/Diagnostic.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/Tooling/Core/Diagnostic.h"
+#include "third_party/llvm/llvm-project/clang/include/clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -34,6 +35,16 @@
 FileOffset = Sources.getFileOffset(Loc);
 }
 
+DiagnosticAssociatedRange::DiagnosticAssociatedRange(
+const SourceManager &Sources, CharSourceRange Range)
+: FileOffset(0), Length(0) {
+  FilePath = std::string(Sources.getFilename(Range.getBegin()));
+  if (!FilePath.empty()) {
+FileOffset = Sources.getFileOffset(Range.getBegin());
+Length = Sources.getFileOffset(Range.getEnd()) - FileOffset;
+  }
+}
+
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
Diagnostic::Level DiagLevel, StringRef BuildDirectory)
 : DiagnosticName(DiagnosticName), DiagLevel(DiagLevel),
@@ -42,9 +53,10 @@
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
const SmallVector &Notes,
-   Level DiagLevel, llvm::StringRef BuildDirectory)
+   Level DiagLevel, llvm::StringRef BuildDirectory,
+   const SmallVector &Ranges)
 : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
-  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {}
 
 const llvm::StringMap *selectFirstFix(const Diagnostic& D) {
if (!D.Message.Fix.empty())
Index: include/clang/Tooling/DiagnosticsYaml.h
===
--- include/clang/Tooling/DiagnosticsYaml.h
+++ include/clang/Tooling/DiagnosticsYaml.h
@@ -20,12 +20,21 @@
 #include "llvm/Support/YAMLTraits.h"
 #include 
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::DiagnosticAssociatedRange)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Diagnostic)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::DiagnosticMessage)
 
 namespace llvm {
 namespace yaml {
 
+template <> struct MappingTraits {
+  static void mapping(IO &Io, clang::tooling::DiagnosticAssociatedRange &R) {
+Io.mapRequired("FilePath", R.FilePath);
+Io.mapRequired("FileOffset", R.FileOffset);
+Io.mapRequired("Length", R.Length);
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO &Io, clang::tooling::DiagnosticMessage &M) {
 Io.mapRequired("Message", M.Message);
@@ -58,11 +67,12 @@
 
 NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D)
 : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes),
-  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {}
+  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory),
+  Ranges(D.Ranges) {}
 
 clang::tooling::Diagnostic denormalize(const IO &) {
   return clang::tooling::Diagnostic(DiagnosticName, Message, Notes,
-DiagLevel, BuildDirectory);
+DiagLevel, BuildDirectory, Ranges);
 }
 
 std::string DiagnosticName;
@@ -71,6 +81,7 @@
 SmallVector Notes;
 clang::tooling::Diagnostic::Level DiagLevel;
 std::string BuildDirectory;
+SmallVector Ranges;
   };
 
   static void mapping(IO &Io, clang::tooling::Diagnostic &D) {
@@ -79,6 +90,7 @@
 Io.mapRequired("DiagnosticName", Keys->DiagnosticName);
 Io.mapRequired("DiagnosticMessage", Keys->Message);
 Io.mapOptional("Notes", Keys->Notes);
+Io.mapOptional("Ranges", Keys->Ranges);
 
 // FIXME: Export properly all the different fields.
   }
Index: include/clang/Tooling/Core/Diagnostic.h
===
--- include/clang/Tooling/Core/Diagnostic.h
+++ include/clang/Tooling/Core/Diagnostic.h
@@ -47,6 +47,18 @@
   llvm::StringMap Fix;
 };
 
+/// Represents a single source ranges to be associated with a diagnostic.
+struct DiagnosticAssociatedRange {
+  DiagnosticAssociatedRange() = default;
+
+  DiagnosticAssociatedRange(const SourceManager &Sources,
+CharSourceRange Range);
+
+  std::string FilePath;
+  unsigned FileOffset;
+  unsigned Length;
+};
+
 /// Represents the diagnostic with the level of severity and possi

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2020-02-14 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes updated this revision to Diff 244737.
compositeprimes added a comment.

Updated to serialize the ranges in yaml. This required making a few 
abstractions around the representation of CharSourceRange.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69782/new/

https://reviews.llvm.org/D69782

Files:
  include/clang/Tooling/Core/Diagnostic.h
  include/clang/Tooling/DiagnosticsYaml.h
  lib/Tooling/Core/Diagnostic.cpp

Index: lib/Tooling/Core/Diagnostic.cpp
===
--- lib/Tooling/Core/Diagnostic.cpp
+++ lib/Tooling/Core/Diagnostic.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/Tooling/Core/Diagnostic.h"
+#include "third_party/llvm/llvm-project/clang/include/clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -34,6 +35,23 @@
 FileOffset = Sources.getFileOffset(Loc);
 }
 
+DiagnosticAssociatedRange::DiagnosticAssociatedRange(
+const SourceManager &Sources, CharSourceRange Range)
+: FileOffset(0), Length(0) {
+  FilePath = std::string(Sources.getFilename(Range.getBegin()));
+  if (!FilePath.empty()) {
+FileOffset = Sources.getFileOffset(Range.getBegin());
+Length = Sources.getFileOffset(Range.getEnd()) - FileOffset;
+  }
+}
+
+DiagnosticAssociatedRanges::DiagnosticAssociatedRanges(
+const SourceManager &Sources, ArrayRef SourceRanges) {
+  for (const CharSourceRange &Range : SourceRanges) {
+Ranges.emplace_back(DiagnosticAssociatedRange(Sources, Range));
+  }
+}
+
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
Diagnostic::Level DiagLevel, StringRef BuildDirectory)
 : DiagnosticName(DiagnosticName), DiagLevel(DiagLevel),
@@ -42,9 +60,10 @@
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
const SmallVector &Notes,
-   Level DiagLevel, llvm::StringRef BuildDirectory)
+   Level DiagLevel, llvm::StringRef BuildDirectory,
+   const DiagnosticAssociatedRanges &Ranges)
 : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
-  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {}
 
 const llvm::StringMap *selectFirstFix(const Diagnostic& D) {
if (!D.Message.Fix.empty())
Index: include/clang/Tooling/DiagnosticsYaml.h
===
--- include/clang/Tooling/DiagnosticsYaml.h
+++ include/clang/Tooling/DiagnosticsYaml.h
@@ -20,12 +20,27 @@
 #include "llvm/Support/YAMLTraits.h"
 #include 
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::DiagnosticAssociatedRange)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Diagnostic)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::DiagnosticMessage)
 
 namespace llvm {
 namespace yaml {
 
+template <> struct MappingTraits {
+  static void mapping(IO &Io, clang::tooling::DiagnosticAssociatedRange &R) {
+Io.mapRequired("FilePath", R.FilePath);
+Io.mapRequired("FileOffset", R.FileOffset);
+Io.mapRequired("Length", R.Length);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &Io, clang::tooling::DiagnosticAssociatedRanges &R) {
+Io.mapRequired("Ranges", R.Ranges);
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO &Io, clang::tooling::DiagnosticMessage &M) {
 Io.mapRequired("Message", M.Message);
@@ -58,11 +73,12 @@
 
 NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D)
 : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes),
-  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {}
+  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory),
+  Ranges(D.Ranges) {}
 
 clang::tooling::Diagnostic denormalize(const IO &) {
   return clang::tooling::Diagnostic(DiagnosticName, Message, Notes,
-DiagLevel, BuildDirectory);
+DiagLevel, BuildDirectory, Ranges);
 }
 
 std::string DiagnosticName;
@@ -71,6 +87,7 @@
 SmallVector Notes;
 clang::tooling::Diagnostic::Level DiagLevel;
 std::string BuildDirectory;
+clang::tooling::DiagnosticAssociatedRanges Ranges;
   };
 
   static void mapping(IO &Io, clang::tooling::Diagnostic &D) {
@@ -79,6 +96,7 @@
 Io.mapRequired("DiagnosticName", Keys->DiagnosticName);
 Io.mapRequired("DiagnosticMessage", Keys->Message);
 Io.mapOptional("Notes", Keys->Notes);
+Io.mapOptional("Ranges", Keys->Ranges);
 
 // FIXME: Export properly all the different fields.
   }
Index: include/clang/Tooling/Core/Diagnostic.h
===
--- include/clang/Tooling/Core/Diagnostic.h
+++ include/cla

[PATCH] D68885: Add a Ranges field to Diagnostic struct

2019-11-03 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes marked 3 inline comments as done.
compositeprimes added a comment.

Sorry for the delay; I got caught up in some other work.

I moved this to https://reviews.llvm.org/D69782 using arc diff.

@gribozavr: I'm not sure how to merge this with the other commit, because it's 
in clang-tidy; do you think you could share how this might be done?

Thanks!




Comment at: include/clang/Tooling/Core/Diagnostic.h:92
+  /// `Message` field, but if `Ranges` is nonempty, they will be preferred
+  /// representation of the source code associated with the diagnostic.
+  ArrayRef Ranges;

gribozavr wrote:
> I don't understand the "preferred" wording here. What do you mean?
> 
> Also, this is a departure from how compiler warnings and ClangTidy messages 
> work. Additional ranges are used to highlight parts of the code relevant to 
> the problem, not to reposition the diagnostic.
Reworded to make this more generic



Comment at: include/clang/Tooling/Core/Diagnostic.h:93
+  /// representation of the source code associated with the diagnostic.
+  ArrayRef Ranges;
 };

alexfh wrote:
> Please add YAML serialization/deserialization code for this. See 
> include/clang/Tooling/DiagnosticsYaml.h
How to I add Yaml support for CharSourceRange? Should I add the relevant 
`template <> struct MappingTraits`, and if so where would this go?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68885/new/

https://reviews.llvm.org/D68885



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

2019-11-03 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added reviewers: gribozavr, alexfh.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D69782

Files:
  include/clang/Tooling/Core/Diagnostic.h
  include/clang/Tooling/DiagnosticsYaml.h
  lib/Tooling/Core/Diagnostic.cpp


Index: lib/Tooling/Core/Diagnostic.cpp
===
--- lib/Tooling/Core/Diagnostic.cpp
+++ lib/Tooling/Core/Diagnostic.cpp
@@ -42,9 +42,10 @@
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
const SmallVector &Notes,
-   Level DiagLevel, llvm::StringRef BuildDirectory)
+   Level DiagLevel, llvm::StringRef BuildDirectory,
+   ArrayRef Ranges)
 : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
-  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {}
 
 const llvm::StringMap *selectFirstFix(const Diagnostic& D) {
if (!D.Message.Fix.empty())
Index: include/clang/Tooling/DiagnosticsYaml.h
===
--- include/clang/Tooling/DiagnosticsYaml.h
+++ include/clang/Tooling/DiagnosticsYaml.h
@@ -58,11 +58,12 @@
 
 NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D)
 : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes),
-  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {}
+  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory),
+  Ranges(D.Ranges) {}
 
 clang::tooling::Diagnostic denormalize(const IO &) {
   return clang::tooling::Diagnostic(DiagnosticName, Message, Notes,
-DiagLevel, BuildDirectory);
+DiagLevel, BuildDirectory, Ranges);
 }
 
 std::string DiagnosticName;
@@ -71,6 +72,7 @@
 SmallVector Notes;
 clang::tooling::Diagnostic::Level DiagLevel;
 std::string BuildDirectory;
+ArrayRef Ranges;
   };
 
   static void mapping(IO &Io, clang::tooling::Diagnostic &D) {
Index: include/clang/Tooling/Core/Diagnostic.h
===
--- include/clang/Tooling/Core/Diagnostic.h
+++ include/clang/Tooling/Core/Diagnostic.h
@@ -62,7 +62,7 @@
 
   Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message,
  const SmallVector &Notes, Level DiagLevel,
- llvm::StringRef BuildDirectory);
+ llvm::StringRef BuildDirectory, ArrayRef Ranges);
 
   /// Name identifying the Diagnostic.
   std::string DiagnosticName;
@@ -84,6 +84,10 @@
   ///
   /// Note: it is empty in unittest.
   std::string BuildDirectory;
+
+  /// Extra source ranges associated with the diagnostic (in addition to the
+  /// location of the Message above).
+  ArrayRef Ranges;
 };
 
 /// Collection of Diagnostics generated from a single translation unit.


Index: lib/Tooling/Core/Diagnostic.cpp
===
--- lib/Tooling/Core/Diagnostic.cpp
+++ lib/Tooling/Core/Diagnostic.cpp
@@ -42,9 +42,10 @@
 Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
const SmallVector &Notes,
-   Level DiagLevel, llvm::StringRef BuildDirectory)
+   Level DiagLevel, llvm::StringRef BuildDirectory,
+   ArrayRef Ranges)
 : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
-  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+  DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {}
 
 const llvm::StringMap *selectFirstFix(const Diagnostic& D) {
if (!D.Message.Fix.empty())
Index: include/clang/Tooling/DiagnosticsYaml.h
===
--- include/clang/Tooling/DiagnosticsYaml.h
+++ include/clang/Tooling/DiagnosticsYaml.h
@@ -58,11 +58,12 @@
 
 NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D)
 : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes),
-  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {}
+  DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory),
+  Ranges(D.Ranges) {}
 
 clang::tooling::Diagnostic denormalize(const IO &) {
   return clang::tooling::Diagnostic(DiagnosticName, Message, Notes,
-DiagLevel, BuildDirectory);
+DiagLevel, BuildDirectory, Ranges);
 }
 
 std::string DiagnosticName;
@@ -71,6 +72,7 @@
 SmallVector Notes;
 clang::tooling::Diagnostic::Level DiagLevel;
 std::string Bui

[PATCH] D68887: Copy the Ranges field from the Diagnostic when creating the ClangTidyError

2019-10-11 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added a reviewer: clang-tools-extra.
compositeprimes added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

Part 2 of the change:
The goal is instead of dropping all the ranges associated with a Diagnostic 
when converting them to a ClangTidy error, instead attach them to the 
ClangTidyError, so they can be consumed by other APIs.

I could move this to the ClangTidyError class itself, but I thought it could be 
useful here, as Diagnostic could use a concept of ranges not associated with a 
FixIt


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68887

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp


Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -61,6 +61,7 @@
 }
 assert(Error.Message.Message.empty() && "Overwriting a diagnostic 
message");
 Error.Message = TidyMessage;
+Error.Ranges = Ranges;
   }
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,


Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -61,6 +61,7 @@
 }
 assert(Error.Message.Message.empty() && "Overwriting a diagnostic message");
 Error.Message = TidyMessage;
+Error.Ranges = Ranges;
   }
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68885: Add a Ranges field to Diagnostic struct

2019-10-11 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added reviewers: clang, clang-tools-extra.
compositeprimes added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Part 1 of the change:
The goal is instead of dropping all the ranges associated with a Diagnostic 
when converting them to a ClangTidy error, instead attach them to the 
ClangTidyError, so they can be consumed by other APIs.

I could move this to the ClangTidyError class itself, but I thought it could be 
useful here, as Diagnostic could use a concept of ranges not associated with a 
FixIt


Repository:
  rC Clang

https://reviews.llvm.org/D68885

Files:
  include/clang/Tooling/Core/Diagnostic.h


Index: include/clang/Tooling/Core/Diagnostic.h
===
--- include/clang/Tooling/Core/Diagnostic.h
+++ include/clang/Tooling/Core/Diagnostic.h
@@ -84,6 +84,13 @@
   ///
   /// Note: it is empty in unittest.
   std::string BuildDirectory;
+
+
+  /// Extra source ranges associated with the diagnostic. By default, the
+  /// diagnostic is only associated with the source location specified in the
+  /// `Message` field, but if `Ranges` is nonempty, they will be preferred
+  /// representation of the source code associated with the diagnostic.
+  ArrayRef Ranges;
 };
 
 /// Collection of Diagnostics generated from a single translation unit.


Index: include/clang/Tooling/Core/Diagnostic.h
===
--- include/clang/Tooling/Core/Diagnostic.h
+++ include/clang/Tooling/Core/Diagnostic.h
@@ -84,6 +84,13 @@
   ///
   /// Note: it is empty in unittest.
   std::string BuildDirectory;
+
+
+  /// Extra source ranges associated with the diagnostic. By default, the
+  /// diagnostic is only associated with the source location specified in the
+  /// `Message` field, but if `Ranges` is nonempty, they will be preferred
+  /// representation of the source code associated with the diagnostic.
+  ArrayRef Ranges;
 };
 
 /// Collection of Diagnostics generated from a single translation unit.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55545: Allow IncludeSorter to use #import for Objective-C files

2018-12-10 Thread Joe Turner via Phabricator via cfe-commits

compositeprimes created this revision.
compositeprimes added reviewers: klimek, alexfh.
Herald added a subscriber: cfe-commits.

The change infers whether a source file is Objective-C using LangOpts, and if 
it detects Objective-C, it uses "#import" instead of "#include" for all 
inserted imports.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55545

Files:
  clang-tidy/utils/IncludeSorter.cpp


Index: clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tidy/utils/IncludeSorter.cpp
+++ clang-tidy/utils/IncludeSorter.cpp
@@ -113,9 +113,10 @@
 
 Optional IncludeSorter::CreateIncludeInsertion(StringRef FileName,
   bool IsAngled) {
+  std::string IncludeDirective = LangOpts->ObjC ? "#import " : "#include ";
   std::string IncludeStmt =
-  IsAngled ? llvm::Twine("#include <" + FileName + ">\n").str()
-   : llvm::Twine("#include \"" + FileName + "\"\n").str();
+  IsAngled ? llvm::Twine(IncludeDirective + "<" + FileName + ">\n").str()
+   : llvm::Twine(IncludeDirective + "\"" + 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.


Index: clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tidy/utils/IncludeSorter.cpp
+++ clang-tidy/utils/IncludeSorter.cpp
@@ -113,9 +113,10 @@
 
 Optional IncludeSorter::CreateIncludeInsertion(StringRef FileName,
   bool IsAngled) {
+  std::string IncludeDirective = LangOpts->ObjC ? "#import " : "#include ";
   std::string IncludeStmt =
-  IsAngled ? llvm::Twine("#include <" + FileName + ">\n").str()
-   : llvm::Twine("#include \"" + FileName + "\"\n").str();
+  IsAngled ? llvm::Twine(IncludeDirective + "<" + FileName + ">\n").str()
+   : llvm::Twine(IncludeDirective + "\"" + 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.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D89276: Support ObjC in IncludeInserter

[PATCH] D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D68885: Add a Ranges field to Diagnostic struct

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[PATCH] D68887: Copy the Ranges field from the Diagnostic when creating the ClangTidyError

[PATCH] D68885: Add a Ranges field to Diagnostic struct

[PATCH] D55545: Allow IncludeSorter to use #import for Objective-C files

28 matches

Site Navigation

Mail list logo

Footer information