[Clang-tidy] Add a check for definitions in the global namespace.

[bkramer]

Summary

This is prone to ODR violations and generally frowned upon in many
codebases (e.g. LLVM). The checker flags definitions, variables and
classes in the global namespace. Common false positives like extern "C"
functions are filtered, symbols with internal linkage or hidden
visibility are not warned on either.

On LLVM most of the instances are helper functions that should be just
made static or globals that belong into a namespace.

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

[bkramer]

Add relnote.

[alexfh]

Do we want to merge this check with google/GlobalNamesInHeadersCheck.cpp that targets a similar set of issues?

[bkramer]

DefinitionsInHeaders is tackling a different problem. IMO DefinitionsInHeaders is something that should be on by default everywhere, while this check for definitions in the global namespace is more of a coding style issue.

GlobalNamesInHeaders is a bit of a misnomer, it looks for using declarations in headers. I don't think it makes sense to merge this check into it either, with the new check we mostly find things exported by accident in .cpp files, it has little to do with headers.

I'm welcome to naming suggestions for this check, btw ;)

[alexfh]

In D23130#505242, @bkramer wrote:

DefinitionsInHeaders is tackling a different problem. IMO DefinitionsInHeaders is something that should be on by default everywhere, while this check for definitions in the global namespace is more of a coding style issue.

Yes, DefinitionsInHeaders is a different beast.

GlobalNamesInHeaders is a bit of a misnomer, it looks for using declarations in headers.

The google-global-names-in-headers check aims to "flag global namespace pollution in header files" including declarations of classes, functions and other entities. However, "right now it only triggers on using declarations and directives", so it's basically under-implemented. The only problem, I guess, is keeping false positives under control. The check you're proposing is somewhat close. It isn't specifically limited to headers (the google-global-names-in-headers check can run on all files if properly configured, and we can rename it to remove the -in-headers part). And the second difference is that it is limited to definitions, but I don't yet understand, why it is important, since declarations in the global namespace (except for using declarations, typedefs, etc.) will have a corresponding definition in the global namespace. I guess, these checks, if somewhat different, may be similar enough to share some implementation.

[Prazek]

clang-tidy/misc/GlobalNamespaceCheck.cpp

46 ↗

