[PATCH] D55346: [clang-tidy] check for using declaration qualification

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:29
+  // Finds the nested-name-specifier location.
+  const NestedNameSpecifierLoc QualifiedLoc = MatchedDecl->getQualifierLoc();
+  const SourceLocation FrontLoc = QualifiedLoc.getBeginLoc();
----------------
Drop the top-level `const` qualifier (here and elsewhere).


================
Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:34
+  const SourceManager *SM = Result.SourceManager;
+  CharSourceRange FrontRange = CharSourceRange();
+  FrontRange.setBegin(FrontLoc);
----------------
No need for the assignment.


================
Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:39
+
+  // If the using declaration is fully qualified, don't produce a warning.
+  if (Beg.startswith("::"))
----------------
This is missing a check for whether the referenced name is within the current namespace.


================
Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:43-44
+
+  diag(FrontLoc, "using declaration is not fully qualified: see "
+  "https://abseil.io/tips/119");
+}
----------------
Do not put HTML links into the diagnostic -- the wording should stand on its own. I would probably phrase it "using declaration should use a fully qualified name" or something along those lines.


================
Comment at: clang-tidy/abseil/QualifiedAliasesCheck.h:19
+
+/// FIXME: Write a short description.
+///
----------------
This should be fixed.


================
Comment at: test/clang-tidy/abseil-qualified-aliases.cpp:22-23
+
+    using innermost::Color;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration is not fully qualified: see https://abseil.io/tips/119 [abseil-qualified-aliases]
+
----------------
I don't think this should warn, per the tip.


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

https://reviews.llvm.org/D55346