[clang-tidy] Move fuchsia-restrict-system-includes to portability module for general...
authorPaula Toth <paulatoth@google.com>
Tue, 10 Mar 2020 17:28:23 +0000 (10:28 -0700)
committerPaula Toth <paulatoth@google.com>
Tue, 10 Mar 2020 20:33:06 +0000 (13:33 -0700)
Summary:
Created a general check for restrict-system-includes under portability as recommend in the comments under D75332. I also fleshed out the user facing documentation to show examples for common use-cases such as allow-list, block-list, and wild carding.

Removed fuchsia's check as per phosek sugguestion.

Reviewers: aaron.ballman, phosek, alexfh, hokein, njames93

Reviewed By: phosek

Subscribers: Eugene.Zelenko, mgorny, xazax.hun, phosek, cfe-commits, MaskRay

Tags: #clang-tools-extra, #clang

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

26 files changed:
clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tools-extra/clang-tidy/portability/CMakeLists.txt
clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp [moved from clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp with 97% similarity]
clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h [moved from clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h with 74% similarity]
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst [deleted file]
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/portability-restrict-system-includes.rst [new file with mode: 0644]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/a.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/a.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/cstdarg.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/cstdarg.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/cstdlib.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/cstdlib.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/j.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/j.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/r.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/r.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/s.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/s.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/t.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/t.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/system/transitive.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/system/transitive.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-restrict-system-includes/transitive2.h [moved from clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-system-includes/transitive2.h with 100% similarity]
clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-all.cpp [deleted file]
clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-glob.cpp [deleted file]
clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes.cpp [deleted file]
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp [new file with mode: 0644]
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp [new file with mode: 0644]
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-glob.cpp [new file with mode: 0644]
clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-transitive.cpp [moved from clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-headers.cpp with 54% similarity]