(On Diff #66716)

I think it would be better to check it in matcher.
I see that there is isExternC, but it only works for FunctionDecl, but I think that adding isExternC for VarDecl would be good

67 ↗

(On Diff #66716)

What about macros? I think you should check isMacroId on location here (don't do fixit when it is in macro, but print warning)

Also please add tests for it.

[bkramer]

In D23130#505290, @alexfh wrote:

And the second difference is that it is limited to definitions, but I don't yet understand, why it is important, since declarations in the global namespace (except for using declarations, typedefs, etc.) will have a corresponding definition in the global namespace. I guess, these checks, if somewhat different, may be similar enough to share some implementation.

My reasoning was to avoid duplicated warnings (on both declaration and definition) and false positives on headers importing C symbols. I think that the latter would trigger an explosion in false positives :(

clang-tidy/misc/GlobalNamespaceCheck.cpp

46 ↗

(On Diff #66716)

I'll fix this once we figured out where to put this check.

67 ↗

(On Diff #66716)

The FixIt isn't actually implemented. Adding FixIts for this check is something I'm planning to do, but it's not part of this patch and doesn't make sense to implement before we know that this check is actually working properly.

[aaron.ballman]

clang-tidy/misc/GlobalNamespaceCheck.cpp

72 ↗

(On Diff #66716)

ODR violations instead of ODR conflicts?

docs/clang-tidy/checks/misc-global-namespace.rst

7 ↗

(On Diff #66716)

violations instead of conflicts. Also, expanding the documentation a bit more would be useful -- not everyone understands what an ODR violation is, why it would be bad, etc. It can even be as simple as adding in an external link for more info.

test/clang-tidy/misc-global-namespace.cpp

13 ↗

(On Diff #66716)

Add a case for extern "C" int i;?

Also, isn't this a definition with external linkage in C++ (or is it only in C)? extern int i = 30; Assuming it's a definition, should this also trigger the diagnostic?

[bkramer]

• Merge google-global-names-in-headers and misc-global-namespace into a new check google-global-names.

[alexfh]

Could you run the check on LLVM and post a summary of results?

clang-tidy/google/GlobalNamesCheck.cpp

90

Is this already filtered-out by the matcher?

114

The warning explains, what's wrong, but doesn't tell why. We don't need the level of details of the documentation here, but maybe expand the message a bit to cover some of the possible effects (e.g. "this can cause ODR violations" or something like this)?

docs/ReleaseNotes.rst

62

Please mention the old check name as well.

docs/clang-tidy/checks/google-global-names.rst

4

Cut extra =s.

[bkramer]

Address review comments.

[bkramer]

In D23130#505769, @alexfh wrote:

Could you run the check on LLVM and post a summary of results?

I have a full list at https://reviews.llvm.org/P7085. It's huge.

[aaron.ballman]

For cases where the external linkage is desired, how would you go about silencing this diagnostic?

test/clang-tidy/google-global-names.cpp

14–15

This strikes me as being intentional enough to warrant not diagnosing because of the extern keyword.

[bkramer]

test/clang-tidy/google-global-names.cpp

14–15

The only case I see where this pattern is valuable is interfacing with C code. Not sure yet if we want to allow that or enforce extern "C" instead. Ideas?

an extern global in the global namespace still feels like something we should warn on :|

[aaron.ballman]

test/clang-tidy/google-global-names.cpp

14–15

Yet externs in the global namespace do happen for valid reasons (such as not breaking ABIs by putting the extern definition into a namespace or changing the language linkage) -- I'm trying to think of ways we can allow the user to silence this diagnostic in those cases. I feel like in cases where the user writes "extern", they're explicitly specifying their intent and that doesn't seem like a case to warn them about, in some regards. It would give us two ways to silence the diagnostic (well, three, but two are morally close enough):

1. Put it into a namespace
2. Slap extern on it if it is global for C++ compatibility (such as ABIs)
3. Slap extern "C" on it if it global for C compatibility

I suppose we could require extern "C++" instead of extern, but I don't think that's a particularly common use of the language linkage specifier?

[alexfh]

Benjamin, what's the plan here?

[bkramer]

Update to head

[bkramer]

In D23130#588681, @alexfh wrote:

Benjamin, what's the plan here?

I still think this check is useful, particularly for LLVM. I also don't think any of the existing review comments still apply or have ever applied in the first place, so I rebased this onto head, it's up for review again.

[aaron.ballman]

clang-tidy/google/GlobalNamesCheck.cpp

78

Is this formatting that clang-format generates?

test/clang-tidy/google-global-names.cpp

14–15

I still think that a user explicitly writing 'extern' is expecting external linkage and all that goes along with it.

19

Can you also add a test:

extern "C++" void h2() {}

I believe this will diagnose, but whether it should diagnose or not, I'm less certain of.

[alexfh]

and generally frowned upon in many codebases (e.g. LLVM)

Should it still be a part of google/? The old check was enforcing a part of the Google C++ style guide, but the new one seems to be somewhat broader. Am I mistaken?

[bkramer]

Add extern "C++" test case.

[bkramer]

In D23130#589643, @alexfh wrote:

and generally frowned upon in many codebases (e.g. LLVM)

Should it still be a part of google/? The old check was enforcing a part of the Google C++ style guide, but the new one seems to be somewhat broader. Am I mistaken?

The google style guide has a rule that code should usually be placed in a namespace. The wording there is fuzzy, but I believe this check still belongs under google/

clang-tidy/google/GlobalNamesCheck.cpp

78

Yes. I clang-formatted the entire file.

test/clang-tidy/google-global-names.cpp

14–15

I disagree. If this is a special variable to be accessed via dlopen it should be extern "C". If not it should be in a namespace.

19

Test added, doesn't diagnose.

[malcolm.parsons]

clang-tidy/google/GlobalNamesCheck.cpp

97

Should isMSVCRTEntryPoint() be checked too?

[aaron.ballman]

test/clang-tidy/google-global-names.cpp

14–15

I'm thinking more that it's not a special variable to be accessed via dlopen, but instead is a special implementation detail that doesn't need to be exposed via a header file. For instance, see ClangTidyMain.cpp. Such uses have no way to silence this check, which may be fine since it's in the Google module. I merely wanted to point it out because it does happen for valid reasons, and having a way to silence a diagnostic aside from NOLINT is nice.

[bkramer]

test/clang-tidy/google-global-names.cpp

14–15

That's an example of something that should be in a namespace. Currently it's polluting the global namespace which will break anyone who happens to use the same name for a symbol anywhere else. I think that flagging those cases is useful.

[aaron.ballman]

clang-tidy/google/GlobalNamesCheck.cpp

63

s/unless/Unless

85

I wonder if this check would have better performance by making this a local AST matcher instead? UsingDecl and UsingDirectiveDecl do not have linkage, so it may require adjusting the matcher somewhat.

97

Yes, it should.

clang-tidy/google/GoogleTidyModule.cpp

68

Given that this was shipped under the old name, I think we need to figure out our policy for how to handle this. It also comes up in D26511, so I would like us to be consistent with what we do.

test/clang-tidy/google-global-names.cpp

14–15

Okay, that's fair. I was thinking that since it was in the global namespace, it was there for a reason. After looking a bit more closely, I think you're correct, this would be a true-positive. You've convinced me and I retract my concerns. :-)

[bkramer]

• Moved external linkage check to matcher
• added msvcrt entry point check
• fixed comment typos.

[bkramer]

clang-tidy/google/GlobalNamesCheck.cpp

90

Nope.

clang-tidy/google/GoogleTidyModule.cpp

68

There was no special handling in D26511, so I left this as-is.

[malcolm.parsons]

clang-tidy/google/GoogleTidyModule.cpp

68

D26511 made the existing check documentation redirect to the new documentation.

[alexfh]

It looks like all concerns were resolved.

LG, if you still want to proceed with this patch.

[bkramer]

I'd like to, but I don't know when I find time to rebase this thing after more than a year of waiting for review.

[alexfh]

In D23130#1037256, @bkramer wrote:

I'd like to, but I don't know when I find time to rebase this thing after more than a year of waiting for review.

Sorry, it was just lost ¯\_(ツ)_/¯. I may be relying on pings from the other side too much.

[shafik]

Is this still relevant? It looks like some of these changes made it in.

clang-tidy/google/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyGoogleModule
   AvoidCStyleCastsCheck.cpp
   DefaultArgumentsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
-  GlobalNamesInHeadersCheck.cpp
+  GlobalNamesCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   MemsetZeroLengthCheck.cpp
   NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp
   StringReferenceMemberCheck.cpp
   TodoCommentCheck.cpp
   UnnamedNamespaceInHeaderCheck.cpp

clang-tidy/google/GlobalNamesCheck.h

-//===--- GlobalNamesInHeadersCheck.h - clang-tidy ---------------*- C++ -*-===//
+//===--- GlobalNamesCheck.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_GOOGLE_GLOBALNAMESINHEADERSCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESINHEADERSCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESCHECK_H
 
 #include "../ClangTidy.h"
 #include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace google {
 namespace readability {
 
-/// Flag global namespace pollution in header files.
-/// Right now it only triggers on using declarations and directives.
+/// Flag global namespace pollution.
+/// Right now it only triggers on using declarations and directives in header
+/// files and declarations and definitions of functions, classes and variables
+/// in the global namespace.
 ///
 /// The check supports these options:
 ///   - `HeaderFileExtensions`: a comma-separated list of filename extensions
 ///     of header files (the filename extensions should not contain "." prefix).
 ///     "h" by default.
 ///     For extension-less header files, using an empty string or leaving an
 ///     empty string between "," if there are other filename extensions.
-class GlobalNamesInHeadersCheck : public ClangTidyCheck {
+class GlobalNamesCheck : public ClangTidyCheck {
 public:
-  GlobalNamesInHeadersCheck(StringRef Name, ClangTidyContext *Context);
+  GlobalNamesCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   const std::string RawStringHeaderFileExtensions;
   utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace readability
 } // namespace google
 } // namespace tidy
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESINHEADERSCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESCHECK_H

clang-tidy/google/GlobalNamesCheck.cpp

+//===--- GlobalNamesCheck.cpp - clang-tidy ----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GlobalNamesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace readability {
+
+GlobalNamesCheck::GlobalNamesCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringHeaderFileExtensions(
+          Options.getLocalOrGlobal("HeaderFileExtensions", "h")) {
+  if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+                                        HeaderFileExtensions,
+                                        ',')) {
+    llvm::errs() << "Invalid header file extension: "
+                 << RawStringHeaderFileExtensions << "\n";
+  }
+}
+
+void GlobalNamesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+}
+
+void GlobalNamesCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  Finder->addMatcher(
+      namedDecl(hasDeclContext(translationUnitDecl()),
+                anyOf(usingDecl(), usingDirectiveDecl(), cxxRecordDecl(),
+                      functionDecl(unless(isExternC())),
+                      varDecl(unless(isExternC()))))
+          .bind("decl"),
+      this);
+}
+
+void GlobalNamesCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *D = Result.Nodes.getNodeAs<NamedDecl>("decl");
+
+  // If it comes from a macro, we'll assume it is fine.
+  if (D->getLocStart().isMacroID())
+    return;
+
+  if (isa<UsingDirectiveDecl>(D) || isa<UsingDecl>(D)) {
+    // Ignore if it comes from the "main" file ...
+    if (Result.SourceManager->isInMainFile(
+            Result.SourceManager->getExpansionLoc(D->getLocStart()))) {
+      // unless that file is a header.
+      if (!utils::isSpellingLocInHeaderFile(
+              D->getLocStart(), *Result.SourceManager, HeaderFileExtensions))
+        return;
+    }
+
+    if (const auto *UsingDirective = dyn_cast<UsingDirectiveDecl>(D)) {
+      if (UsingDirective->getNominatedNamespace()->isAnonymousNamespace()) {
+        // Anynoumous namespaces inject a using directive into the AST to import
+        // the names into the containing namespace.
+        // We should not have them in headers, but there is another warning for
+        // that.
+        return;
+      }
+    }
+    diag(
+        D->getLocStart(),
+        "using declarations in the global namespace in headers are prohibited");
+    return;
+  }
+
+  // Ignore decls with internal linkage.
+  if (!D->isExternallyVisible())
+    return;
+
+  if (const auto *VDecl = dyn_cast<VarDecl>(D)) {
+    // extern "C" globals need to be in the global namespace.
+    if (VDecl->isExternC())
+      return;
+  }
+
+  if (const auto *RDecl = dyn_cast<CXXRecordDecl>(D)) {
+    // Ignore C-like records. Those are often used in C wrappers and don't
+    // contain anything with linkage.
+    if (RDecl->isCLike())
+      return;
+  }
+
+  if (const auto *FDecl = dyn_cast<FunctionDecl>(D)) {
+    // main() should be in the global namespace.
+    if (FDecl->isMain())
+      return;
+
+    // Ignore overloaded operators.
+    // FIXME: Should this only ignore global operator new/delete?
+    if (FDecl->isOverloadedOperator())
+      return;
+  }
+
+  // FIXME: If we want to be really fancy we could move declaration-less
+  // definitions into a global namespace or make them static with a FixIt.
+  diag(D->getLocation(), "%0 declared in the global namespace") << D;
+}
+
+} // namespace readability
+} // namespace google
+} // namespace tidy
+} // namespace clang

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

-//===--- GlobalNamesInHeadersCheck.cpp - clang-tidy -----------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "GlobalNamesInHeadersCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Lex/Lexer.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace google {
-namespace readability {
-
-GlobalNamesInHeadersCheck::GlobalNamesInHeadersCheck(StringRef Name,
-                                                     ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context),
-      RawStringHeaderFileExtensions(
-          Options.getLocalOrGlobal("HeaderFileExtensions", "h")) {
-  if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
-                                        HeaderFileExtensions,
-                                        ',')) {
-    llvm::errs() << "Invalid header file extension: "
-                 << RawStringHeaderFileExtensions << "\n";
-  }
-}
-
-void GlobalNamesInHeadersCheck::storeOptions(
-    ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
-}
-
-void
-GlobalNamesInHeadersCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  Finder->addMatcher(
-      decl(anyOf(usingDecl(), usingDirectiveDecl()),
-           hasDeclContext(translationUnitDecl())).bind("using_decl"),
-      this);
-}
-
-void GlobalNamesInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *D = Result.Nodes.getNodeAs<Decl>("using_decl");
-  // If it comes from a macro, we'll assume it is fine.
-  if (D->getLocStart().isMacroID())
-    return;
-
-  // Ignore if it comes from the "main" file ...
-  if (Result.SourceManager->isInMainFile(
-          Result.SourceManager->getExpansionLoc(D->getLocStart()))) {
-    // unless that file is a header.
-    if (!utils::isSpellingLocInHeaderFile(
-            D->getLocStart(), *Result.SourceManager, HeaderFileExtensions))
-      return;
-  }
-
-  if (const auto* UsingDirective = dyn_cast<UsingDirectiveDecl>(D)) {
-    if (UsingDirective->getNominatedNamespace()->isAnonymousNamespace()) {
-      // Anynoumous namespaces inject a using directive into the AST to import
-      // the names into the containing namespace.
-      // We should not have them in headers, but there is another warning for
-      // that.
-      return;
-    }
-  }
-
-  diag(D->getLocStart(),
-       "using declarations in the global namespace in headers are prohibited");
-}
-
-} // namespace readability
-} // namespace google
-} // namespace tidy
-} // namespace clang

