hugoeg updated this revision to Diff 160168.
hugoeg added a comment.
corrections from comments applied
https://reviews.llvm.org/D50542
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoInternalDepsCheck.cpp
clang-tidy/abseil/NoInternalDepsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-internal-deps.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-no-internal-deps.cpp
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===================================================================
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+ return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+
+ void InternalFunction(){}
+
+ template <class P>
+ P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+ struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+ absl::strings_internal::InternalFunction();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+ absl::strings_internal::InternalTemplateFunction <std::string>("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+ friend struct absl::container_internal::InternalStruct;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+ void foo3(){
+ strings_internal::InternalFunction();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+ }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+ std::string str = absl::StringsFunction("a");
+ absl::SomeContainer b;
+}
+
+namespace absl {
+ SomeContainer b;
+ std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-string-find-startswith
+ abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===================================================================
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+=======================
+
+Gives a warning if code using Abseil depends on internal details. If something is in a namespace or filename/path that includes the word “internal”, code is not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+ friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
Improvements to clang-rename
----------------------------
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+ <clang-tidy/checks/abseil-no-internal-deps>` check.
+
+ Gives a warning if code using Abseil depends on internal details.
Improvements to clang-tidy
--------------------------
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===================================================================
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.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_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances where the user depends on internal details and warns them
+/// against doing so.
+/// This check should not be run on internal Abseil files or Abseil source code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-internal-deps.html
+class NoInternalDepsCheck : public ClangTidyCheck {
+public:
+ NoInternalDepsCheck(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_NOINTERNALDEPSCHECK_H
Index: clang-tidy/abseil/NoInternalDepsCheck.cpp
===================================================================
--- clang-tidy/abseil/NoInternalDepsCheck.cpp
+++ clang-tidy/abseil/NoInternalDepsCheck.cpp
@@ -0,0 +1,46 @@
+//===--- NoInternalDepsCheck.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 "NoInternalDepsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void NoInternalDepsCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // TODO(hugoeg): refactor matcher to be configurable or just match on any internal access from outside the enclosing namespace.
+
+ Finder->addMatcher(
+ nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(
+ matchesName("internal"),
+ hasParent(namespaceDecl(hasName("absl")))))))
+ .bind("InternalDep"),
+ this);
+}
+
+void NoInternalDepsCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *InternalDependency =
+ Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("InternalDep");
+
+ diag(InternalDependency->getBeginLoc(),
+ "depends upon internal implementation details, which violates the "
+ "Abseil compatibilty guidelines. See "
+ "https://abseil.io/about/compatibility";);
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
+ NoInternalDepsCheck.cpp
StringFindStartswithCheck.cpp
LINK_LIBS
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tidy/abseil/AbseilTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "NoInternalDepsCheck.h"
#include "StringFindStartswithCheck.h"
namespace clang {
@@ -19,6 +20,8 @@
class AbseilModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<NoInternalDepsCheck>(
+ "abseil-no-internal-deps");
CheckFactories.registerCheck<StringFindStartswithCheck>(
"abseil-string-find-startswith");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
