Skip to content

Commit

Permalink
[clang-tidy] Add abseil-no-internal-dependencies check
Browse files Browse the repository at this point in the history
Finds instances where the user depends on internal details and warns them against doing so.
Should not be run on internal Abseil files or Abseil source code.

Patch by hugoeg!

Differential Revision: https://reviews.llvm.org/D50542

llvm-svn: 340928
  • Loading branch information
JonasToth committed Aug 29, 2018
1 parent 0ef60da commit 51aadb4
Show file tree
Hide file tree
Showing 10 changed files with 196 additions and 2 deletions.
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
Expand Up @@ -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"
Expand All @@ -28,6 +29,8 @@ class AbseilModule : public ClangTidyModule {
"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");
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
Expand Up @@ -4,6 +4,7 @@ add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
DurationDivisionCheck.cpp
FasterStrsplitDelimiterCheck.cpp
NoInternalDependenciesCheck.cpp
NoNamespaceCheck.cpp
RedundantStrcatCallsCheck.cpp
StringFindStartswithCheck.cpp
Expand Down
@@ -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
36 changes: 36 additions & 0 deletions 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
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Expand Up @@ -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.

Expand Down
@@ -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
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Expand Up @@ -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
Expand Down
@@ -1 +1,6 @@
namespace absl {}
namespace absl {
namespace base_internal {
void InternalFunction() {}
} // namespace base_internal
} //namespace absl
void DirectAccess2() { absl::base_internal::InternalFunction(); }
@@ -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
@@ -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

0 comments on commit 51aadb4

Please sign in to comment.