[clang-tdiy] Add header file extension configuration support.

[hokein]

Summary

• Add a HeaderFileExtensions check option in misc-definitions-in-headers, google-build-namespaces and google-global-names-in-headers.

[aaron.ballman]

Public-facing documentation should also be updated to specify the new option for the various checkers.

[aaron.ballman]

clang-tidy/ClangTidy.h

55

"in both options" -> "in either list"

57

Any reason for Default to not be a StringRef (I assume this is always going to be a hard-coded string literal in callers)? At the very least, it should be a const std::string &.

clang-tidy/ClangTidyOptions.cpp

269

This code seems like it will fail if there are spaces in the extension list, like .h, .hxx instead of .h,.hxx. I think the list, as given by the end user, should be processed into a vector of sorts so that things can be normalized (entries without a period can be diagnosed, spaces removed, etc).

270

clang-format the patch.

clang-tidy/ClangTidyOptions.h

216

endsWithHeaderFileExtension instead? However, given that uses of this all start with a SourceLocation, I wonder if that makes for a cleaner API: isLocInHeaderFile(SourceLocation, <Extensions>);

Also, how does this work if I want to include an extension-less file as the header file "extension?" It would be plausible if the extensions were passed in as a list, but as it stands it doesn't seem possible without weird conventions like leaving a blank in the list (e.g., .h,,.hpp), which seems error-prone.

Also, I'm not certain what I can pass in. The documentation should be updated to state whether these extensions are intended to include the ".".

[hokein]

clang-tidy/ClangTidyOptions.h

216

endsWithHeaderFileExtension instead? However, given that uses of this all start with a SourceLocation, I wonder if that makes for a cleaner API: isLocInHeaderFile(SourceLocation, <Extensions>);

Using SourceLocation only is not enough to retrieve the belonging file name (we need SourceManager too).

Also, how does this work if I want to include an extension-less file as the header file "extension?" It would be plausible if the extensions were passed in as a list, but as it stands it doesn't seem possible without weird conventions like leaving a blank in the list (e.g., .h,,.hpp), which seems error-prone.

Yeah, for extensions-less header file, you can pass the string like .h,,.hpp, which is a bit of weird. Do you have a better idea here? Passing a string into header-file-extensions seems the most reasonable choice.

[alexfh]

Thank you for the patch! I have a few concerns though. See inline comments.

clang-tidy/ClangTidy.cpp

348

There's no need to declare a vector: for (const string &s : {"a", "b", "c"}) ....

Also, I don't think it makes sense to create a vector of two elements and run a loop over it instead of duplicating three lines of code.

clang-tidy/ClangTidy.h

54

We need to explicitly specify the order in which the options are considered: local, global, default.

clang-tidy/ClangTidyOptions.cpp

269

It's rather inefficient to split the string each time the function is called. If even we needed this function, we would better pass an ArrayRef<StringRef>. But even better, we can use llvm::StringSet and get rid of this function at all (we might need a helper function to parse a comma-separated list of extensions to a StringSet though).

clang-tidy/ClangTidyOptions.h

216

This function doesn't belong here. I'm also not sure we need this function at all. First, it's ineffective to split the string containing the list of extensions each time. Second, if we store a set of extensions, then we can just search for the actual file extension in this set.

clang-tidy/google/GlobalNamesInHeadersCheck.h

23

nit: s/There is one option:/The check supports these options:/

24

s/the file extensions that regard as header file/a comma-separated list of filename extensions of header files/

I don't think this list should include the period. It's better to just store extensions as returned by llvm::sys::path::extension(), so that it can be simply looked up in a set.

Also, it's common to indent lists, so please insert two spaces at the beginning of this line and the next one.

33

As mentioned earlier, this can be a set of strings (e.g. llvm::StringSet).

clang-tidy/misc/DefinitionsInHeadersCheck.cpp

22

s/useHeaderFileExtension/usesHeaderFileExtension/

