[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+  Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+                     this);
----------------
hokein wrote:
> aaron.ballman wrote:
> > I think this needs a `not(isExpansionInSystemHeader())` in there, as this is going to trigger on code that includes a header file using an `absl` namespace. If I'm incorrect and users typically include abseil as something other than system includes, you'll have to find a different way to solve this.
> Yeah, we have discussed this issue internally.  abseil-user code usually includes abseil header like `#include "whatever/abseil/base/optimization.h"`, so `IsExpansionInSystemHeader` doesn't work for most cases. 
> 
> We need a way to filter out all headers being a part of abseil library. I think we can create a matcher `InExpansionInAbseilHeader` -- basically using `IsExpansionInFileMatching` with a regex expression that matches typical abseil include path. What do you think?
> 
> And we'll have more abseil checks (e.g.  `abseil-no-internal-deps`) that use `InExpansionInAbseilHeader`, we should put it to a common header.
I think that is a sensible approach.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50580