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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/abseil/AbseilMatcher.h:16
+
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,
----------------
nit: We need proper documentation for this matcher, since it is exposed to users.


================
Comment at: clang-tidy/abseil/AbseilMatcher.h:17
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
----------------
- using negative for AST matcher seems uncommon in ASTMatchers, for negative we have `unless`, so I'd suggest implementing `isInAbseilHeader`.
- it seems that this matcher is very similar to the matcher in https://reviews.llvm.org/D50542.  I think `isInAbseilFile` should also cover your case here, and also ignore the warning if you run the check on abseil core files.


================
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+                 "synchronization|types|utiliy)");
----------------
The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. 


================
Comment at: clang-tidy/abseil/AbseilMatcher.h:33
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+                 "synchronization|types|utiliy)");
+  return (!RE.match(Filename));
----------------
typo: `utility`


================
Comment at: clang-tidy/abseil/AbseilMatcher.h:34
+                 "synchronization|types|utiliy)");
+  return (!RE.match(Filename));
+}
----------------
nit: remove the outer `()`. 


https://reviews.llvm.org/D50580