clang-tidy/tool/ClangTidyMain.cpp

78

We don't need a command-line flag for this. The regular way to configure clang-tidy is .clang-tidy files. However, if needed, check options can be configured from the command line via the -config= option.

[alexfh]

clang-tidy/ClangTidyOptions.h

216

isLocInHeaderFile(SourceLocation, ...) is a nice idea, but we'd need to be more specific: either isExpansionLocInHeaderFile(SourceLoc, ...) or isSpellingLocInHeaderFile(SourceLoc, ...) (or both).

[hokein]

clang-tidy/tool/ClangTidyMain.cpp

78

From my understanding, it is a global option for header-file-extensions, right? If we remove this command-line flag, there is only local header-file-extensions option remained for particular checks.

[alexfh]

clang-tidy/tool/ClangTidyMain.cpp

78

Both local and global options reside in CheckOptions and can be configured in the same way using either a .clang-tidy file or the -config= command-line option. The only difference between local and global options is that the name of the latter is not prefixed with the "CheckName.". Makes sense?

[aaron.ballman]

clang-tidy/ClangTidyOptions.h

216

Yeah, for extensions-less header file, you can pass the string like .h,,.hpp, which is a bit of weird. Do you have a better idea here? Passing a string into header-file-extensions seems the most reasonable choice.

I thought those user configurations from the command line were in YAML or JSON format, those both have the notion of lists, don't they? I would imagine this would take a SmallVectorImpl<StringRef/std::string> for the list of extensions.

216

isLocInHeaderFile(SourceLocation, ...) is a nice idea, but we'd need to be more specific: either isExpansionLocInHeaderFile(SourceLoc, ...) or isSpellingLocInHeaderFile(SourceLoc, ...) (or both).

That's true, and I would think both are reasonable to add. I rather prefer that as an API instead of passing around file names as strings, personally.

clang-tidy/tool/ClangTidyMain.cpp

78

Both local and global options reside in CheckOptions and can be configured in the same way using either a .clang-tidy file or the -config= command-line option. The only difference between local and global options is that the name of the latter is not prefixed with the "CheckName.". Makes sense?

Ah, thank you for that explanation. I learned something new today. ;-)

[alexfh]

clang-tidy/ClangTidyOptions.h

216

User configurations are stored in YAML, however, CheckOptions is a map of strings to strings, so we can't use YAML lists to represent lists of extensions.
I personally see nothing wrong with ",.h,.hh,.hpp" for example, to represent the list of extensions (<empty> being the first one makes it somewhat stand out and provided there's a good documentation, this shouldn't be too confusing).

[aaron.ballman]

clang-tidy/ClangTidyOptions.h

216

I personally see nothing wrong with ",.h,.hh,.hpp" for example, to represent the list of extensions (<empty> being the first one makes it somewhat stand out and provided there's a good documentation, this shouldn't be too confusing).

I find it to be more clever than intuitive. If it were not user-facing, I would be less concerned. I don't think users should have to read documentation to figure out the *syntax* of how to pass options if we can at all avoid it. ;-)

Regardless, I would like to separate the two concepts -- there's the way we expose the option to the users, and there's our internal APIs that we call. I don't think the internal API has to take such an awkward thing directly just because the user-facing option has to be that way currently.

[alexfh]

clang-tidy/ClangTidyOptions.h

216

This aligns with what I wrote in my first comment on this:

This function doesn't belong here. I'm also not sure we need this function at all. First, it's ineffective to split the string containing the list of extensions each time. Second, if we store a set of extensions, then we can just search for the actual file extension in this set.

I still think the function doesn't belong here. And we also shouldn't split the string each time we call it. So I suggest adding these functions to clang-tidy/utils/:

bool isExpansionLocInHeader(const? SourceManager &SM, SourceLocation Loc, const StringSet &HeaderFileExtensions);
bool isSpellingLocInHeader(const? SourceManager &SM, SourceLocation Loc, const StringSet &HeaderFileExtensions);
StringSet splitCommaSeparatedSet(StringRef S); // Or something similar.

