From 06af01bbc23d422df3bbac82752b935fa217893d Mon Sep 17 00:00:00 2001 From: Julie Hockett Date: Wed, 9 May 2018 22:25:42 +0000 Subject: [PATCH] [clang-tidy] Adding RestrictSystemIncludes check to Fuchsia module 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. Differential Revision: https://reviews.llvm.org/D43778 llvm-svn: 331930 --- .../clang-tidy/fuchsia/CMakeLists.txt | 1 + .../clang-tidy/fuchsia/FuchsiaTidyModule.cpp | 3 + .../fuchsia/RestrictSystemIncludesCheck.cpp | 126 +++++++++++++++++++++ .../fuchsia/RestrictSystemIncludesCheck.h | 46 ++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 10 +- .../checks/fuchsia-restrict-system-includes.rst | 32 ++++++ clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + .../Inputs/fuchsia-restrict-system-includes/a.h | 0 .../fuchsia-restrict-system-includes/system/j.h | 0 .../fuchsia-restrict-system-includes/system/r.h | 1 + .../fuchsia-restrict-system-includes/system/s.h | 0 .../fuchsia-restrict-system-includes/system/t.h | 0 .../system/transitive.h | 3 + .../fuchsia-restrict-system-includes/transitive2.h | 2 + .../fuchsia-restrict-system-includes-headers.cpp | 20 ++++ .../fuchsia-restrict-system-includes.cpp | 25 ++++ 16 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/a.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/j.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/s.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/t.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h create mode 100644 clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h create mode 100644 clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp create mode 100644 clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes.cpp diff --git a/clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt b/clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt index c2ed252..b69a5c0 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp b/clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp index 67b3bc9..0d3bbfe 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp +++ b/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( "fuchsia-overloaded-operator"); + CheckFactories.registerCheck( + "fuchsia-restrict-system-includes"); CheckFactories.registerCheck( "fuchsia-statically-constructed-objects"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp new file mode 100644 index 0000000..ee00cb2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp @@ -0,0 +1,126 @@ +//===--- 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 + +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; + llvm::SmallDenseMap 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 (!llvm::is_contained(Check.getAllowedIncludes(), 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() { + 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) { + // 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( + *this, Compiler.getSourceManager())); +} + +void RestrictSystemIncludesCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Includes", + utils::options::serializeStringList(AllowedIncludes)); +} + +} // namespace fuchsia +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h b/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h new file mode 100644 index 0000000..07b47cc --- /dev/null +++ b/clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h @@ -0,0 +1,46 @@ +//===--- 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 &getAllowedIncludes() const { + return AllowedIncludes; + } + +private: + std::vector AllowedIncludes; +}; + +} // namespace fuchsia +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d005a2a..c678947 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -110,8 +110,14 @@ 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 - ` check. +- New `fuchsia-restrict-system-includes + `_ 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 + `_ check Warns if global, non-trivial objects with static storage are constructed, unless the object is statically initialized with a ``constexpr`` constructor diff --git a/clang-tools-extra/docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst b/clang-tools-extra/docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst new file mode 100644 index 0000000..f2615ea --- /dev/null +++ b/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. 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 + #include + #include // 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. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index add8bdb..4ce3779 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/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 diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/a.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/a.h new file mode 100644 index 0000000..e69de29 diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/j.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/j.h new file mode 100644 index 0000000..e69de29 diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h new file mode 100644 index 0000000..56757a7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h @@ -0,0 +1 @@ +void f() {} diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/s.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/s.h new file mode 100644 index 0000000..e69de29 diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/t.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/t.h new file mode 100644 index 0000000..e69de29 diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h new file mode 100644 index 0000000..6dba769 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h @@ -0,0 +1,3 @@ +#include +#include +#include diff --git a/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h new file mode 100644 index 0000000..ea56788 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h @@ -0,0 +1,2 @@ +#include +#include diff --git a/clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp b/clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp new file mode 100644 index 0000000..895a347 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp @@ -0,0 +1,20 @@ +// 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 and +#include +// 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 and +#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 + +int main() { + // f() is declared in r.h +} diff --git a/clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes.cpp b/clang-tools-extra/test/clang-tidy/fuchsia-restrict-system-includes.cpp new file mode 100644 index 0000000..e2ba710 --- /dev/null +++ b/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 +#include +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed +// CHECK-FIXES-NOT: #include + +#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 + +#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 -- 2.7.4