[clang-tidy] Adding RestrictIncludes check to Fuchsia module

[juliehockett]

Summary

Adding a check to restrict system includes to a whitelist. Given a list of includes that are explicitly allowed, the check issues a fixit to remove any system include not on that list from the source file.

[Eugene.Zelenko]

clang-tidy/fuchsia/CMakeLists.txt

8

Will be good idea to be have consistent name and documentation terminology: include versus header.

clang-tidy/fuchsia/RestrictIncludesCheck.cpp

48 ↗

(On Diff #135950)

Please use using and run Clang-tidy modernize.

70 ↗

(On Diff #135950)

Please don't use auto here, since type is not obvious.

80 ↗

(On Diff #135950)

Please include <cstring>.

81 ↗

(On Diff #135950)

Please don't use auto here, since type is not obvious.

101 ↗

(On Diff #135950)

I don't think that you should prefix llvm with ::

clang-tidy/fuchsia/RestrictIncludesCheck.h

25 ↗

(On Diff #135950)

Please run Clang-format over code.

30 ↗

(On Diff #135950)

Please separate constructor with empty line from rest of methods.

[hokein]

docs/clang-tidy/checks/fuchsia-restrict-includes.rst

27 ↗

(On Diff #135950)

The check seems do nothing with the default option.

Do we have a corresponding guideline of fuchsia?

[aaron.ballman]

clang-tidy/fuchsia/RestrictIncludesCheck.cpp

37–39 ↗

(On Diff #135950)

Please don't suffix with an underscore -- you can actually drop the underscore here (we allow constructor arguments to hide the names of class members).

48–49 ↗

(On Diff #135950)

Are you sure that vector and map are the best data types to use, rather than LLVM ADTs?

Also you can drop the elaborated type specifier for FileID.

69 ↗

(On Diff #135950)

const auto &

74 ↗

(On Diff #135950)

This comment doesn't really add much value.

75 ↗

(On Diff #135950)

Diagnostics are not complete sentences, so you should remove the punctuation and capitalization here.

clang-tidy/fuchsia/RestrictIncludesCheck.h

32 ↗

(On Diff #135950)

Function can be const and return a const ref.

test/clang-tidy/fuchsia-restrict-includes.cpp

13 ↗

(On Diff #135950)

I think this could use some edge case tests for awful things people can do:

#define foo <stdio.h>

#include foo

# /* hahaha */ include /* oops */ foo

[juliehockett]

Fixing comments and updating tests.

[aaron.ballman]

clang-tidy/fuchsia/RestrictIncludesCheck.cpp

42–44 ↗

(On Diff #136096)

We don't really use Doxygen comments for local class stuff, so you can convert these into regular comments.

75 ↗

(On Diff #136096)

I believe this check will be finding transitive includes, correct? If so, maybe this should be two diagnostics: one to say "'%0' is prohibited from being included" and a note for the transitive case to say "file included transitively from here"?

Regardless of the behavior you'd like for it, we should have a test case:

// a.h
// Assumed that a.h is prohibited
void f();

// b.h
#include "a.h"

// c.c
#include "b.h"

int main() {
  f();
}

83 ↗

(On Diff #136096)

Are you okay with breaking user's code from this removal? If you remove the header file, but not the parts of the code that require that header file to be included, this FixIt will break code.

87 ↗

(On Diff #136096)

Do you need to clear this? I don't think EndOfMainFile() can be called twice, can it?

92–95 ↗

(On Diff #136096)

This can be replaced with: return llvm::is_contained(Check.getRestrictedIncludes(), IncludeFile); which suggests the private function isn't needed at all.

clang-tidy/fuchsia/RestrictIncludesCheck.h

20 ↗

(On Diff #136096)

s/headers/includes

28 ↗

(On Diff #136096)

There is a mild problem here (present in the previous form, using commas, as well): the separator is technically a legal character for a file name. You may want to provide a comment in the documentation that explicitly states a file with the delimiter in the name is not supported. e.g., a file named foo.h;bar.h is not something you can prohibit with this check. I don't think it's worth picking a different delimiter over given how unlikely this file name is in practice.

29 ↗

(On Diff #136096)

The default value here seems very strange as it's not an include file. I assume this intended to use vector?

docs/clang-tidy/checks/fuchsia-restrict-includes.rst

28 ↗

(On Diff #136096)

The list is not comma-separated, it's semicolon-separated.

[juliehockett]

Sorry for the delay in updating this -- check now restricts includes to a whitelist, rather than a blacklist, and only applies to system headers, to more strictly control which are allowed.

Also updated warning text and addressed comments.

clang-tidy/fuchsia/RestrictIncludesCheck.cpp

75 ↗

(On Diff #136096)

It will flag, but not fix, if a disallowed file is transitively included *and* the relevant flag is passed (e.g. -system-headers for system, or -header-filter=.* for other. I changed the warning text for header warnings to clarify that.

83 ↗

(On Diff #136096)

Yes, but I did add a note to the documentation to call that out.

docs/clang-tidy/checks/fuchsia-restrict-includes.rst

27 ↗

(On Diff #135950)

The idea is to maintain a list of disallowed headers in the local .clang-tidy file, so it will be dependent on that. If no headers are restricted, the check won't do anything. I'll add documentation to that effect.

[aaron.ballman]

clang-tidy/fuchsia/FuchsiaTidyModule.cpp

41

I think this should be named fuchsia-restrict-system-includes.

clang-tidy/fuchsia/RestrictIncludesCheck.cpp

60 ↗

(On Diff #145547)

I'm not certain that IsAngled is sufficient. For instance, say the user prohibits use of vector, nothing prevents the user from writing #include "vector" to access the system header (assuming there is no user header with the same name). e.g., https://godbolt.org/g/Scfv3U

Perhaps we could use SourceManager::isInSystemHeader() or extract some logic from it to test whether the actual included file is in the system header search path?

docs/ReleaseNotes.rst

116

Be explicit that this only impacts system headers.

docs/clang-tidy/checks/fuchsia-restrict-includes.rst

6–7 ↗

(On Diff #145547)

Should also be explicit about system headers.

test/clang-tidy/Inputs/fuchsia-restrict-includes/system/r.h

2 ↗

(On Diff #145547)

Please add in the EOF.

test/clang-tidy/Inputs/fuchsia-restrict-includes/system/transitive.h

2 ↗

(On Diff #145547)

Please add in the EOF.

test/clang-tidy/fuchsia-restrict-includes.cpp

31 ↗

(On Diff #145547)

Please add in the EOF.

[juliehockett]

Made the check for system headers more comprehensive & fixed newline issues

[Eugene.Zelenko]

docs/ReleaseNotes.rst

116

Is it necessary to highlight that warnings will not be emitted in case of disallowed headers are not found? Same in documentation.

[aaron.ballman]

clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp

67

Rather than use this StringMap, can you change InclusionDirective() to filter out includes you don't care about? Something along the lines of:

FileID FID = SM.translateFile(File);
assert(FID != FileID() && "Source Manager does not know about this header file");

bool Invalid;
const SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
if (!Invalid && SrcMgr::isSystem(Entry.getFile().getFileCharacteristic())) {
  // It's a system file.
}

This way you don't have to store all the path information and test against paths in EndOfMainFile(), unless I'm missing something. Note, this is untested code and perhaps that assert will fire (maybe SourceManager is unaware of this file by the time InclusionDirective() is called).

Alternatively, you could alter InclusionDirective() to also pass in the CharacteristicKind calculated by Preprocessor::HandleIncludeDirective() as that seems like generally useful information for the callback to have on hand.

clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

1–2

This should only be on a single line -- you can remove some - characters.

docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst

32

This default seems a bit odd to me, but perhaps it's fine. What's novel is that the check is a no-op by default, so how do Fuchsia developers get the correct list? Or is there no canonical list and developers are expected to populate their own manually?

[juliehockett]

Updating the inclusiondirective to filter out non-system files

docs/ReleaseNotes.rst

116

I'm not sure I understand what you're saying...are you saying if the documentation should not include anything about what happens in the case of no headers?

docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst

32

The idea is that it's a case-by-case basis -- this may change at some point in the future if we decide there's a standard whitelist of system includes across the board, but at the moment the thought is to allow everything in some places, and use this check to limit them in others. It'll need to be populated on a case-by-case basis, since different directories will have different requirements.

[aaron.ballman]

docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst

32

So there's never a case where you need to prohibit use of all system includes? Right now, an empty string means "allow all includes" while a non-empty string means "allow only these includes", so there's no way to prohibit all includes except by listing them manually. I'm wondering whether you want to use a glob for allowed file names, which gives you extra flexibility. e.g., * allows all includes (default value), foo.h allows only <foo.h>, experimental/* allows all headers in the experimental directory, cstd* allows <cstdint> while prohibiting <stdint.h>, and the empty string will disallow all system headers.

Perhaps you don't need that level of flexibility, but for the use cases I'm imagining it seems more user friendly to be able to write cstd* rather than manually listing out all the C++ headers explicitly.

[juliehockett]

Sorry, branches got crossed. Reverted and reopened.

[juliehockett]

Updating parameter list to be a glob-style list.

[aaron.ballman]

Thank you for adding the glob feature, I think that will be very powerful in practice. Aside from a minor nit, this LGTM!

clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

36

Function can be marked const.

[juliehockett]

clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

36

GlobList::contains isn't const, so it can't...

[aaron.ballman]

clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

36

Bah, I didn't see that. You can ignore, sorry for the noise.

clang-tidy/fuchsia/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyFuchsiaModule
   DefaultArgumentsCheck.cpp
   FuchsiaTidyModule.cpp
   MultipleInheritanceCheck.cpp
   OverloadedOperatorCheck.cpp
+  RestrictSystemIncludesCheck.cpp
   StaticallyConstructedObjectsCheck.cpp
   TrailingReturnCheck.cpp
   VirtualInheritanceCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyGoogleModule
   clangTidyUtils
   )

clang-tidy/fuchsia/FuchsiaTidyModule.cpp

 //===--- FuchsiaTidyModule.cpp - clang-tidy--------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../google/UnnamedNamespaceInHeaderCheck.h"
 #include "DefaultArgumentsCheck.h"
 #include "MultipleInheritanceCheck.h"
 #include "OverloadedOperatorCheck.h"
+#include "RestrictSystemIncludesCheck.h"
 #include "StaticallyConstructedObjectsCheck.h"
 #include "TrailingReturnCheck.h"
 #include "VirtualInheritanceCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace fuchsia {
 
 /// This module is for Fuchsia-specific checks.
 class FuchsiaModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<DefaultArgumentsCheck>(
         "fuchsia-default-arguments");
     CheckFactories.registerCheck<google::build::UnnamedNamespaceInHeaderCheck>(
         "fuchsia-header-anon-namespaces");
     CheckFactories.registerCheck<MultipleInheritanceCheck>(
         "fuchsia-multiple-inheritance");
     CheckFactories.registerCheck<OverloadedOperatorCheck>(
         "fuchsia-overloaded-operator");
+    CheckFactories.registerCheck<RestrictSystemIncludesCheck>(
+        "fuchsia-restrict-system-includes");
     CheckFactories.registerCheck<StaticallyConstructedObjectsCheck>(
         "fuchsia-statically-constructed-objects");
     CheckFactories.registerCheck<TrailingReturnCheck>(
         "fuchsia-trailing-return");
     CheckFactories.registerCheck<VirtualInheritanceCheck>(
         "fuchsia-virtual-inheritance");
   }
 };

clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

+//===--- RestrictSystemIncludesCheck.h - clang-tidy---------------------*-
+//C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/OptionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Checks for allowed includes and suggests removal of any others. If no
+/// includes are specified, the check will exit without issuing any warnings.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-restrict-system-includes.html
+class RestrictSystemIncludesCheck : public ClangTidyCheck {
+public:
+  RestrictSystemIncludesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        AllowedIncludes(
+            utils::options::parseStringList(Options.get("Includes", ""))) {}
+
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  const std::vector<std::string> &getAllowedIncludes() const {
+    return AllowedIncludes;
+  }
+
+private:
+  std::vector<std::string> AllowedIncludes;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H

clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp

+//===--- RestrictSystemIncludesCheck.cpp - clang-tidy----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "RestrictSystemIncludesCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Path.h"
+#include <cstring>
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+class RestrictedIncludesPPCallbacks : public PPCallbacks {
+public:
+  explicit RestrictedIncludesPPCallbacks(RestrictSystemIncludesCheck &Check,
+                                         SourceManager &SM)
+      : Check(Check), SM(SM) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+                          StringRef FileName, bool IsAngled,
+                          CharSourceRange FilenameRange, const FileEntry *File,
+                          StringRef SearchPath, StringRef RelativePath,
+                          const Module *Imported) override;
+  void EndOfMainFile() override;
+
+  void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+                   SrcMgr::CharacteristicKind FileType,
+                   FileID PrevFID) override {
+    // Record all files we enter, tracking which are system.
+    if (Reason == EnterFile && SrcMgr::isSystem(FileType)) {
+      if (const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc))) {
+        SmallString<256> Result = FE->getName();
+        llvm::sys::path::remove_dots(Result, true);
+        IsSystem[Result.str()] = true;
+      }
+    }
+  }
+
+private:
+  struct IncludeDirective {
+    IncludeDirective() = default;
+    IncludeDirective(SourceLocation Loc, CharSourceRange Range,
+                     StringRef Filename, StringRef FullPath, bool IsInMainFile)
+        : Loc(Loc), Range(Range), IncludeFile(Filename), IncludePath(FullPath),
+          IsInMainFile(IsInMainFile) {}
+
+    SourceLocation Loc;      // '#' location in the include directive
+    CharSourceRange Range;   // SourceRange for the file name
+    std::string IncludeFile; // Filename as a string
+    std::string IncludePath; // Full file path as a string
+    bool IsInMainFile;       // Whether or not the include is in the main file
+  };
+
+  using FileIncludes = llvm::SmallVector<IncludeDirective, 8>;
+  llvm::SmallDenseMap<FileID, FileIncludes> IncludeDirectives;
+  llvm::StringMap<bool> IsSystem;
+
+  RestrictSystemIncludesCheck &Check;
+  SourceManager &SM;
+};
+
+void RestrictedIncludesPPCallbacks::InclusionDirective(
+    SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
+    bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (!llvm::is_contained(Check.getAllowedIncludes(), FileName)) {
+    SmallString<256> FullPath;
+    llvm::sys::path::append(FullPath, SearchPath);
+    llvm::sys::path::append(FullPath, RelativePath);
+    // Bucket the allowed include directives by the id of the file they were
+    // declared in.
+    IncludeDirectives[SM.getFileID(HashLoc)].emplace_back(
+        HashLoc, FilenameRange, FileName, FullPath.str(),
+        SM.isInMainFile(HashLoc));
+  }
+}
+
+void RestrictedIncludesPPCallbacks::EndOfMainFile() {
+  if (IncludeDirectives.empty())
+    return;
+
+  for (const auto &Bucket : IncludeDirectives) {
+    const FileIncludes &FileDirectives = Bucket.second;
+
+    // Emit fixits for all restricted includes.
+    for (const auto &Include : FileDirectives) {
+      // Skip anything that isn't system.
+      if (!IsSystem.count(Include.IncludePath))
+        continue;
+
+      // Fetch the length of the include statement from the start to just after
+      // the newline, for finding the end (including the newline).
+      unsigned ToLen = std::strcspn(SM.getCharacterData(Include.Loc), "\n") + 1;
+      CharSourceRange ToRange = CharSourceRange::getCharRange(
+          Include.Loc, Include.Loc.getLocWithOffset(ToLen));
+
+      if (!Include.IsInMainFile) {
+        auto D = Check.diag(
+            Include.Loc,
+            "system include %0 not allowed, transitively included from %1");
+        D << Include.IncludeFile << SM.getFilename(Include.Loc);
+        D << FixItHint::CreateRemoval(ToRange);
+        continue;
+      }
+      auto D = Check.diag(Include.Loc, "system include %0 not allowed");
+      D << Include.IncludeFile;
+      D << FixItHint::CreateRemoval(ToRange);
+    }
+  }
+}
+
+void RestrictSystemIncludesCheck::registerPPCallbacks(
+    CompilerInstance &Compiler) {
+  // Do nothing if there are no restricted includes.
+  if (AllowedIncludes.empty())
+    return;
+  Compiler.getPreprocessor().addPPCallbacks(
+      llvm::make_unique<RestrictedIncludesPPCallbacks>(
+          *this, Compiler.getSourceManager()));
+}
+
+void RestrictSystemIncludesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Includes",
+                utils::options::serializeStringList(AllowedIncludes));
+}
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

- New :doc:`fuchsia-multiple-inheritance
   Warns if a class inherits from multiple classes that are not pure virtual.
 
 - New :doc:`abseil-string-find-startswith
   <clang-tidy/checks/abseil-string-find-startswith>` check.
 
   Checks whether a ``std::string::find()`` result is compared with 0, and
   suggests replacing with ``absl::StartsWith()``.
 
-- New :doc:`fuchsia-statically-constructed-objects
-  <clang-tidy/checks/fuchsia-statically-constructed-objects>` check.
+- New `fuchsia-restrict-system-includes
+  <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-restrict-system-includes.html>`_ check
+
+  Checks for allowed system includes and suggests removal of any others. If no
+  includes are specified, the check will exit without issuing any warnings.
+
+- New `fuchsia-statically-constructed-objects
+  <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-statically-constructed-objects.html>`_ check
 
   Warns if global, non-trivial objects with static storage are constructed,
   unless the object is statically initialized with a ``constexpr`` constructor
   or has no explicit constructor.
 
 - New :doc:`fuchsia-trailing-return
   <clang-tidy/checks/fuchsia-trailing-return>` check.
 

docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst

+.. title:: clang-tidy - fuchsia-restrict-system-includes
+
+fuchsia-restrict-system-includes
+================================
+
+Checks for allowed system includes and suggests removal of any others. If no
+includes are specified, the check will exit without issuing any warnings. 
+
+It is important to note that running this check with fixes may break code, as
+the fix removes headers. Fixes are applied to source and header files, but not
+to system headers.
+
+Note that the separator for identifying allowed includes is a semi-colon, and
+therefore this check is unable to allow an include with a semi-colon in the
+filename (e.g. 'foo;bar.h' will be parsed as allowing 'foo' and 'bar.h', and not
+as allowing a file called 'foo;bar.h').
+
+For example, given the allowed system includes 'a.h; b.h':
+
+.. code-block:: c++
+
+  #include <a.h>
+  #include <b.h>
+  #include <c.h>    // Warning, as c.h is not explicitly allowed
+  
+Options
+-------
+
+.. option:: Includes
+
+   A string containing a semi-colon separated list of allowed include filenames.
+   The default is an empty string, which allows all includes.

docs/clang-tidy/checks/list.rst

.. toctree::
    cppcoreguidelines-pro-type-union-access
    cppcoreguidelines-pro-type-vararg
    cppcoreguidelines-slicing
    cppcoreguidelines-special-member-functions
    fuchsia-header-anon-namespaces (redirects to google-build-namespaces) <fuchsia-header-anon-namespaces>
    fuchsia-default-arguments
    fuchsia-multiple-inheritance
    fuchsia-overloaded-operator
+   fuchsia-restrict-system-includes
    fuchsia-statically-constructed-objects
    fuchsia-trailing-return
    fuchsia-virtual-inheritance
    google-build-explicit-make-pair
    google-build-namespaces
    google-build-using-namespace
    google-default-arguments
    google-explicit-constructor

test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h

+void f() {}

test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h

+#include <r.h>
+#include <t.h>
+#include <s.h>

test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h

+#include <s.h>
+#include <t.h>

test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp

+// RUN: cp -r %S/Inputs/fuchsia-restrict-system-includes %T/Inputs
+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN:   -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 'transitive.h;s.h'}]}" \
+// RUN:   -system-headers -header-filter=.* \
+// RUN:   -- -std=c++11 -I %T/Inputs/fuchsia-restrict-system-includes -isystem %T/Inputs/fuchsia-restrict-system-includes/system
+// RUN: FileCheck -input-file=%T/Inputs/transitive2.h %s -check-prefix=CHECK-HEADER-FIXES
+
+// transitive.h includes <r.h> and <t.h>
+#include <transitive.h>
+// CHECK-MESSAGES: :1:1: warning: system include r.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+
+// transitive.h includes <s.h> and <t.h>
+#include "transitive2.h"
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/transitive2.h
+// CHECK-HEADER-FIXES-NOT: #include <t.h>
+
+int main() {
+  // f() is declared in r.h
+}

test/clang-tidy/fuchsia-restrict-system-includes.cpp

+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN:   -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 's.h'}]}" \
+// RUN:   -- -std=c++11 -I %S/Inputs/fuchsia-restrict-system-includes -isystem %S/Inputs/fuchsia-restrict-system-includes/system
+
+#include "a.h"
+
+#include <s.h>
+#include <t.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include <t.h>
+
+#include "s.h"
+#include "t.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include "t.h"
+
+#define foo <j.h>
+
+#include foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: #include foo
+
+#/* comment */ include /* comment */ foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: # /* comment */ include /* comment */ foo