Ywicheng updated this revision to Diff 195661.
Ywicheng marked 3 inline comments as done.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+
+} // namespace bar
+
+namespace foo1 {
+
+// CHECK-MESSAGES: :[[@LINE+4]]:12: warning: using declaration 'A' is not
+// declared in the innermost namespace. Use discretion when moving using
+// declarations as it might necessitate moving lines containing relevant
+// aliases. [abseil-safely-scoped]
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+
+} // namespace foo1
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+
+void g(B b);
+
+} // namespace foo2
+
+// Check should not be triggered below when we are at
+// function (instead of namespace) scope.
+namespace outer {
+
+int fun_scope() {
+  using ::bar::A;
+  return 0;
+} // function scope
+
+class Base {
+public:
+  void f();
+}; // class scope
+
+class Derived : public Base {
+public:
+  using Base::f;
+}; // class scope
+
+namespace inner {} // namespace inner
+
+} // namespace outer
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
    abseil-no-internal-dependencies
    abseil-no-namespace
    abseil-redundant-strcat-calls
+   abseil-safely-scoped
    abseil-str-cat-append
    abseil-string-find-startswith
    abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+====================
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --------------------------
 
+- New :doc:`abseil-safely-scoped
+  <clang-tidy/checks/abseil-safely-scoped>` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   <clang-tidy/checks/abseil-duration-addition>` check.
 
Index: clang-tidy/abseil/SafelyScopedCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,45 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy -------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SafelyScopedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void SafelyScopedCheck::registerMatchers(MatchFinder *Finder) {
+  // The target using declaration is either:
+  // 1. not in any scope, or
+  // 2. in some namespace declaration but not in the innermost layer
+  Finder->addMatcher(
+      usingDecl(
+          eachOf(allOf(usingDecl(unless(hasAncestor(functionDecl()))),
+                       usingDecl(unless(hasAncestor(cxxRecordDecl()))),
+                       usingDecl(unless(hasAncestor(namespaceDecl())))),
+                 usingDecl(hasParent(namespaceDecl(forEach(namespaceDecl()))))))
+          .bind("use"),
+      this);
+}
+
+void SafelyScopedCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<UsingDecl>("use");
+  diag(MatchedDecl->getLocation(),
+       "using declaration %0 is not declared in the innermost namespace. "
+       "Use discretion when moving using declarations as it might "
+       "necessitate moving lines containing relevant aliases.")
+      << MatchedDecl;
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -15,6 +15,7 @@
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
   RedundantStrcatCallsCheck.cpp
+  SafelyScopedCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
   TimeSubtractionCheck.cpp
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tidy/abseil/AbseilTidyModule.cpp
@@ -21,6 +21,7 @@
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
 #include "RedundantStrcatCallsCheck.h"
+#include "SafelyScopedCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
 #include "TimeSubtractionCheck.h"
@@ -56,6 +57,8 @@
     CheckFactories.registerCheck<NoNamespaceCheck>("abseil-no-namespace");
     CheckFactories.registerCheck<RedundantStrcatCallsCheck>(
         "abseil-redundant-strcat-calls");
+    CheckFactories.registerCheck<SafelyScopedCheck>(
+        "abseil-safely-scoped");
     CheckFactories.registerCheck<StrCatAppendCheck>(
         "abseil-str-cat-append");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D55411: [clang-tidy]... Yucheng Wu via Phabricator via cfe-commits

Reply via email to