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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the updates. Looks mostly good, a few nits.



================
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+
+AST_POLYMORPHIC_MATCHER(isExpansionInAbseilHeader,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(
----------------
nit: this matcher name now should be `isInAbseilFile`.


================
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:26
+  Finder->addMatcher(
+      namespaceDecl(hasName("absl"), unless(isExpansionInAbseilHeader()))
+          .bind("abslNamespace"),
----------------
Does the `absl` namespace is always the top level namespace? If so, I think it would be safer to use qualified name "::absl" here.


================
Comment at: clang-tidy/abseil/NoNamespaceCheck.h:19
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
----------------
nit: test => check.


================
Comment at: clang-tidy/abseil/NoNamespaceCheck.h:20
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
----------------
nit: our = abseil?


================
Comment at: test/clang-tidy/abseil-no-namespace.cpp:22
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' 
+
----------------
nit: put it immediately after the above `name absl {`.


https://reviews.llvm.org/D50580