[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:41
+    CheckFactories.registerCheck<RestrictIncludesCheck>(
+        "fuchsia-restrict-includes");
     CheckFactories.registerCheck<StaticallyConstructedObjectsCheck>(
----------------
I think this should be named `fuchsia-restrict-system-includes`.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:60
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (!llvm::is_contained(Check.getAllowedIncludes(), FileName) && IsAngled)
+    // Bucket the allowed include directives by the id of the file they were
----------------
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?


================
Comment at: docs/ReleaseNotes.rst:116
+
+  Checks for allowed includes and suggests removal of any others. If no includes
+  are specified, the check will exit without issuing any warnings.
----------------
Be explicit that this only impacts system headers.


================
Comment at: docs/clang-tidy/checks/fuchsia-restrict-includes.rst:6-7
+
+Checks for allowed includes and suggests removal of any others. If no includes
+are specified, the check will exit without issuing any warnings. 
+
----------------
Should also be explicit about system headers.


================
Comment at: test/clang-tidy/Inputs/fuchsia-restrict-includes/system/r.h:2
+void f() {}
\ No newline at end of file

----------------
Please add in the EOF.


================
Comment at: test/clang-tidy/Inputs/fuchsia-restrict-includes/system/transitive.h:2
+#include <r.h>
\ No newline at end of file

----------------
Please add in the EOF.


================
Comment at: test/clang-tidy/fuchsia-restrict-includes.cpp:31
+}
\ No newline at end of file

----------------
Please add in the EOF.


https://reviews.llvm.org/D43778