diff options
10 files changed, 196 insertions, 2 deletions
diff --git a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp index e7e0a2bdc3c..b3de5c0799e 100644 --- a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "DurationDivisionCheck.h" #include "FasterStrsplitDelimiterCheck.h" +#include "NoInternalDependenciesCheck.h" #include "NoNamespaceCheck.h" #include "RedundantStrcatCallsCheck.h" #include "StringFindStartswithCheck.h" @@ -28,6 +29,8 @@ public: "abseil-duration-division"); CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>( "abseil-faster-strsplit-delimiter"); + CheckFactories.registerCheck<NoInternalDependenciesCheck>( + "abseil-no-internal-dependencies"); CheckFactories.registerCheck<NoNamespaceCheck>("abseil-no-namespace"); CheckFactories.registerCheck<RedundantStrcatCallsCheck>( "abseil-redundant-strcat-calls"); diff --git a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt index f36c0d979ec..e268b34bf81 100644 --- a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangTidyAbseilModule AbseilTidyModule.cpp DurationDivisionCheck.cpp FasterStrsplitDelimiterCheck.cpp + NoInternalDependenciesCheck.cpp NoNamespaceCheck.cpp RedundantStrcatCallsCheck.cpp StringFindStartswithCheck.cpp diff --git a/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp new file mode 100644 index 00000000000..cb77e95fa81 --- /dev/null +++ b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp @@ -0,0 +1,48 @@ +//===--- NoInternalDependenciesCheck.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 "NoInternalDependenciesCheck.h" +#include "AbseilMatcher.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace abseil { + +void NoInternalDependenciesCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + // TODO: 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")))))), + unless(isInAbseilFile())) + .bind("InternalDep"), + this); +} + +void NoInternalDependenciesCheck::check(const MatchFinder::MatchResult &Result) { + const auto *InternalDependency = + Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("InternalDep"); + + diag(InternalDependency->getBeginLoc(), + "do not reference any 'internal' namespaces; those implementation " + "details are reserved to Abseil"); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.h b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.h new file mode 100644 index 00000000000..7e659df5908 --- /dev/null +++ b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.h @@ -0,0 +1,36 @@ +//===--- NoInternalDependenciesCheck.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. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-internal-dependencies.html +class NoInternalDependenciesCheck : public ClangTidyCheck { +public: + NoInternalDependenciesCheck(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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1e3a74abedc..c660bc762a1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -70,6 +70,11 @@ Improvements to clang-tidy Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the delimiter is a single character string literal and replaces with a character. +- New :doc:`abseil-no-internal-dependencies + <clang-tidy/checks/abseil-no-internal-dependencies>` check. + + Gives a warning if code using Abseil depends on internal details. + - New :doc:`abseil-no-namespace <clang-tidy/checks/abseil-no-namespace>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst b/clang-tools-extra/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst new file mode 100644 index 00000000000..bcf68ffd2d5 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst @@ -0,0 +1,24 @@ +subl.. title:: clang-tidy - abseil-no-internal-dependencies + +abseil-no-internal-dependencies +=============================== + +Warns if code using Abseil depends on internal details. If something is in a +namespace 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 violates Abseil's +compatibility guidelines and may result in breakage. See +https://abseil.io/about/compatibility for more information. + +The following cases will result in warnings: + +.. code-block:: c++ + + absl::strings_internal::foo(); + // warning triggered on this line + class foo { + friend struct absl::container_internal::faa; + // warning triggered on this line + }; + absl::memory_internal::MakeUniqueResult(); + // warning triggered on this line diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 35deccd8e96..600969fbdd6 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -6,6 +6,7 @@ Clang-Tidy Checks .. toctree:: abseil-duration-division abseil-faster-strsplit-delimiter + abseil-no-internal-dependencies abseil-no-namespace abseil-redundant-strcat-calls abseil-string-find-startswith diff --git a/clang-tools-extra/test/clang-tidy/Inputs/absl/external-file.h b/clang-tools-extra/test/clang-tidy/Inputs/absl/external-file.h index 94f6687b501..11e2e4aa96c 100644 --- a/clang-tools-extra/test/clang-tidy/Inputs/absl/external-file.h +++ b/clang-tools-extra/test/clang-tidy/Inputs/absl/external-file.h @@ -1 +1,6 @@ -namespace absl {} +namespace absl { +namespace base_internal { +void InternalFunction() {} +} // namespace base_internal +} //namespace absl +void DirectAccess2() { absl::base_internal::InternalFunction(); } diff --git a/clang-tools-extra/test/clang-tidy/Inputs/absl/strings/internal-file.h b/clang-tools-extra/test/clang-tidy/Inputs/absl/strings/internal-file.h index 94f6687b501..6014278e260 100644 --- a/clang-tools-extra/test/clang-tidy/Inputs/absl/strings/internal-file.h +++ b/clang-tools-extra/test/clang-tidy/Inputs/absl/strings/internal-file.h @@ -1 +1,33 @@ -namespace absl {} +namespace std { +struct string { + string(const char *); + ~string(); +}; +} // namespace 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 + +// should not trigger warnings because inside Abseil files +void DirectAcessInternal() { + absl::strings_internal::InternalFunction(); + absl::strings_internal::InternalTemplateFunction<std::string>("a"); +} + +class FriendUsageInternal { + friend struct absl::container_internal::InternalStruct; +}; + +namespace absl { +void OpeningNamespaceInternally() { strings_internal::InternalFunction(); } +} // namespace absl diff --git a/clang-tools-extra/test/clang-tidy/abseil-no-internal-dependencies.cpp b/clang-tools-extra/test/clang-tidy/abseil-no-internal-dependencies.cpp new file mode 100644 index 00000000000..bd56cb04df7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/abseil-no-internal-dependencies.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t, -- -- -I %S/Inputs +// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s + +#include "absl/strings/internal-file.h" +// CHECK-NOT: warning: + +#include "absl/external-file.h" +// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-dependencies] + +void DirectAcess() { + absl::strings_internal::InternalFunction(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil + + absl::strings_internal::InternalTemplateFunction<std::string>("a"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +} + +class FriendUsage { + friend struct absl::container_internal::InternalStruct; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +}; + +namespace absl { +void OpeningNamespace() { + strings_internal::InternalFunction(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +} +} // namespace absl + +// should not trigger warnings +void CorrectUsage() { + std::string Str = absl::StringsFunction("a"); + absl::SomeContainer b; +} + +namespace absl { +SomeContainer b; +std::string Str = absl::StringsFunction("a"); +} // namespace absl |
