[clang-tidy] Add readability-redundant-using check

[nullptr.cpp]

Summary

Finds redundant using declarations and directives.

Example:

namespace n {
void func();
}

namespace n {
using n::func; // redundant using declaration, already in namespace 'n'.
}

namespace n {
using namespace n; // redundant using directive, already in namespace 'n'.
}

[Eugene.Zelenko]

I think this check belongs to readability module.

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

1 ↗

(On Diff #325995)

Please make single line from two. See other checks as example.

[njames93]

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

23 ↗

(On Diff #325995)

Why restrict these to main file only?

30–33 ↗

(On Diff #325995)

nit: Elide braces of single line if statements.

35–38 ↗

(On Diff #325995)

45 ↗

(On Diff #325995)

nit: don't use auto as type isn't explicitly spelled in the initialization.

50 ↗

(On Diff #325995)

Shouldn't there be a break after we have diagnosed this?

78 ↗

(On Diff #325995)

nit: The canonical way to do diagnostics like this is with select. Then pass 0 for declaration and 1 for directive.

[nullptr.cpp]

Modify according to suggestions

[balazske]

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp

115

Add more tests?

namespace outer {
namespace inner {
void func();
} // namespace inner
using inner::func();
} // namespace outer

using namespace outer;
using namespace inner;

[njames93]

Can you override the isLanguageVersionSupported method to ensure this check only runs over c++ code.
Can you add some macro tests, typically we err on the side of caution and don't emit any fixes for macro code. You can use SourceLocation::IsMacroID() to detect macro expansions.
The description looks to have an error in the 2nd example, guess you are messing 'n' after the namespace.
How will this handle inline namespaces.

namespace N {
inline namespace M {
void foo();
void bar();
} // namespace M
inline namespace O {
void bar();
}
using M::foo; // This is redundant.
using M::bar; // This is gonna give you a hard time.
} // namespace N

clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp

63

nit: New line between methods.

[balazske]

You can look into misc/UnusedUsingDeclsCheck.cpp (if not done yet), that check handles using too. And that check is related to this check, the module should be the same too (misc)?

[balazske]

There should be already a name lookup implemented somewhere in clang (DeclContext? or Sema) that maybe usable here (check if the name of the "used" entity is visible in the declaration context of the using statement). Probably it is not as simple to do because the using is already there and it may depend on the order.

[Eugene.Zelenko]

In D97361#2589673, @balazske wrote:

You can look into misc/UnusedUsingDeclsCheck.cpp (if not done yet), that check handles using too. And that check is related to this check, the module should be the same too (misc)?

Majority of redundant checks are in readability. Awhile ago I suggested separate group for unused checks or move them to Clang.

[nullptr.cpp]

• Restrict to only run on C++ code
• Ignore using defined in macro
• Support inline namespace
• Support global namespace

[nullptr.cpp]

Ping

[nullptr.cpp]

Update

[nullptr.cpp]

Rebase

[nullptr.cpp]

In D97361#2609616, @nullptr.cpp wrote:

• Restrict to only run on C++ code
• Ignore using defined in macro
• Support inline namespace
• Support global namespace

Ping.

[nullptr.cpp]

Ping.

[nullptr.cpp]

Ping

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantPreprocessorCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
+  RedundantUsingCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp
   UseAnyOfAllOfCheck.cpp

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
+#include "RedundantUsingCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
 #include "UseAnyOfAllOfCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
         "readability-redundant-access-specifiers");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
         "readability-redundant-function-ptr-dereference");
     CheckFactories.registerCheck<RedundantMemberInitCheck>(
         "readability-redundant-member-init");
     CheckFactories.registerCheck<RedundantPreprocessorCheck>(
         "readability-redundant-preprocessor");
+    CheckFactories.registerCheck<RedundantUsingCheck>(
+        "readability-redundant-using");
     CheckFactories.registerCheck<SimplifySubscriptExprCheck>(
         "readability-simplify-subscript-expr");
     CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>(
         "readability-static-accessed-through-instance");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<StringCompareCheck>(
         "readability-string-compare");

clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.h

+//===--- RedundantUsingCheck.h - clang-tidy ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTUSINGCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTUSINGCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds redundant using declarations and directives.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-using.html
+class RedundantUsingCheck : public ClangTidyCheck {
+public:
+  RedundantUsingCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void checkUsingDecl(const UsingDecl *Declaration,
+                      const ast_matchers::MatchFinder::MatchResult &Result);
+  void
+  checkUsingDirective(const UsingDirectiveDecl *Directive,
+                      const ast_matchers::MatchFinder::MatchResult &Result);
+  void diagUsing(int Type, const Decl *Using, const NamedDecl *ND,
+                 const NamespaceDecl *NSD,
+                 const ast_matchers::MatchFinder::MatchResult &Result);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTUSINGCHECK_H

clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp

+//===--- RedundantUsingCheck.cpp - clang-tidy -----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RedundantUsingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantUsingCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(usingDecl().bind("using-declaration"), this);
+  Finder->addMatcher(usingDirectiveDecl().bind("using-directive"), this);
+}
+
+void RedundantUsingCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Declaration =
+      Result.Nodes.getNodeAs<UsingDecl>("using-declaration");
+  if (Declaration && !Declaration->getLocation().isMacroID())
+    checkUsingDecl(Declaration, Result);
+
+  const auto *Directive =
+      Result.Nodes.getNodeAs<UsingDirectiveDecl>("using-directive");
+  if (Directive && !Directive->getLocation().isMacroID())
+    checkUsingDirective(Directive, Result);
+}
+
+void RedundantUsingCheck::checkUsingDecl(
+    const UsingDecl *Declaration, const MatchFinder::MatchResult &Result) {
+  const DeclContext *DC = Declaration->getDeclContext();
+
+  for (const UsingShadowDecl *UsingShadow : Declaration->shadows()) {
+    const Decl *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
+    const DeclContext *TargetDC = TargetDecl->getDeclContext();
+
+    if (TargetDC->Encloses(DC) || DC->InEnclosingNamespaceSetOf(TargetDC)) {
+      /// In the lookup result of the declaration, if the using declaration is
+      /// redundant, OtherDeclCount should be no greater than ShadowCount.
+      ///
+      /// For example,
+      /// \code
+      /// namespace n {
+      /// namespace a {
+      ///   void func();
+      /// }
+      /// inline namespace b {
+      ///   void func(); // FunctionDecl
+      /// }
+      /// inline namespace c {
+      ///   void func(); // FunctionDecl
+      /// }
+      /// using c::func; // UsingDecl + UsingShadowDecl
+      ///                // not redundant(OtherDeclCount=2, ShadowCount=1)
+      /// } // namespace n
+      ///
+      /// namespace n1 {
+      /// void func();
+      /// void func(int);
+      /// void func(bool);
+      /// namespace n2 {
+      /// using n1::func; // UsingDecl + UsingShadowDecl*3
+      ///                 // redundant(OtherDeclCount=0, ShadowCount=3)
+      /// }
+      /// } // namespace n1
+      /// \endcode
+      constexpr size_t UsingDeclCount = 1;
+      unsigned int ShadowCount = Declaration->shadow_size();
+      DeclContextLookupResult LR = DC->lookup(UsingShadow->getDeclName());
+      size_t OtherDeclCount = LR.size() - ShadowCount - UsingDeclCount;
+
+      if (OtherDeclCount <= ShadowCount) {
+        const DeclContext *EnclosingNS =
+            TargetDC->getEnclosingNamespaceContext();
+        const auto *NSD = dyn_cast<NamespaceDecl>(EnclosingNS);
+
+        diagUsing(0, Declaration, Declaration, NSD, Result);
+        break;
+      }
+    }
+  }
+}
+
+void RedundantUsingCheck::checkUsingDirective(
+    const UsingDirectiveDecl *Directive,
+    const MatchFinder::MatchResult &Result) {
+  const NamespaceDecl *NDNominated = Directive->getNominatedNamespace();
+  const DeclContext *DC = Directive->getDeclContext();
+
+  if (NDNominated->Encloses(DC) || DC->InEnclosingNamespaceSetOf(NDNominated)) {
+    const NamedDecl *NDAsWritten = Directive->getNominatedNamespaceAsWritten();
+
+    diagUsing(1, Directive, NDAsWritten, NDNominated, Result);
+  }
+}
+
+void RedundantUsingCheck::diagUsing(int Type, const Decl *Using,
+                                    const NamedDecl *ND,
+                                    const NamespaceDecl *NSD,
+                                    const MatchFinder::MatchResult &Result) {
+  SourceLocation End =
+      Lexer::findLocationAfterToken(Using->getEndLoc(), tok::semi,
+                                    *Result.SourceManager, getLangOpts(), true);
+  CharSourceRange RemoveRange =
+      CharSourceRange::getCharRange(Using->getBeginLoc(), End);
+
+  diag(Using->getLocation(),
+       "using %select{declaration|directive}0 %1 is "
+       "redundant, already in %select{namespace %3|global namespace}2")
+      << Type << ND << !NSD << NSD;
+  diag(Using->getLocation(), "remove the using %select{declaration|directive}0",
+       DiagnosticIDs::Note)
+      << Type << FixItHint::CreateRemoval(RemoveRange);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`concurrency-thread-canceltype-asynchronous
   type is set to asynchronous.
 
 - New :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
 
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
+- New :doc:`readability-redundant-using
+  <clang-tidy/checks/readability-redundant-using>` check.
+
+  Finds redundant ``using`` declarations and directives.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-pos47-c
   <clang-tidy/checks/cert-pos47-c>` to
   :doc:`concurrency-thread-canceltype-asynchronous
   <clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `readability-redundant-control-flow <readability-redundant-control-flow.html>`_, "Yes"
    `readability-redundant-declaration <readability-redundant-declaration.html>`_, "Yes"
    `readability-redundant-function-ptr-dereference <readability-redundant-function-ptr-dereference.html>`_, "Yes"
    `readability-redundant-member-init <readability-redundant-member-init.html>`_, "Yes"
    `readability-redundant-preprocessor <readability-redundant-preprocessor.html>`_,
    `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes"
    `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_, "Yes"
    `readability-redundant-string-init <readability-redundant-string-init.html>`_, "Yes"
+   `readability-redundant-using <readability-redundant-using.html>`_, "Yes"
    `readability-simplify-boolean-expr <readability-simplify-boolean-expr.html>`_, "Yes"
    `readability-simplify-subscript-expr <readability-simplify-subscript-expr.html>`_, "Yes"
    `readability-static-accessed-through-instance <readability-static-accessed-through-instance.html>`_, "Yes"
    `readability-static-definition-in-anonymous-namespace <readability-static-definition-in-anonymous-namespace.html>`_, "Yes"
    `readability-string-compare <readability-string-compare.html>`_, "Yes"
    `readability-uniqueptr-delete-release <readability-uniqueptr-delete-release.html>`_, "Yes"
    `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `readability-use-anyofallof <readability-use-anyofallof.html>`_,

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-using.rst

+.. title:: clang-tidy - readability-redundant-using
+
+readability-redundant-using
+===========================
+
+Finds redundant ``using`` declarations and directives.
+
+Example:
+
+.. code-block:: c++
+
+  namespace n {
+  void func();
+  }
+
+  namespace n {
+  using n::func; // redundant using declaration, already in namespace 'n'.
+  }
+
+.. code-block:: c++
+
+  namespace n {
+  using namespace n; // redundant using directive, already in namespace 'n'.
+  }
+

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp

+// RUN: %check_clang_tidy %s readability-redundant-using %t
+
+// using directive
+namespace n1 {
+using namespace n1;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: using directive 'n1' is redundant, already in namespace 'n1' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: remove the using directive
+// CHECK-FIXES: {{^}}
+} // namespace n1
+
+namespace n2 {
+using namespace n1; // ok
+}
+
+namespace n3 {
+namespace n = n3;
+using namespace n;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: using directive 'n' is redundant, already in namespace 'n3' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: remove the using directive
+// CHECK-FIXES: {{^}}
+} // namespace n3
+
+namespace n4 {
+namespace inner {
+using namespace n4;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: using directive 'n4' is redundant, already in namespace 'n4' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: remove the using directive
+// CHECK-FIXES: {{^}}
+
+using namespace n4::inner;
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: using directive 'inner' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: remove the using directive
+// CHECK-FIXES: {{^}}
+
+namespace n = n4::inner;
+using namespace n;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: using directive 'n' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: remove the using directive
+// CHECK-FIXES: {{^}}
+} // namespace inner
+
+using namespace inner; // ok
+} // namespace n4
+
+namespace n5 {
+namespace inner {
+using namespace n4::inner; // ok
+}
+} // namespace n5
+
+#define USING_DIRECTIVE(n) using namespace n
+
+namespace n6 {
+USING_DIRECTIVE(n6); // skip macro
+}
+
+// using declaration
+namespace m1 {
+void func();
+
+using m1::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration 'func' is redundant, already in namespace 'm1' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:11: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace m1
+
+namespace m2 {
+using m1::func; // ok
+}
+
+namespace m3 {
+void func();
+
+namespace n = m3;
+using n::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using declaration 'func' is redundant, already in namespace 'm3' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace m3
+
+namespace m4 {
+void outerFunc();
+
+namespace inner {
+using m4::outerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration 'outerFunc' is redundant, already in namespace 'm4' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:11: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+
+void innerFunc();
+using inner::innerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: using declaration 'innerFunc' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+
+using m4::inner::innerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: using declaration 'innerFunc' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+
+namespace n = m4::inner;
+using n::innerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using declaration 'innerFunc' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace inner
+
+using inner::innerFunc; // ok
+} // namespace m4
+
+namespace m5 {
+namespace inner {
+using m4::inner::innerFunc; // ok
+}
+} // namespace m5
+
+#define USING_DECLARATION(n, d) using n::d
+
+namespace m6 {
+void func();
+
+USING_DECLARATION(m6, func); // skip macro
+} // namespace m6
+
+namespace m7 {
+void func();
+}
+
+namespace m7 {
+using m7::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration 'func' is redundant, already in namespace 'm7' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:11: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace m7
+
+namespace m8 {
+inline namespace inner {
+void func();
+}
+
+using inner::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: using declaration 'func' is redundant, already in namespace 'inner' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace m8
+
+namespace m9 {
+namespace a {
+void func();
+}
+
+inline namespace b {
+void func();
+}
+
+inline namespace c {
+void func();
+}
+
+using c::func; // ok
+} // namespace m9
+
+namespace m10 {
+void func();
+void func(int);
+void func(bool);
+
+namespace inner {
+using m10::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: using declaration 'func' is redundant, already in namespace 'm10' [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace inner
+} // namespace m10
+
+void funcOuter();
+
+namespace m11 {
+using ::funcOuter;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: using declaration 'funcOuter' is redundant, already in global namespace [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:9: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace m11
+
+using ::funcOuter;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: using declaration 'funcOuter' is redundant, already in global namespace [readability-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:9: note: remove the using declaration
+// CHECK-FIXES: {{^}}