Reland "[clang-tidy] Adding RestrictSystemIncludes check to Fuchsia module"

This relands r332125 with a fixed test.

llvm-svn: 332141

clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt

@@ -5,6 +5,7 @@ add_clang_library(clangTidyFuchsiaModule
   FuchsiaTidyModule.cpp
   MultipleInheritanceCheck.cpp
   OverloadedOperatorCheck.cpp
+  RestrictSystemIncludesCheck.cpp
   StaticallyConstructedObjectsCheck.cpp
   TrailingReturnCheck.cpp
   VirtualInheritanceCheck.cpp

clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp

@@ -14,6 +14,7 @@
 #include "DefaultArgumentsCheck.h"
 #include "MultipleInheritanceCheck.h"
 #include "OverloadedOperatorCheck.h"
+#include "RestrictSystemIncludesCheck.h"
 #include "StaticallyConstructedObjectsCheck.h"
 #include "TrailingReturnCheck.h"
 #include "VirtualInheritanceCheck.h"
@@ -36,6 +37,8 @@ public:
         "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>(

clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp

@@ -0,0 +1,118 @@
+//===--- 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,
+                          SrcMgr::CharacteristicKind FileType) override;
+  void EndOfMainFile() override;
+
+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;
+
+  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,
+    SrcMgr::CharacteristicKind FileType) {
+  if (!Check.contains(FileName) && SrcMgr::isSystem(FileType)) {
+    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() {
+  for (const auto &Bucket : IncludeDirectives) {
+    const FileIncludes &FileDirectives = Bucket.second;
+
+    // Emit fixits for all restricted includes.
+    for (const auto &Include : FileDirectives) {
+      // 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) {
+  Compiler.getPreprocessor().addPPCallbacks(
+      llvm::make_unique<RestrictedIncludesPPCallbacks>(
+          *this, Compiler.getSourceManager()));
+}
+
+void RestrictSystemIncludesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Includes", AllowedIncludes);
+}
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h

@@ -0,0 +1,48 @@
+//===--- 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 "../ClangTidyDiagnosticConsumer.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(Options.get("Includes", "*")),
+        AllowedIncludesGlobList(AllowedIncludes) {}
+
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  bool contains(StringRef FileName) {
+    return AllowedIncludesGlobList.contains(FileName);
+  }
+
+private:
+  std::string AllowedIncludes;
+  GlobList AllowedIncludesGlobList;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -110,8 +110,13 @@ Improvements to clang-tidy
   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.
+
+- 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

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

@@ -0,0 +1,32 @@
+.. title:: clang-tidy - fuchsia-restrict-system-includes
+
+fuchsia-restrict-system-includes
+================================
+
+Checks for allowed system includes and suggests removal of any others.
+
+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.
+
+For example, given the allowed system includes 'a.h,b*':
+
+.. code-block:: c++
+
+  #include <a.h>
+  #include <b.h>
+  #include <bar.h>
+  #include <c.h>    // Warning, as c.h is not explicitly allowed
+  
+All system includes can be allowed with '*', and all can be disallowed with an
+empty string ('').
+  
+Options
+-------
+
+.. option:: Includes
+
+   A string containing a comma separated glob list of allowed include filenames.
+   Similar to the -checks glob list for running clang-tidy itself, the two
+   wildcard characters are '*' and '-', to include and exclude globs,
+   respectively.The default is '*', which allows all includes.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -95,6 +95,7 @@ Clang-Tidy Checks
    fuchsia-default-arguments
    fuchsia-multiple-inheritance
    fuchsia-overloaded-operator
+   fuchsia-restrict-system-includes
    fuchsia-statically-constructed-objects
    fuchsia-trailing-return
    fuchsia-virtual-inheritance

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

@@ -0,0 +1 @@
+void f() {}

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

@@ -0,0 +1,3 @@
+#include <r.h>
+#include <t.h>
+#include <s.h>

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

@@ -0,0 +1,2 @@
+#include <s.h>
+#include <t.h>

clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-all.cpp

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

clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-glob.cpp

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

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

@@ -0,0 +1,22 @@
+// RUN: mkdir %T/Inputs
+// RUN: cp -r %S/Inputs/fuchsia-restrict-system-includes %T/Inputs/fuchsia-restrict-system-includes
+// 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/fuchsia-restrict-system-includes/transitive2.h %s -check-prefix=CHECK-FIXES
+// RUN: rm -rf %T/Inputs
+
+// 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-FIXES-NOT: #include <t.h>
+
+int main() {
+  // f() is declared in r.h
+}

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

@@ -0,0 +1,25 @@
+// 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