[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:6
+
+Gives a warning if code using Abseil depends on internal details. If something is in a namespace or filename/path that includes the word “internal”, code is not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
----------------
aaron.ballman wrote:
> JonasToth wrote:
> > s/violtaes/violates/
> Please wrap lines to 80 cols.
> 
> Nothing in this check looks at filenames and paths, but this suggests the check will find those. Is that intended work that's missed in this patch, or are the docs incorrect?
Please provide a link for the abseil guideline.


================
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
----------------
nit: please make sure the code follow LLVM code style, even for test code :)


================
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:11
+
+namespace absl {
+std::string StringsFunction (std::string s1){
----------------
Since we have multiple abseil checks that might use these fake abseil declarations, I'd suggest pull out these to a common header, and include it in this test file.


https://reviews.llvm.org/D50542