[clang-tools-extra] 020ed67 - [clang-tidy] Fix check for Abseil internal namespace access

[Eric Fiselier via cfe-commits]

Author: Eric Fiselier
Date: 2020-01-21T15:21:53-05:00
New Revision: 020ed6713d889a95f8c98d7725c87b458d99f6b3

URL: https://github.com/llvm/llvm-project/commit/020ed6713d889a95f8c98d7725c87b458d99f6b3
DIFF: https://github.com/llvm/llvm-project/commit/020ed6713d889a95f8c98d7725c87b458d99f6b3.diff

LOG: [clang-tidy] Fix check for Abseil internal namespace access

This change makes following modifications:
  * If reference originated from macro expansion, we report location inside of
    the macro instead of location where macro is referenced.
  * If for any reason deduced location is not correct we silently ignore it.

Patch by Gennadiy Rozental (rogeeff at google.com)
Reviewed as https://reviews.llvm.org/D72484

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
    clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
index dcb8585d55a9..3ce937a75649 100644
--- a/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
@@ -37,7 +37,13 @@ void NoInternalDependenciesCheck::check(const MatchFinder::MatchResult &Result)
   const auto *InternalDependency =
       Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("InternalDep");
 
-  diag(InternalDependency->getBeginLoc(),
+  SourceLocation LocAtFault =
+      Result.SourceManager->getSpellingLoc(InternalDependency->getBeginLoc());
+
+  if (!LocAtFault.isValid())
+    return;
+
+  diag(LocAtFault,
        "do not reference any 'internal' namespaces; those implementation "
        "details are reserved to Abseil");
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
index 6014278e2606..31798661a80f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
@@ -15,6 +15,8 @@ template <class P> P InternalTemplateFunction(P a) {}
 
 namespace container_internal {
 struct InternalStruct {};
+
+template <typename T> struct InternalTemplate {};
 } // namespace container_internal
 } // namespace absl
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
index 272d0060bdb7..2949d7fdd027 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
@@ -44,5 +44,18 @@ std::string Str = absl::StringsFunction("a");
 void MacroUse() {
   USE_INTERNAL(Function); // no-warning
   USE_EXTERNAL(Function);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  // CHECK-MESSAGES: :[[@LINE-5]]:25: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
 }
+
+class A : absl::container_internal::InternalStruct {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template <typename T>
+class B : absl::container_internal::InternalTemplate<T> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template <typename T> class C : absl::container_internal::InternalTemplate<T> {
+public:
+  template <typename U> static C Make(U *p) { return C{}; }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:33: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil