hugoeg updated this revision to Diff 161263. hugoeg marked 5 inline comments as done. hugoeg added a comment.
applied corrections form comments 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/Inputs/absl/strings/str_cat.h 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,33 @@ +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t + +#include "Inputs/absl/strings/str_cat.h" + +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 +}; +i hate this +namespace absl { + void OpeningNamespace() { + strings_internal::InternalFunction(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil + } +} // namespace absl + +void CorrectUsage() { + std::string Str = absl::StringsFunction("a"); + absl::SomeContainer b; +} + +namespace absl { + SomeContainer b; + std::string Str = absl::StringsFunction("a"); +} // namespace absl Index: test/clang-tidy/Inputs/absl/strings/str_cat.h =================================================================== --- test/clang-tidy/Inputs/absl/strings/str_cat.h +++ test/clang-tidy/Inputs/absl/strings/str_cat.h @@ -0,0 +1,33 @@ +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 Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ ================= .. toctree:: + abseil-no-internal-deps abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 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,21 @@ +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 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(); + class foo{ + friend struct absl::container_internal::faa; + }; + absl::memory_internal::MakeUniqueResult(); Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,7 +57,10 @@ Improvements to clang-tidy -------------------------- -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 include-fixer ----------------------------- 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,60 @@ +//===--- 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 { + +bool IsInAbseilFile(const SourceManager &manager, SourceLocation loc) { + if (loc.isInvalid()) return false; + const FileEntry *FileEntry = manager.getFileEntryForID(manager.getFileID(loc)); + if (!FileEntry) return false; + StringRef Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); + return RE.match(Filename); +} + +void NoInternalDepsCheck::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"))))))) + .bind("InternalDep"), + this); +} + +void NoInternalDepsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *InternalDependency = + Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("InternalDep"); + + if (!IsInAbseilFile(*Result.SourceManager, + InternalDependency->getBeginLoc())) { + + diag(InternalDependency->getBeginLoc(), + "do not reference any 'internal' namespaces; those implementation " + "details are reserved to Abseil"); + } +} + +} // 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