[hokein]

Address review comments.

[hokein]

clang-tidy/ClangTidy.h

64

Makes sense. I just refer from std::string get(StringRef LocalName, std::string Default) const. Also change get to use StringRef.

clang-tidy/tool/ClangTidyMain.cpp

78

I see it.

[hokein]

Add extension-less header file doc.

[aaron.ballman]

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

48 ↗

(On Diff #45006)

One thing we may want to reconsider for this function is using comma as a delimiter. We have another in-flight review which has to use semi-colons for a checker option list because commas are part of the syntax for a type description (think Foo<A, B>). It would be really nice to have one utility for parsing lists of information from checker options, and for that to use the same delimiter everywhere so that users aren't confused by using commas in some places and semi-colons in others.

[hokein]

clang-tidy/ClangTidyOptions.cpp

269

Now it's tolerant of whitespace around the commas.

[hokein]

Format code style.

[aaron.ballman]

clang-tidy/google/GlobalNamesInHeadersCheck.h

25

s/includ/include

Is "." tolerated in the extension? It sounds like you can write: h,.hpp,hxx and it will work? Best to clarify the requirements (here and elsewhere). I don't have a strong opinion on whether the extension should include the full stop or not.

27

What if you only want to match extensionless header files?

clang-tidy/google/UnnamedNamespaceInHeaderCheck.h

24

s/includ/include

Same comment about use of ".".

clang-tidy/misc/DefinitionsInHeadersCheck.h

36

s/includ/include

Also, same comments about the full stop.

[hokein]

Update comments.

[hokein]

clang-tidy/google/GlobalNamesInHeadersCheck.h

25

The "." is not tolerated in the extension. Having updated comments.

27

As the comment states, using an empty string "" only match extension-less header files.

[aaron.ballman]

Mostly looks good, except I do still have the question about list delimiters that I'd like to hear some thoughts on. My primary concern is that I hope to avoid having different separators for different lists, and I wonder whether we want a common "parse a list of stuff" interface for the options that then gets used by functions like parseHeaderFileExtensions() to do sanitization of the list as-needed.

[hokein]

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

49 ↗

(On Diff #45349)

I'm +1 on keeping the consistence of list option.

I also check the option in clang-tidy checks, seems that this is the first time to introduce list option into clang-tidy. So ; is fine to me.

[hokein]

Change delimeter from "," to ";".

[aaron.ballman]

LGTM, thank you for working on this!

[hokein]

ping @alexfh.

[alexfh]

Sorry for the delay. A few more comments.

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

30

That's not the best way to report errors, especially, when clang-tidy is used as a library. There are multiple possibilities, e.g. add a callback to ClangTidyContext (or a method to ClangTidyCheck) to report a problem with configuration, or move to two-stage initialization of checks, or something similar. No need to change this now, but please add a FIXME to find a more suitable way to handle invalid configuration options.

56

No need for the parentheses around Result.SourceManager.

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

50 ↗

(On Diff #45852)

seems that this is the first time to introduce list option into clang-tidy.

Comma is used as a delimiter in two more checks' options: clang-tidy/modernize/UseNullptrCheck.cpp and clang-tidy/misc/AssertSideEffectCheck.cpp. It's also used on the command line for the -checks and -warnings-as-errors options. I'm not sure what to do here, if we want consistency. For now, I'd rather switch back to comma and maybe provide a more generic facility to parse lists from clang-tidy options, which could also handle both separators in a smart way (e.g. when the list contains semicolons, use only them - this will allow listing entries like "A<B,T>", otherwise use comma; maybe also support some other separator, say | for cases when we need to list entries containing semicolons).

clang-tidy/utils/HeaderFileExtensionsUtils.h

23 ↗

(On Diff #45852)

That's an unnecessary long and too specific namespace name. I'd go for just utils, at least for now.

33 ↗

(On Diff #45852)

Don't add unused functions. When someone needs them, they can add them.

[hokein]

Address Alex's comments.

[hokein]

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

51 ↗

(On Diff #46897)

Thanks for the explanation.
Now I add delimiter parameter to this function to allow developers to set specified character.

clang-tidy/utils/HeaderFileExtensionsUtils.h

34 ↗

(On Diff #46897)

It is used in UnnamedNamespaceInHeaderCheck.

[alexfh]

Looks good with a couple of comments. Thanks for working on this!

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

45 ↗

(On Diff #46897)

No need for llvm:: here.

46 ↗

(On Diff #46897)

Actually, this could be changed to a more generic parseStringSetOption or something similar. Let's do this in a follow up.

But for now we need to change the output parameter type to llvm::SmallSetImpl<StringRef> to avoid hardcoding the small size template argument. The typedef is then not useful anymore.

clang-tidy/utils/HeaderFileExtensionsUtils.h

34 ↗

(On Diff #46897)

Ah, missed this somehow. Then it's fine. I'm not sure though, whether PresumedLocation is the right thing in that check, but we can figure this out later.

[alexfh]

clang-tidy/misc/DefinitionsInHeadersCheck.cpp

44

It's common to use just FIXME:, without the reference to the author.

[hokein]

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

46 ↗

(On Diff #46897)

But for now we need to change the output parameter type to llvm::SmallSetImpl<StringRef> to avoid hardcoding the small size template argument. The typedef is then not useful anymore.

I can't find llvm::SmallSetImpl or any similar Set without size template argument.

[alexfh]

clang-tidy/utils/HeaderFileExtensionsUtils.cpp

46 ↗

(On Diff #46897)

Hm, indeed SmallSet has no base class unlike SmallVector, for example. Ok, then seems good for now.

[hokein]

Update.

[alexfh]

Still looks good.

[alexfh]

A couple of post-commit comments.

clang-tools-extra/trunk/clang-tidy/google/GlobalNamesInHeadersCheck.h

24 ↗

(On Diff #47007)

BTW, this belongs to the user-facing doc, so please move this to the .rst file. Same for other checks.

clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp

53 ↗

(On Diff #47007)

BTW, you should be able to use a range-based for loop.

clang-tidy/ClangTidy.h

public:
 
   /// \brief Read a named option from the \c Context.
   ///
   /// Reads the option with the check-local name \p LocalName from the
   /// \c CheckOptions. If the corresponding key is not present, returns
   /// \p Default.
   std::string get(StringRef LocalName, std::string Default) const;
 
+  /// \brief Read a named option from the \c Context.
+  ///
+  /// Reads the option with the check-local name \p LocalName from local or
+  /// global \c CheckOptions. If the option is not present in both options,
+  /// returns Default.
+  std::string getLocalOrGlobal(StringRef LocalName, std::string Default) const;
+
   /// \brief Read a named option from the \c Context and parse it as an integral
   /// type \c T.
   ///
   /// Reads the option with the check-local name \p LocalName from the
   /// \c CheckOptions. If the corresponding key is not present, returns
   /// \p Default.
   template <typename T>
   typename std::enable_if<std::is_integral<T>::value, T>::type
   get(StringRef LocalName, T Default) const {
     std::string Value = get(LocalName, "");
     T Result = Default;
     if (!Value.empty())
       StringRef(Value).getAsInteger(10, Result);
     return Result;

clang-tidy/ClangTidy.cpp

 
 std::string OptionsView::get(StringRef LocalName, std::string Default) const {
   const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
   if (Iter != CheckOptions.end())
     return Iter->second;
   return Default;
 }
 
+std::string OptionsView::getLocalOrGlobal(
+    StringRef LocalName, std::string Default) const {
+  std::vector<std::string> Names = { NamePrefix + LocalName.str(),
+                                     LocalName.str() };
+  for (const auto &Name : Names) {
+    auto Iter = CheckOptions.find(Name);
+    if (Iter != CheckOptions.end())
+      return Iter->second;
+  }
+  return Default;
+}
+
 void OptionsView::store(ClangTidyOptions::OptionMap &Options,
                         StringRef LocalName, StringRef Value) const {
   Options[NamePrefix + LocalName.str()] = Value;
 }
 
 void OptionsView::store(ClangTidyOptions::OptionMap &Options,
                         StringRef LocalName, int64_t Value) const {
   store(Options, LocalName, llvm::itostr(Value));

clang-tidy/ClangTidyOptions.h

protected:
   ClangTidyOptions OverrideOptions;
   ConfigFileHandlers ConfigHandlers;
 };
 
 /// \brief Parses LineFilter from JSON and stores it to the \p Options.
 std::error_code parseLineFilter(llvm::StringRef LineFilter,
                                 ClangTidyGlobalOptions &Options);
 
+/// \brief Checks whether file extension of Filename ends with
+/// HeaderFileExtensions.
+bool endWithHeaderFileExtensions(llvm::StringRef FileName,
+                                 llvm::StringRef HeaderFileExtensions);
+
 /// \brief Parses configuration from JSON and returns \c ClangTidyOptions or an
 /// error.
 llvm::ErrorOr<ClangTidyOptions> parseConfiguration(llvm::StringRef Config);
 
 /// \brief Serializes configuration to a YAML-encoded string.
 std::string configurationAsText(const ClangTidyOptions &Options);
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H

clang-tidy/ClangTidyOptions.cpp

 /// \brief Parses -line-filter option and stores it to the \c Options.
 std::error_code parseLineFilter(StringRef LineFilter,
                                 clang::tidy::ClangTidyGlobalOptions &Options) {
   llvm::yaml::Input Input(LineFilter);
   Input >> Options.LineFilter;
   return Input.error();
 }
 
+bool endWithHeaderFileExtensions(llvm::StringRef Filename,
+                                 llvm::StringRef HeaderFileExtensions) {
+  SmallVector<llvm::StringRef, 5> Suffixes;
+  HeaderFileExtensions.split(Suffixes, ',');
+  for (const auto& Suffix : Suffixes) {
+    if (Suffix.empty() && llvm::sys::path::extension(Filename).empty())
+      return true;
+    if (Filename.endswith(Suffix))
+      return true;
+  }
+  return false;
+}
+
 llvm::ErrorOr<ClangTidyOptions> parseConfiguration(StringRef Config) {
   llvm::yaml::Input Input(Config);
   ClangTidyOptions Options;
   Input >> Options;
   if (Input.error())
     return Input.error();
   return Options;
 }

clang-tidy/google/GlobalNamesInHeadersCheck.h

 
 #include "../ClangTidy.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 in header files.
+/// Right now it only triggers on using declarations and directives.
+///
+/// There is one option:
+/// - `HeaderFileExtensions`: the file extensions that regard as header file.
+///   ".h" by default.
 class GlobalNamesInHeadersCheck : public ClangTidyCheck {
 public:
-  GlobalNamesInHeadersCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  GlobalNamesInHeadersCheck(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 HeaderFileExtensions;
 };
 
 } // namespace readability
 } // namespace google
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBALNAMESINHEADERSCHECK_H

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace google {
 namespace readability {
 
+GlobalNamesInHeadersCheck::GlobalNamesInHeadersCheck(
+    StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        HeaderFileExtensions(Options.getLocalOrGlobal("HeaderFileExtensions",
+                                                      ".h")) {}
+
+void GlobalNamesInHeadersCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", HeaderFileExtensions);
+}
+
 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.
     StringRef Filename = Result.SourceManager->getFilename(
         Result.SourceManager->getSpellingLoc(D->getLocStart()));
 
-    if (!Filename.endswith(".h"))
+    if (!endWithHeaderFileExtensions(Filename, 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.

clang-tidy/google/UnnamedNamespaceInHeaderCheck.h

 
 namespace clang {
 namespace tidy {
 namespace google {
 namespace build {
 
 /// Finds anonymous namespaces in headers.
 ///
+/// There is one option:
+/// - `HeaderFileExtensions`: the file extensions that regard as header file.
+///   ".h,.hh,.hpp,.hxx" by default.
+///
 /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Namespaces#Namespaces
 ///
 /// Corresponding cpplint.py check name: 'build/namespaces'.
 class UnnamedNamespaceInHeaderCheck : public ClangTidyCheck {
 public:
-  UnnamedNamespaceInHeaderCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UnnamedNamespaceInHeaderCheck(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 HeaderFileExtensions;
 };
 
 } // namespace build
 } // namespace google
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_UNNAMEDNAMESPACEINHEADERCHECK_H

clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp

 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace google {
 namespace build {
 
+UnnamedNamespaceInHeaderCheck::UnnamedNamespaceInHeaderCheck(
+    StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        HeaderFileExtensions(Options.getLocalOrGlobal("HeaderFileExtensions",
+                                                      ".h,.hh,.hpp,.hxx")) {}
+
+void UnnamedNamespaceInHeaderCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", HeaderFileExtensions);
+}
+
 void UnnamedNamespaceInHeaderCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
   if (getLangOpts().CPlusPlus)
     Finder->addMatcher(namespaceDecl(isAnonymous()).bind("anonymousNamespace"),
                        this);
 }
 
 void
 UnnamedNamespaceInHeaderCheck::check(const MatchFinder::MatchResult &Result) {
   SourceManager *SM = Result.SourceManager;
   const auto *N = Result.Nodes.getNodeAs<NamespaceDecl>("anonymousNamespace");
   SourceLocation Loc = N->getLocStart();
   if (!Loc.isValid())
     return;
 
-  // Look if we're inside a header, check for common suffixes only.
-  // TODO: Allow configuring the set of file extensions.
   StringRef FileName = SM->getPresumedLoc(Loc).getFilename();
-  if (FileName.endswith(".h") || FileName.endswith(".hh") ||
-      FileName.endswith(".hpp") || FileName.endswith(".hxx"))
+  if (endWithHeaderFileExtensions(FileName, HeaderFileExtensions))
     diag(Loc, "do not use unnamed namespaces in header files");
 }
 
 } // namespace build
 } // namespace google
 } // namespace tidy
 } // namespace clang

clang-tidy/misc/DefinitionsInHeadersCheck.h

 namespace misc {
 
 // Finds non-extern non-inline function and variable definitions in header
 // files, which can lead to potential ODR violations.
 //
 // There is one option:
 // - `UseHeaderFileExtension`: Whether to use file extension (h, hh, hpp, hxx)
 //   to distinguish header files. True by default.
+// - `HeaderFileExtensions`: the file extensions that regard as header file.
+//   ",.h,.hh,.hpp,.hxx" by default.
 //
 // For the user-facing documentation see:
 // http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html
 class DefinitionsInHeadersCheck : public ClangTidyCheck {
 public:
   DefinitionsInHeadersCheck(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 bool UseHeaderFileExtension;
+  const std::string HeaderFileExtensions;
 };
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H

clang-tidy/misc/DefinitionsInHeadersCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 namespace {
 
-AST_MATCHER(NamedDecl, isHeaderFileExtension) {
+AST_MATCHER_P(NamedDecl, useHeaderFileExtension,
+              StringRef, HeaderFileExtensions) {
   SourceManager& SM = Finder->getASTContext().getSourceManager();
   SourceLocation ExpansionLoc = SM.getExpansionLoc(Node.getLocStart());
   StringRef Filename = SM.getFilename(ExpansionLoc);
-  return Filename.endswith(".h") || Filename.endswith(".hh") ||
-         Filename.endswith(".hpp") || Filename.endswith(".hxx") ||
-         llvm::sys::path::extension(Filename).empty();
+  return endWithHeaderFileExtensions(Filename, HeaderFileExtensions);
 }
 
 } // namespace
 
 DefinitionsInHeadersCheck::DefinitionsInHeadersCheck(
     StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context),
-        UseHeaderFileExtension(Options.get("UseHeaderFileExtension", true)) {}
+        UseHeaderFileExtension(Options.get("UseHeaderFileExtension", true)),
+        HeaderFileExtensions(Options.getLocalOrGlobal("HeaderFileExtensions",
+                                                      ",.h,.hh,.hpp,.hxx")) {}
 
 void DefinitionsInHeadersCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "UseHeaderFileExtension", UseHeaderFileExtension);
+  Options.store(Opts, "HeaderFileExtensions", HeaderFileExtensions);
 }
 
 void DefinitionsInHeadersCheck::registerMatchers(MatchFinder *Finder) {
   if (UseHeaderFileExtension) {
     Finder->addMatcher(
         namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
-                  isHeaderFileExtension()).bind("name-decl"),
+                  useHeaderFileExtension(HeaderFileExtensions))
+             .bind("name-decl"),
         this);
   } else {
     Finder->addMatcher(
         namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
-                  anyOf(isHeaderFileExtension(),
-                        unless(isExpansionInMainFile()))).bind("name-decl"),
+                  anyOf(useHeaderFileExtension(HeaderFileExtensions),
+                        unless(isExpansionInMainFile())))
+            .bind("name-decl"),
         this);
   }
 }
 
 void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
   // C++ [basic.def.odr] p6:
   // There can be more than one definition of a class type, enumeration type,
   // inline function with external linkage, class template, non-static function

clang-tidy/tool/ClangTidyMain.cpp

HeaderFilter("header-filter",
                       "headers to output diagnostics from. Diagnostics\n"
                       "from the main file of each translation unit are\n"
                       "always displayed.\n"
                       "Can be used together with -line-filter.\n"
                       "This option overrides the value read from a\n"
                       ".clang-tidy file."),
              cl::init(""), cl::cat(ClangTidyCategory));
 
+static cl::opt<std::string>
+HeaderFileExtensions("header-file-extensions",
+                     cl::desc("File extensions that regard as header file.\n"
+                              "The format is a comma-list of file extensions\n"
+                              "with '.' prefix:\n"
+                              "  '.h,.hh,.hpp,.hxx'\n"),
+                     cl::init(".h,.hh,.hpp,.hxx"), cl::cat(ClangTidyCategory));
+
 static cl::opt<bool>
     SystemHeaders("system-headers",
                   cl::desc("Display the errors from system headers."),
                   cl::init(false), cl::cat(ClangTidyCategory));
 static cl::opt<std::string>
 LineFilter("line-filter",
            cl::desc("List of files with line ranges to filter the\n"
                     "warnings. Can be used together with\n"
static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider() {
   if (Checks.getNumOccurrences() > 0)
     OverrideOptions.Checks = Checks;
   if (HeaderFilter.getNumOccurrences() > 0)
     OverrideOptions.HeaderFilterRegex = HeaderFilter;
   if (SystemHeaders.getNumOccurrences() > 0)
     OverrideOptions.SystemHeaders = SystemHeaders;
   if (AnalyzeTemporaryDtors.getNumOccurrences() > 0)
     OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
+  if (HeaderFileExtensions.getNumOccurrences() > 0)
+    OverrideOptions.CheckOptions["HeaderFileExtensions"] = HeaderFileExtensions;
 
   if (!Config.empty()) {
     if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
             parseConfiguration(Config)) {
       return llvm::make_unique<DefaultOptionsProvider>(
           GlobalOptions, ClangTidyOptions::getDefaults()
                              .mergeWith(DefaultOptions)
                              .mergeWith(*ParsedConfig)