[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

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

[njames93]

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

23

Why restrict these to main file only?

30–33

nit: Elide braces of single line if statements.

35–38

45

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

50

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

78

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

114 ↗

(On Diff #326320)

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

62 ↗

(On Diff #326320)

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/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyMiscModule
   DefinitionsInHeadersCheck.cpp
   MiscTidyModule.cpp
   MisplacedConstCheck.cpp
   NewDeleteOverloadsCheck.cpp
   NoRecursionCheck.cpp
   NonCopyableObjects.cpp
   NonPrivateMemberVariablesInClassesCheck.cpp
   RedundantExpressionCheck.cpp
+  RedundantUsingCheck.cpp
   StaticAssertCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   UniqueptrResetReleaseCheck.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedUsingDeclsCheck.cpp
 

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

 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NoRecursionCheck.h"
 #include "NonCopyableObjects.h"
 #include "NonPrivateMemberVariablesInClassesCheck.h"
 #include "RedundantExpressionCheck.h"
+#include "RedundantUsingCheck.h"
 #include "StaticAssertCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedUsingDeclsCheck.h"
 
CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
         "misc-new-delete-overloads");
     CheckFactories.registerCheck<NoRecursionCheck>("misc-no-recursion");
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "misc-non-copyable-objects");
     CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
         "misc-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<RedundantExpressionCheck>(
         "misc-redundant-expression");
+    CheckFactories.registerCheck<RedundantUsingCheck>("misc-redundant-using");
     CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
     CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
         "misc-throw-by-value-catch-by-reference");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
         "misc-unconventional-assign-operator");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");
     CheckFactories.registerCheck<UnusedAliasDeclsCheck>(

clang-tools-extra/clang-tidy/misc/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_MISC_REDUNDANTUSINGCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTUSINGCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds redundant using declarations and directives.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-using.html
+class RedundantUsingCheck : public ClangTidyCheck {
+public:
+  RedundantUsingCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  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(const char *Type, const Decl *Using, const NamedDecl *ND,
+                 const NamespaceDecl *NSD,
+                 const ast_matchers::MatchFinder::MatchResult &Result);
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_REDUNDANTUSINGCHECK_H

clang-tools-extra/clang-tidy/misc/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 misc {
+
+void RedundantUsingCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      usingDecl(isExpansionInMainFile()).bind("using-declaration"), this);
+  Finder->addMatcher(
+      usingDirectiveDecl(isExpansionInMainFile()).bind("using-directive"),
+      this);
+}
+
+void RedundantUsingCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Declaration =
+          Result.Nodes.getNodeAs<UsingDecl>("using-declaration")) {
+    checkUsingDecl(Declaration, Result);
+  }
+
+  if (const auto *Directive =
+          Result.Nodes.getNodeAs<UsingDirectiveDecl>("using-directive")) {
+    checkUsingDirective(Directive, Result);
+  }
+}
+
+void RedundantUsingCheck::checkUsingDecl(
+    const UsingDecl *Declaration, const MatchFinder::MatchResult &Result) {
+  const DeclContext *DC = Declaration->getDeclContext();
+
+  for (const auto *UsingShadow : Declaration->shadows()) {
+    const Decl *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
+
+    if (TargetDecl->getDeclContext()->Encloses(DC)) {
+      const auto *NSD = cast<NamespaceDecl>(TargetDecl->getDeclContext());
+      diagUsing("declaration", Declaration, Declaration, NSD, Result);
+    }
+  }
+}
+
+void RedundantUsingCheck::checkUsingDirective(
+    const UsingDirectiveDecl *Directive,
+    const MatchFinder::MatchResult &Result) {
+  const NamespaceDecl *NDNominated = Directive->getNominatedNamespace();
+  const DeclContext *DC = Directive->getDeclContext();
+
+  if (NDNominated->Encloses(DC)) {
+    const NamedDecl *NDAsWritten = Directive->getNominatedNamespaceAsWritten();
+
+    diagUsing("directive", Directive, NDAsWritten, NDNominated, Result);
+  }
+}
+void RedundantUsingCheck::diagUsing(const char *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 %0 %1 is redundant, already in namespace %2")
+      << Type << ND << NSD;
+  diag(Using->getLocation(), "remove the using %0", DiagnosticIDs::Note)
+      << Type << FixItHint::CreateRemoval(RemoveRange);
+}
+
+} // namespace misc
+} // 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:`misc-redundant-using
+  <clang-tidy/checks/misc-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::
    `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-no-recursion <misc-no-recursion.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
+   `misc-redundant-using <misc-redundant-using.html>`_, "Yes"
    `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
    `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
    `misc-uniqueptr-reset-release <misc-uniqueptr-reset-release.html>`_, "Yes"
    `misc-unused-alias-decls <misc-unused-alias-decls.html>`_, "Yes"
    `misc-unused-parameters <misc-unused-parameters.html>`_, "Yes"
    `misc-unused-using-decls <misc-unused-using-decls.html>`_, "Yes"
    `modernize-avoid-bind <modernize-avoid-bind.html>`_, "Yes"

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

+.. title:: clang-tidy - misc-redundant-using
+
+misc-redundant-using
+====================
+
+Finds redundant ``using`` declarations and directives.
+
+Example:
+
+.. code-block:: c++
+
+  namespace n {
+  void func();
+  }
+
+  namespace  {
+  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/misc-redundant-using.cpp

+// RUN: %check_clang_tidy %s misc-redundant-using %t
+
+namespace n1 {
+using namespace n1;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: using directive 'n1' is redundant, already in namespace 'n1' [misc-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' [misc-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' [misc-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' [misc-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' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: remove the using directive
+// CHECK-FIXES: {{^}}
+} // namespace inner
+} // namespace n4
+
+namespace n5 {
+namespace inner {
+using namespace n4::inner; // ok
+}
+} // namespace n5
+
+namespace n6 {
+void func();
+
+using n6::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration 'func' is redundant, already in namespace 'n6' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:11: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace n6
+
+namespace n7 {
+using n6::func; // ok
+}
+
+namespace n8 {
+void func();
+
+namespace n = n8;
+using n::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using declaration 'func' is redundant, already in namespace 'n8' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace n8
+
+namespace n9 {
+void outerFunc();
+
+namespace inner {
+using n9::outerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration 'outerFunc' is redundant, already in namespace 'n9' [misc-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' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:14: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+
+using n9::inner::innerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: using declaration 'innerFunc' is redundant, already in namespace 'inner' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+
+namespace n = n9::inner;
+using n::innerFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using declaration 'innerFunc' is redundant, already in namespace 'inner' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace inner
+} // namespace n9
+
+namespace n10 {
+namespace inner {
+using n9::inner::innerFunc; // ok
+}
+} // namespace n10
+
+namespace n11 {
+void func();
+}
+
+namespace n11 {
+using n11::func;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: using declaration 'func' is redundant, already in namespace 'n11' [misc-redundant-using]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: remove the using declaration
+// CHECK-FIXES: {{^}}
+} // namespace n11