[clang-tidy] IncludeInserter: allow <> in header name

This adds a pair of overloads for create(MainFile)?IncludeInsertion methods that
use the presence of the <> in the file name to control whether the #include
directive will use angle brackets or quotes. Motivating examples:
https://reviews.llvm.org/D82089#inline-789412 and
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp#L433

The overloads with the IsAngled parameter can be removed after the users are
updated.

Update usages of createIncludeInsertion.

Update (almost all) usages of createMainFileIncludeInsertion.

Reviewed By: hokein

Differential Revision: https://reviews.llvm.org/D85666

clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp

@@ -106,8 +106,8 @@ void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
   // whether this already exists).
   Diagnostic << IncludeInserter.createIncludeInsertion(
-      Source.getFileID(ComparisonExpr->getBeginLoc()), AbseilStringsMatchHeader,
-      false);
+      Source.getFileID(ComparisonExpr->getBeginLoc()),
+      AbseilStringsMatchHeader);
 }
 
 void StringFindStartswithCheck::registerPPCallbacks(

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

@@ -28,7 +28,7 @@ InitVariablesCheck::InitVariablesCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
                                                utils::IncludeSorter::IS_LLVM)),
-      MathHeader(Options.get("MathHeader", "math.h")) {}
+      MathHeader(Options.get("MathHeader", "<math.h>")) {}
 
 void InitVariablesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
@@ -103,7 +103,7 @@ void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
                InitializationString);
     if (AddMathInclude) {
       Diagnostic << IncludeInserter.createIncludeInsertion(
-          Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false);
+          Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader);
     }
   }
 }

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

@@ -85,8 +85,7 @@ void ProBoundsConstantArrayIndexCheck::check(
                               IndexRange.getBegin().getLocWithOffset(-1)),
                   ", ")
            << FixItHint::CreateReplacement(Matched->getEndLoc(), ")")
-           << Inserter.createMainFileIncludeInsertion(GslHeader,
-                                                      /*IsAngled=*/false);
+           << Inserter.createMainFileIncludeInsertion(GslHeader);
     }
     return;
   }

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp

@@ -19,7 +19,6 @@ namespace modernize {
 
 namespace {
 
-constexpr char StdMemoryHeader[] = "memory";
 constexpr char ConstructorCall[] = "constructorCall";
 constexpr char ResetCall[] = "resetCall";
 constexpr char NewExpression[] = "newExpression";
@@ -47,7 +46,7 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
       Inserter(Options.getLocalOrGlobal("IncludeStyle",
                                         utils::IncludeSorter::IS_LLVM)),
       MakeSmartPtrFunctionHeader(
-          Options.get("MakeSmartPtrFunctionHeader", StdMemoryHeader)),
+          Options.get("MakeSmartPtrFunctionHeader", "<memory>")),
       MakeSmartPtrFunctionName(
           Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
       IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -430,9 +429,7 @@ void MakeSmartPtrCheck::insertHeader(DiagnosticBuilder &Diag, FileID FD) {
   if (MakeSmartPtrFunctionHeader.empty()) {
     return;
   }
-  Diag << Inserter.createIncludeInsertion(
-      FD, MakeSmartPtrFunctionHeader,
-      /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader);
+  Diag << Inserter.createIncludeInsertion(FD, MakeSmartPtrFunctionHeader);
 }
 
 } // namespace modernize

clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp

@@ -216,8 +216,7 @@ void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
               Initializer->getLParenLoc().getLocWithOffset(1), "std::move(")
        << Inserter.createIncludeInsertion(
               Result.SourceManager->getFileID(Initializer->getSourceLocation()),
-              "utility",
-              /*IsAngled=*/true);
+              "<utility>");
 }
 
 } // namespace modernize

clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp

@@ -147,8 +147,7 @@ void ReplaceAutoPtrCheck::check(const MatchFinder::MatchResult &Result) {
     auto Diag = diag(Range.getBegin(), "use std::move to transfer ownership")
                 << FixItHint::CreateInsertion(Range.getBegin(), "std::move(")
                 << FixItHint::CreateInsertion(Range.getEnd(), ")")
-                << Inserter.createMainFileIncludeInsertion("utility",
-                                                           /*IsAngled=*/true);
+                << Inserter.createMainFileIncludeInsertion("<utility>");
 
     return;
   }

clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp

@@ -94,7 +94,7 @@ void ReplaceRandomShuffleCheck::check(const MatchFinder::MatchResult &Result) {
   Diag << IncludeInserter.createIncludeInsertion(
       Result.Context->getSourceManager().getFileID(
           MatchedCallExpr->getBeginLoc()),
-      "random", /*IsAngled=*/true);
+      "<random>");
 }
 
 } // namespace modernize

clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp

@@ -192,7 +192,7 @@ void TypePromotionInMathFnCheck::check(const MatchFinder::MatchResult &Result) {
   if (FnInCmath)
     Diag << IncludeInserter.createIncludeInsertion(
         Result.Context->getSourceManager().getFileID(Call->getBeginLoc()),
-        "cmath", /*IsAngled=*/true);
+        "<cmath>");
 }
 
 } // namespace performance

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

@@ -203,8 +203,7 @@ void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
   Diag << FixItHint::CreateInsertion(CopyArgument.getBeginLoc(), "std::move(")
        << FixItHint::CreateInsertion(EndLoc, ")")
        << Inserter.createIncludeInsertion(
-              SM.getFileID(CopyArgument.getBeginLoc()), "utility",
-              /*IsAngled=*/true);
+              SM.getFileID(CopyArgument.getBeginLoc()), "<utility>");
 }
 
 } // namespace performance

clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp

@@ -77,6 +77,14 @@ IncludeInserter::createIncludeInsertion(FileID FileID, StringRef Header,
   return getOrCreate(FileID).CreateIncludeInsertion(Header, IsAngled);
 }
 
+llvm::Optional<FixItHint>
+IncludeInserter::createIncludeInsertion(FileID FileID, llvm::StringRef Header) {
+  bool IsAngled = Header.consume_front("<");
+  if (IsAngled != Header.consume_back(">"))
+    return llvm::None;
+  return createIncludeInsertion(FileID, Header, IsAngled);
+}
+
 llvm::Optional<FixItHint>
 IncludeInserter::createMainFileIncludeInsertion(StringRef Header,
                                                 bool IsAngled) {
@@ -85,6 +93,13 @@ IncludeInserter::createMainFileIncludeInsertion(StringRef Header,
   return createIncludeInsertion(SourceMgr->getMainFileID(), Header, IsAngled);
 }
 
+llvm::Optional<FixItHint>
+IncludeInserter::createMainFileIncludeInsertion(StringRef Header) {
+  assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call "
+                      "registerPreprocessor()?");
+  return createIncludeInsertion(SourceMgr->getMainFileID(), Header);
+}
+
 void IncludeInserter::addInclude(StringRef FileName, bool IsAngled,
                                  SourceLocation HashLocation,
                                  SourceLocation EndLocation) {

clang-tools-extra/clang-tidy/utils/IncludeInserter.h

@@ -69,15 +69,35 @@ class IncludeInserter {
   /// Creates a \p Header inclusion directive fixit in the File \p FileID.
   /// Returns ``llvm::None`` on error or if the inclusion directive already
   /// exists.
+  /// FIXME: This should be removed once the clients are migrated to the
+  /// overload without the ``IsAngled`` parameter.
   llvm::Optional<FixItHint>
   createIncludeInsertion(FileID FileID, llvm::StringRef Header, bool IsAngled);
 
+  /// Creates a \p Header inclusion directive fixit in the File \p FileID.
+  /// When \p Header is enclosed in angle brackets, uses angle brackets in the
+  /// inclusion directive, otherwise uses quotes.
+  /// Returns ``llvm::None`` on error or if the inclusion directive already
+  /// exists.
+  llvm::Optional<FixItHint> createIncludeInsertion(FileID FileID,
+                                                   llvm::StringRef Header);
+
   /// Creates a \p Header inclusion directive fixit in the main file.
   /// Returns``llvm::None`` on error or if the inclusion directive already
   /// exists.
+  /// FIXME: This should be removed once the clients are migrated to the
+  /// overload without the ``IsAngled`` parameter.
   llvm::Optional<FixItHint>
   createMainFileIncludeInsertion(llvm::StringRef Header, bool IsAngled);
 
+  /// Creates a \p Header inclusion directive fixit in the main file.
+  /// When \p Header is enclosed in angle brackets, uses angle brackets in the
+  /// inclusion directive, otherwise uses quotes.
+  /// Returns``llvm::None`` on error or if the inclusion directive already
+  /// exists.
+  llvm::Optional<FixItHint>
+  createMainFileIncludeInsertion(llvm::StringRef Header);
+
   IncludeSorter::IncludeStyle getStyle() const { return Style; }
 
 private:

clang-tools-extra/docs/ReleaseNotes.rst

@@ -67,6 +67,12 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- Checks that allow configuring names of headers to include now support wrapping
+  the include in angle brackets to create a system include. For example,
+  :doc:`cppcoreguidelines-init-variables
+  <clang-tidy/checks/cppcoreguidelines-init-variables>` and
+  :doc:`modernize-make-unique <clang-tidy/checks/modernize-make-unique>`.
+
 New modules
 ^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst

@@ -3,13 +3,13 @@
 cppcoreguidelines-init-variables
 ================================
 
-Checks whether there are local variables that are declared without an
-initial value. These may lead to unexpected behaviour if there is a
-code path that reads the variable before assigning to it.
+Checks whether there are local variables that are declared without an initial
+value. These may lead to unexpected behaviour if there is a code path that reads
+the variable before assigning to it.
 
-Only integers, booleans, floats, doubles and pointers are checked. The
-fix option initializes all detected values with the value of zero. An
-exception is float and double types, which are initialized to NaN.
+Only integers, booleans, floats, doubles and pointers are checked. The fix
+option initializes all detected values with the value of zero. An exception is
+float and double types, which are initialized to NaN.
 
 As an example a function that looks like this:
 
@@ -42,10 +42,10 @@ Options
 
 .. option:: IncludeStyle
 
-   A string specifying which include-style is used, `llvm` or
-   `google`. Default is `llvm`.
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
 
 .. option:: MathHeader
 
-   A string specifying the header to include to get the definition of
-   `NAN`. Default is `math.h`.
+   A string specifying the header to include to get the definition of `NAN`.
+   Default is `<math.h>`.

clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst

@@ -37,7 +37,7 @@ Options
 .. option:: MakeSmartPtrFunctionHeader
 
    A string specifying the corresponding header of make-unique-ptr function.
-   Default is `memory`.
+   Default is `<memory>`.
 
 .. option:: IncludeStyle
 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables.cpp

@@ -1,4 +1,5 @@
 // RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// CHECK-FIXES: {{^}}#include <math.h>
 
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);

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

@@ -43,14 +43,12 @@ class IncludeInserterCheckBase : public ClangTidyCheck {
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
     auto Diag = diag(Result.Nodes.getNodeAs<DeclStmt>("stmt")->getBeginLoc(),
                      "foo, bar");
-    for (StringRef Header : HeadersToInclude()) {
-      Diag << Inserter.createMainFileIncludeInsertion(Header,
-                                                      IsAngledInclude());
+    for (StringRef Header : headersToInclude()) {
+      Diag << Inserter.createMainFileIncludeInsertion(Header);
     }
   }
 
-  virtual std::vector<StringRef> HeadersToInclude() const = 0;
-  virtual bool IsAngledInclude() const = 0;
+  virtual std::vector<StringRef> headersToInclude() const = 0;
 
   utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
 };
@@ -60,52 +58,57 @@ class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
   NonSystemHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
-  std::vector<StringRef> HeadersToInclude() const override {
+  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 {
+  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 {
+  std::vector<StringRef> headersToInclude() const override {
     return {"path/to/header.h", "path/to/header2.h", "path/to/header.h"};
   }
-  bool IsAngledInclude() const override { return false; }
 };
 
 class CSystemIncludeInserterCheck : public IncludeInserterCheckBase {
 public:
   CSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
-  std::vector<StringRef> HeadersToInclude() const override {
-    return {"stdlib.h"};
+  std::vector<StringRef> headersToInclude() const override {
+    return {"<stdlib.h>"};
   }
-  bool IsAngledInclude() const override { return true; }
 };
 
 class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
 public:
   CXXSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
-  std::vector<StringRef> HeadersToInclude() const override { return {"set"}; }
-  bool IsAngledInclude() const override { return true; }
+  std::vector<StringRef> headersToInclude() const override { return {"<set>"}; }
+};
+
+class InvalidIncludeInserterCheck : public IncludeInserterCheckBase {
+public:
+  InvalidIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context)
+      : IncludeInserterCheckBase(CheckName, Context) {}
+
+  std::vector<StringRef> headersToInclude() const override {
+    return {"a.h", "<stdlib.h", "cstdlib>", "b.h", "<c.h>", "<d>"};
+  }
 };
 
 template <typename Check>
@@ -598,6 +601,40 @@ void foo() {
                                    "insert_includes_test_header.cc"));
 }
 
+TEST(IncludeInserterTest, InvalidHeaderName) {
+  const char *PreCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <list>
+#include <map>
+
+#include "path/to/a/header.h"
+
+void foo() {
+  int a = 0;
+})";
+  const char *PostCode = R"(
+#include "clang_tidy/tests/insert_includes_test_header.h"
+
+#include <c.h>
+
+#include <d>
+#include <list>
+#include <map>
+
+#include "a.h"
+#include "b.h"
+#include "path/to/a/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode<InvalidIncludeInserterCheck>(
+                          PreCode, "clang_tidy/tests/"
+                                   "insert_includes_test_header.cc"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang