[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:32
+void AnonymousEnclosedAliasesCheck::check(const MatchFinder::MatchResult &Result) {
+
+  const auto *MatchedUsingDecl =
----------------
Spurious newline.


================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41
+    // to the vector containing all candidate using declarations.
+    if (AnonymousNamespaceDecl) {
+  		diag(MatchedUsingDecl->getLocation(),
----------------
I don't think this logic works in practice because there's no way to determine that the anonymous namespace is even a candidate for putting the using declaration into it. Consider a common scenario where there's an anonymous namespace declared in a header file (like an STL header outside of the user's control), and a using declaration inside of an implementation file. Just because the STL declared an anonymous namespace doesn't mean that the user could have put their using declaration in it.


================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:43-44
+  		diag(MatchedUsingDecl->getLocation(),
+  			"UsingDecl %0 should be in the anonymous namespace. See: "
+        "https://abseil.io/tips/119")
+  			<< MatchedUsingDecl;
----------------
Diagnostics should not be complete sentences or contain hyperlinks. (Same below.)


================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.h:20
+
+/// Detecting incorrect practice of putting using declarations outside an
+/// anonymous namespace when there exists one.
----------------
Detecting incorrect -> Detecting the incorrect


================
Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.h:21
+/// Detecting incorrect practice of putting using declarations outside an
+/// anonymous namespace when there exists one.
+/// For the user-facing documentation see:
----------------
when there exists one -> when one exists


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55409/new/

https://reviews.llvm.org/D55409