clang-tidy/google/GoogleTidyModule.cpp

 #include "../readability/BracesAroundStatementsCheck.h"
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "../readability/RedundantSmartptrGetCheck.h"
 #include "AvoidCStyleCastsCheck.h"
 #include "DefaultArgumentsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
-#include "GlobalNamesInHeadersCheck.h"
+#include "GlobalNamesCheck.h"
 #include "IntegerTypesCheck.h"
 #include "MemsetZeroLengthCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "NonConstReferences.h"
 #include "StringReferenceMemberCheck.h"
 #include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>(
         "google-runtime-memset");
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::TodoCommentCheck>(
         "google-readability-todo");
     CheckFactories
         .registerCheck<clang::tidy::readability::BracesAroundStatementsCheck>(
             "google-readability-braces-around-statements");
-    CheckFactories.registerCheck<readability::GlobalNamesInHeadersCheck>(
-        "google-global-names-in-headers");
+    CheckFactories.registerCheck<readability::GlobalNamesCheck>(
+        "google-global-names");
     CheckFactories.registerCheck<clang::tidy::readability::FunctionSizeCheck>(
         "google-readability-function-size");
     CheckFactories
         .registerCheck<clang::tidy::readability::NamespaceCommentCheck>(
             "google-readability-namespace-comments");
     CheckFactories
         .registerCheck<clang::tidy::readability::RedundantSmartptrGetCheck>(
             "google-readability-redundant-smartptr-get");

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
+- `google-global-names
+  <http://clang.llvm.org/extra/clang-tidy/checks/google-global-names.html>`_ check
+
+  Now flags functions, variables and classes in the global namespace too.
+
 - New `cppcoreguidelines-slicing
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-slicing.html>`_ check
 
   Flags slicing of member variables or vtable.
 
 - New `cppcoreguidelines-special-member-functions
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html>`_ check
 

