diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -35,6 +35,7 @@ RedundantSmartptrGetCheck.cpp RedundantStringCStrCheck.cpp RedundantStringInitCheck.cpp + RedundantUsingCheck.cpp SimplifyBooleanExprCheck.cpp SimplifySubscriptExprCheck.cpp StaticAccessedThroughInstanceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -38,6 +38,7 @@ #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" +#include "RedundantUsingCheck.h" #include "SimplifyBooleanExprCheck.h" #include "SimplifySubscriptExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" @@ -98,6 +99,8 @@ "readability-redundant-member-init"); CheckFactories.registerCheck( "readability-redundant-preprocessor"); + CheckFactories.registerCheck( + "readability-redundant-using"); CheckFactories.registerCheck( "readability-simplify-subscript-expr"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.h @@ -0,0 +1,47 @@ +//===--- 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 diff --git a/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp @@ -0,0 +1,126 @@ +//===--- 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("using-declaration"); + if (Declaration && !Declaration->getLocation().isMacroID()) + checkUsingDecl(Declaration, Result); + + const auto *Directive = + Result.Nodes.getNodeAs("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(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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -89,6 +89,11 @@ Finds member initializations in the constructor body which can be placed into the initialization list instead. +- New :doc:`readability-redundant-using + ` check. + + Finds redundant ``using`` declarations and directives. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -303,6 +303,7 @@ `readability-redundant-smartptr-get `_, "Yes" `readability-redundant-string-cstr `_, "Yes" `readability-redundant-string-init `_, "Yes" + `readability-redundant-using `_, "Yes" `readability-simplify-boolean-expr `_, "Yes" `readability-simplify-subscript-expr `_, "Yes" `readability-static-accessed-through-instance `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-using.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-using.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-using.rst @@ -0,0 +1,25 @@ +.. 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'. + } + diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp @@ -0,0 +1,188 @@ +// 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: {{^}}