[clang-tidy] Fix confusable identifiers interaction with DeclContext

Properly checks enclosing DeclContext, and add the related test case.
It would be great to be able to use Sema to check conflicting scopes, but that's
not something clang-tidy seems to be able to do :-/

Fix #56221

Differential Revision: https://reviews.llvm.org/D128715

clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp

@@ -93,22 +93,70 @@ std::string ConfusableIdentifierCheck::skeleton(StringRef Name) {
   return Skeleton;
 }
 
+static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) {
+  const DeclContext *DC0 = ND0->getDeclContext()->getPrimaryContext();
+  const DeclContext *DC1 = ND1->getDeclContext()->getPrimaryContext();
+
+  if (isa<TemplateTypeParmDecl>(ND0) || isa<TemplateTypeParmDecl>(ND0))
+    return true;
+
+  while (DC0->isTransparentContext())
+    DC0 = DC0->getParent();
+  while (DC1->isTransparentContext())
+    DC1 = DC1->getParent();
+
+  if (DC0->Equals(DC1))
+    return true;
+
+  return false;
+}
+
+static bool isMemberOf(const NamedDecl *ND, const CXXRecordDecl *RD) {
+  const DeclContext *NDParent = ND->getDeclContext();
+  if (!NDParent || !isa<CXXRecordDecl>(NDParent))
+    return false;
+  if (NDParent == RD)
+    return true;
+  return !RD->forallBases(
+      [NDParent](const CXXRecordDecl *Base) { return NDParent != Base; });
+}
+
+static bool mayShadow(const NamedDecl *ND0, const NamedDecl *ND1) {
+
+  const DeclContext *DC0 = ND0->getDeclContext()->getPrimaryContext();
+  const DeclContext *DC1 = ND1->getDeclContext()->getPrimaryContext();
+
+  if (const CXXRecordDecl *RD0 = dyn_cast<CXXRecordDecl>(DC0)) {
+    if (ND1->getAccess() != AS_private && isMemberOf(ND1, RD0))
+      return true;
+  }
+  if (const CXXRecordDecl *RD1 = dyn_cast<CXXRecordDecl>(DC1)) {
+    if (ND0->getAccess() != AS_private && isMemberOf(ND0, RD1))
+      return true;
+  }
+
+  if (DC0->Encloses(DC1))
+    return mayShadowImpl(ND0, ND1);
+  if (DC1->Encloses(DC0))
+    return mayShadowImpl(ND1, ND0);
+  return false;
+}
+
 void ConfusableIdentifierCheck::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
   if (const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("nameddecl")) {
-    if (IdentifierInfo *II = ND->getIdentifier()) {
-      StringRef NDName = II->getName();
+    if (IdentifierInfo *NDII = ND->getIdentifier()) {
+      StringRef NDName = NDII->getName();
       llvm::SmallVector<const NamedDecl *> &Mapped = Mapper[skeleton(NDName)];
-      const DeclContext *NDDecl = ND->getDeclContext();
       for (const NamedDecl *OND : Mapped) {
-        if (!NDDecl->isDeclInLexicalTraversal(OND) &&
-            !OND->getDeclContext()->isDeclInLexicalTraversal(ND))
-          continue;
-        if (OND->getIdentifier()->getName() != NDName) {
-          diag(OND->getLocation(), "%0 is confusable with %1")
-              << OND->getName() << NDName;
-          diag(ND->getLocation(), "other declaration found here",
-               DiagnosticIDs::Note);
+        const IdentifierInfo *ONDII = OND->getIdentifier();
+        if (mayShadow(ND, OND)) {
+          StringRef ONDName = ONDII->getName();
+          if (ONDName != NDName) {
+            diag(ND->getLocation(), "%0 is confusable with %1") << ND << OND;
+            diag(OND->getLocation(), "other declaration found here",
+                 DiagnosticIDs::Note);
+          }
         }
       }
       Mapped.push_back(ND);

clang-tools-extra/test/clang-tidy/checkers/misc/confusable-identifiers.cpp

@@ -1,9 +1,9 @@
 // RUN: %check_clang_tidy %s misc-confusable-identifiers %t
 
 int fo;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: fo is confusable with 𝐟o [misc-confusable-identifiers]
 int 𝐟o;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: note: other declaration found here
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: '𝐟o' is confusable with 'fo' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-3]]:5: note: other declaration found here
 
 void no() {
   int 𝐟oo;
@@ -12,14 +12,102 @@ void no() {
 void worry() {
   int foo;
 }
-
 int 𝐟i;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 𝐟i is confusable with fi [misc-confusable-identifiers]
 int fi;
-// CHECK-MESSAGES: :[[#@LINE-1]]:5: note: other declaration found here
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 'fi' is confusable with '𝐟i' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-3]]:5: note: other declaration found here
+
+bool f0(const char *q1, const char *ql) {
+  // CHECK-MESSAGES: :[[#@LINE-1]]:37: warning: 'ql' is confusable with 'q1' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-2]]:21: note: other declaration found here
+  return q1 < ql;
+}
 
 // should not print anything
 namespace ns {
 struct Foo {};
 } // namespace ns
 auto f = ns::Foo();
+
+struct Test {
+  void f1(const char *pl);
+};
+
+bool f2(const char *p1, const char *ql) {
+  return p1 < ql;
+}
+
+bool f3(const char *q0, const char *q1) {
+  return q0 < q1;
+}
+
+template <typename i1>
+struct S {
+  template <typename il>
+  void f4() {}
+  // CHECK-MESSAGES: :[[#@LINE-2]]:22: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-5]]:20: note: other declaration found here
+};
+
+template <typename i1>
+void f5(int il) {
+  // CHECK-MESSAGES: :[[#@LINE-1]]:13: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-3]]:20: note: other declaration found here
+}
+
+template <typename O0>
+void f6() {
+  int OO = 0;
+  // CHECK-MESSAGES: :[[#@LINE-1]]:7: warning: 'OO' is confusable with 'O0' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-4]]:20: note: other declaration found here
+}
+int OO = 0; // no warning, not same scope as f6
+
+namespace f7 {
+int i1;
+}
+
+namespace f8 {
+int il; // no warning, different namespace
+}
+
+namespace f7 {
+int il;
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: 'il' is confusable with 'i1' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-10]]:5: note: other declaration found here
+} // namespace f7
+
+template <typename t1, typename tl>
+// CHECK-MESSAGES: :[[#@LINE-1]]:33: warning: 'tl' is confusable with 't1' [misc-confusable-identifiers]
+// CHECK-MESSAGES: :[[#@LINE-2]]:20: note: other declaration found here
+void f9();
+
+struct Base0 {
+  virtual void mO0();
+
+private:
+  void mII();
+};
+
+struct Derived0 : Base0 {
+  void mOO();
+  // CHECK-MESSAGES: :[[#@LINE-1]]:8: warning: 'mOO' is confusable with 'mO0' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-9]]:16: note: other declaration found here
+
+  void mI1(); // no warning: mII is private
+};
+
+struct Base1 {
+  long mO0;
+
+private:
+  long mII;
+};
+
+struct Derived1 : Base1 {
+  long mOO;
+  // CHECK-MESSAGES: :[[#@LINE-1]]:8: warning: 'mOO' is confusable with 'mO0' [misc-confusable-identifiers]
+  // CHECK-MESSAGES: :[[#@LINE-9]]:8: note: other declaration found here
+
+  long mI1(); // no warning: mII is private
+};