index 30b319e..26e6719 100644 (file)
@@ -6,7 +6,6 @@ add_clang_library(clangTidyFuchsiaModule
   FuchsiaTidyModule.cpp
   MultipleInheritanceCheck.cpp
   OverloadedOperatorCheck.cpp
-  RestrictSystemIncludesCheck.cpp
   StaticallyConstructedObjectsCheck.cpp
   TrailingReturnCheck.cpp
   VirtualInheritanceCheck.cpp
index c59ec85..39872fa 100644 (file)
@@ -14,7 +14,6 @@
 #include "DefaultArgumentsDeclarationsCheck.h"
 #include "MultipleInheritanceCheck.h"
 #include "OverloadedOperatorCheck.h"
-#include "RestrictSystemIncludesCheck.h"
 #include "StaticallyConstructedObjectsCheck.h"
 #include "TrailingReturnCheck.h"
 #include "VirtualInheritanceCheck.h"
@@ -39,8 +38,6 @@ 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>(
index 0420a18..d2e3564 100644 (file)
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyPortabilityModule
   PortabilityTidyModule.cpp
+  RestrictSystemIncludesCheck.cpp
   SIMDIntrinsicsCheck.cpp
 
   LINK_LIBS
index e12821e..c87a119 100644 (file)
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "RestrictSystemIncludesCheck.h"
 #include "SIMDIntrinsicsCheck.h"
 
 namespace clang {
@@ -18,6 +19,8 @@ namespace portability {
 class PortabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<RestrictSystemIncludesCheck>(
+        "portability-restrict-system-includes");
     CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
         "portability-simd-intrinsics");
   }
@@ -1,4 +1,4 @@
-//===--- RestrictSystemIncludesCheck.cpp - clang-tidy----------------------===//
+//===--- RestrictSystemIncludesCheck.cpp - clang-tidy ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -18,7 +18,7 @@
 
 namespace clang {
 namespace tidy {
-namespace fuchsia {
+namespace portability {
 
 class RestrictedIncludesPPCallbacks : public PPCallbacks {
 public:
@@ -111,6 +111,6 @@ void RestrictSystemIncludesCheck::storeOptions(
   Options.store(Opts, "Includes", AllowedIncludes);
 }
 
-} // namespace fuchsia
+} // namespace portability
 } // namespace tidy
 } // namespace clang
@@ -1,4 +1,4 @@
-//===--- RestrictSystemIncludesCheck.h - clang-tidy---------- ----*- C++-*-===//
+//===--- RestrictSystemIncludesCheck.h - clang-tidy --------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,21 +6,21 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_RESTRICTINCLUDESSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_RESTRICTINCLUDESSCHECK_H
 
 #include "../ClangTidyCheck.h"
 #include "../GlobList.h"
 
 namespace clang {
 namespace tidy {
-namespace fuchsia {
+namespace portability {
 
 /// 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
+/// http://clang.llvm.org/extra/clang-tidy/checks/portability-restrict-system-includes.html
 class RestrictSystemIncludesCheck : public ClangTidyCheck {
 public:
   RestrictSystemIncludesCheck(StringRef Name, ClangTidyContext *Context)
@@ -40,8 +40,8 @@ private:
   GlobList AllowedIncludesGlobList;
 };
 
-} // namespace fuchsia
+} // namespace portability 
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_RESTRICTINCLUDESSCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_RESTRICTINCLUDESSCHECK_H
\ No newline at end of file
index 3a58649..68ca5f6 100644 (file)
@@ -133,6 +133,9 @@ Changes in existing checks
 Renamed checks
 ^^^^^^^^^^^^^^
 
+- The 'fuchsia-restrict-system-headers' check was renamed to :doc:`portability-restrict-system-includes
+  <clang-tidy/checks/portability-restrict-system-includes>
+
 Improvements to include-fixer
 -----------------------------
 
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
deleted file mode 100644 (file)
index 622e025..0000000
+++ /dev/null
@@ -1,32 +0,0 @@
-.. 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.
index 5a5140b..6930f6e 100644 (file)
@@ -155,7 +155,6 @@ Clang-Tidy Checks
    `fuchsia-default-arguments-declarations <fuchsia-default-arguments-declarations.html>`_, "Yes"
    `fuchsia-multiple-inheritance <fuchsia-multiple-inheritance.html>`_,
    `fuchsia-overloaded-operator <fuchsia-overloaded-operator.html>`_,
-   `fuchsia-restrict-system-includes <fuchsia-restrict-system-includes.html>`_, "Yes"
    `fuchsia-statically-constructed-objects <fuchsia-statically-constructed-objects.html>`_,
    `fuchsia-trailing-return <fuchsia-trailing-return.html>`_,
    `fuchsia-virtual-inheritance <fuchsia-virtual-inheritance.html>`_,
@@ -256,6 +255,7 @@ Clang-Tidy Checks
    `performance-type-promotion-in-math-fn <performance-type-promotion-in-math-fn.html>`_, "Yes"
    `performance-unnecessary-copy-initialization <performance-unnecessary-copy-initialization.html>`_,
    `performance-unnecessary-value-param <performance-unnecessary-value-param.html>`_, "Yes"
+   `portability-restrict-system-includes <portability-restrict-system-includes.html>`_, "Yes"
    `portability-simd-intrinsics <portability-simd-intrinsics.html>`_,
    `readability-avoid-const-params-in-decls <readability-avoid-const-params-in-decls.html>`_,
    `readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/portability-restrict-system-includes.rst b/clang-tools-extra/docs/clang-tidy/checks/portability-restrict-system-includes.rst
new file mode 100644 (file)
index 0000000..9345506
--- /dev/null
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - portability-restrict-system-includes
+
+portability-restrict-system-includes
+====================================
+
+Checks to selectively allow or disallow a configurable list of system headers.
+
+For example:
+
+In order to **only** allow `zlib.h` from the system you would set the options
+to `-*,zlib.h`.
+
+.. code-block:: c++
+
+  #include <curses.h>       // Bad: disallowed system header.
+  #include <openssl/ssl.h>  // Bad: disallowed system header.
+  #include <zlib.h>         // Good: allowed system header.
+  #include "src/myfile.h"   // Good: non-system header always allowed.
+
+In order to allow everything **except** `zlib.h` from the system you would set
+the options to `*,-zlib.h`.
+
+.. code-block:: c++
+
+  #include <curses.h>       // Good: allowed system header.
+  #include <openssl/ssl.h>  // Good: allowed system header.
+  #include <zlib.h>         // Bad: disallowed system header.
+  #include "src/myfile.h"   // Good: non-system header always allowed.
+
+Since the opions support globbing you can use wildcarding to allow groups of
+headers.
+
+`-*,openssl/*.h` will allow all openssl headers but disallow any others.
+
+.. code-block:: c++
+
+  #include <curses.h>       // Bad: disallowed system header.
+  #include <openssl/ssl.h>  // Good: allowed system header.
+  #include <openssl/rsa.h>  // Good: allowed system header.
+  #include <zlib.h>         // Bad: disallowed system header.
+  #include "src/myfile.h"   // Good: non-system header always allowed.
+
+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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-all.cpp b/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-all.cpp
deleted file mode 100644 (file)
index d521c13..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
-// RUN:     -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: ''}]}" \
-// RUN:     -- -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
diff --git a/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-glob.cpp b/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes-glob.cpp
deleted file mode 100644 (file)
index 104f368..0000000
+++ /dev/null
@@ -1,9 +0,0 @@
-// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
-// RUN:     -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 'cstd*'}]}" \
-// RUN:     -- -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>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes.cpp b/clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-includes.cpp
deleted file mode 100644 (file)
index c366408..0000000
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
-// RUN:     -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 's.h'}]}" \
-// RUN:     -- -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
diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp b/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp
new file mode 100644 (file)
index 0000000..faab977
--- /dev/null
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy %s portability-restrict-system-includes %t \
+// RUN:     -- -config="{CheckOptions: [{key: portability-restrict-system-includes.Includes, value: '*,-stdio.h'}]}"
+
+// Test block-list functionality: allow all but stdio.h.
+
+#include <stdio.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include stdio.h not allowed
+#include <stdlib.h>
+#include <string.h>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp b/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp
new file mode 100644 (file)
index 0000000..58c1d43
--- /dev/null
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s portability-restrict-system-includes %t \
+// RUN:     -- -config="{CheckOptions: [{key: portability-restrict-system-includes.Includes, value: '-*,stdio.h'}]}"
+
+// Test allow-list functionality: disallow all but stdio.h.
+
+#include <stdio.h>
+#include <stdlib.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include stdlib.h not allowed
+#include <string.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include string.h not allowed
diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-glob.cpp b/clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-glob.cpp
new file mode 100644 (file)
index 0000000..ccde943
--- /dev/null
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s portability-restrict-system-includes %t \
+// RUN:     -- -config="{CheckOptions: [{key: portability-restrict-system-includes.Includes, value: '-*,std*.h'}]}"
+
+// Test glob functionality: disallow all headers except those that match
+// pattern "std*.h".
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include string.h not allowed
@@ -1,11 +1,11 @@
 // RUN: rm -rf %T/Headers
 // RUN: mkdir %T/Headers
-// RUN: cp -r %S/Inputs/fuchsia-restrict-system-includes %T/Headers/fuchsia-restrict-system-includes
-// RUN: %check_clang_tidy -std=c++11 %s fuchsia-restrict-system-includes %t \
-// RUN:   -- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 'transitive.h,s.h'}]}" \
+// RUN: cp -r %S/Inputs/portability-restrict-system-includes %T/Headers/portability-restrict-system-includes
+// RUN: %check_clang_tidy -std=c++11 %s portability-restrict-system-includes %t \
+// RUN:   -- -config="{CheckOptions: [{key: portability-restrict-system-includes.Includes, value: 'transitive.h,s.h'}]}" \
 // RUN:   -system-headers -header-filter=.* \
-// RUN:   -- -I %T/Headers/fuchsia-restrict-system-includes -isystem %T/Headers/fuchsia-restrict-system-includes/system
-// RUN: FileCheck -input-file=%T/Headers/fuchsia-restrict-system-includes/transitive2.h %s -check-prefix=CHECK-FIXES
+// RUN:   -- -I %T/Headers/portability-restrict-system-includes -isystem %T/Headers/portability-restrict-system-includes/system
+// RUN: FileCheck -input-file=%T/Headers/portability-restrict-system-includes/transitive2.h %s -check-prefix=CHECK-FIXES
 // RUN: rm -rf %T/Headers
 // FIXME: Make the test work in all language modes.