docs/clang-tidy/checks/google-global-names.rst

-.. title:: clang-tidy - google-global-names-in-headers
+.. title:: clang-tidy - google-global-names
 
-google-global-names-in-headers
+google-global-names
 ==============================
 
 Flag global namespace pollution in header files.
-Right now it only triggers on ``using`` declarations and directives.
+Right now it only triggers on using declarations and directives in header files
+and declarations and definitions of functions, classes and variables in the
+global namespace.
 
 The check supports these options:
   - `HeaderFileExtensions`: a comma-separated list of filename extensions
     of header files (the filename extensions should not contain "." prefix).
     "h" by default.
     For extension-less header files, using an empty string or leaving an
     empty string between "," if there are other filename extensions.
 
 The relevant style guide section is
 https://google.github.io/styleguide/cppguide.html#Namespaces.

test/clang-tidy/google-global-names.cpp

+// RUN: %check_clang_tidy %s google-global-names %t
+
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'f' declared in the global namespace
+void f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'f' declared in the global namespace
+class F;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' declared in the global namespace
+class F {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' declared in the global namespace
+int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'i' declared in the global namespace
+extern int ii = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'ii' declared in the global namespace
+
+// No warnings below.
+extern "C" void g();
+extern "C" void h() {}
+
+#define VAR(v) v
+VAR(int m);
+
+extern "C" {
+void j() {}
+}
+
+struct Clike {
+  int i;
+};
+
+extern "C" int ik;
+extern "C" { int il; }
+
+void *operator new(__SIZE_TYPE__, int) { return 0; }
+void operator delete(void *, int) {}
+
+static void l() {}
+namespace {
+void m() {}
+}
+namespace x {
+void n() {}
+}
+
+int main() {}

unittests/clang-tidy/GoogleModuleTest.cpp

 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
-#include "google/GlobalNamesInHeadersCheck.h"
+#include "google/GlobalNamesCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::google;
 
 namespace clang {
 namespace tidy {
 namespace test {
 
TEST(ExplicitConstructorCheckTest, RemoveExplicitWithMacros) {
   EXPECT_EQ(
       "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
       "A(Foo);",
       runCheckOnCode<ExplicitConstructorCheck>(
           "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
           "A(Foo);"));
 }
 
-class GlobalNamesInHeadersCheckTest : public ::testing::Test {
+class GlobalNamesCheckTest : public ::testing::Test {
 protected:
   bool runCheckOnCode(const std::string &Code, const std::string &Filename) {
     static const char *const Header = "namespace std {\n"
                                       "class string {};\n"
                                       "}  // namespace std\n"
                                       "\n"
                                       "#define SOME_MACRO(x) using x\n";
     std::vector<ClangTidyError> Errors;
     std::vector<std::string> Args;
     if (!StringRef(Filename).endswith(".cpp")) {
       Args.emplace_back("-xc++-header");
     }
-    test::runCheckOnCode<readability::GlobalNamesInHeadersCheck>(
+    test::runCheckOnCode<readability::GlobalNamesCheck>(
         Header + Code, &Errors, Filename, Args);
     if (Errors.empty())
       return false;
     assert(Errors.size() == 1);
     assert(
         Errors[0].Message.Message ==
         "using declarations in the global namespace in headers are prohibited");
     return true;
   }
 };
 
-TEST_F(GlobalNamesInHeadersCheckTest, UsingDeclarations) {
+TEST_F(GlobalNamesCheckTest, UsingDeclarations) {
   EXPECT_TRUE(runCheckOnCode("using std::string;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using std::string;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
                               "using std::string;\n"
                               "}  // my_namespace\n",
                               "foo.h"));
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(std::string);", "foo.h"));
 }
 
-TEST_F(GlobalNamesInHeadersCheckTest, UsingDirectives) {
+TEST_F(GlobalNamesCheckTest, UsingDirectives) {
   EXPECT_TRUE(runCheckOnCode("using namespace std;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using namespace std;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
                               "using namespace std;\n"
                               "}  // my_namespace\n",
                               "foo.h"));
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(namespace std);", "foo.h"));
 }
 
-TEST_F(GlobalNamesInHeadersCheckTest, RegressionAnonymousNamespace) {
+TEST_F(GlobalNamesCheckTest, RegressionAnonymousNamespace) {
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
 } // namespace test
 } // namespace tidy
 } // namespace clang