[PATCH] [clang-tidy] Add support for custom configuration file names/formats.

[Daniel Jasper]

Either the patch description or the FileOptionsProvider need a lot more documentation describing the intended usage. This should roughly outline why you might want to parse different configuration files/formats and what you need to implement to do so.

================
Comment at: clang-tidy/ClangTidyOptions.cpp:183
@@ +182,3 @@
+    if (!Result) {
+      for (const auto& Handler : ConfigHandlers) {
+        Result = TryReadConfigFile(CurrentPath, Handler);
----------------
Not a strong opinion, but using the concrete type here might help understanding this later.

================
Comment at: clang-tidy/ClangTidyOptions.cpp:208-217
@@ -195,9 +207,12 @@
+llvm::ErrorOr<ClangTidyOptions> FileOptionsProvider::TryReadConfigFile(
+    StringRef Directory,
+    const FileOptionsProvider::ConfigFileHandler &ConfigHandler) {
   assert(!Directory.empty());
 
   if (!llvm::sys::fs::is_directory(Directory))
     return make_error_code(llvm::errc::not_a_directory);
 
   SmallString<128> ConfigFile(Directory);
-  llvm::sys::path::append(ConfigFile, ".clang-tidy");
+  llvm::sys::path::append(ConfigFile, ConfigHandler.first);
   DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
 
----------------
I'd move this part of the function into getOptions():
a) The first bit doesn't have to be done for each handler, but just once per directory.
b) We are frequently creating a no_such_file_or_directory error here which we are then explicitly checking later. Using an error code in something that seems like the more frequent use case seems off.

================
Comment at: clang-tidy/ClangTidyOptions.h:124
@@ -120,1 +123,3 @@
 public:
+  typedef std::pair<std::string, std::function<llvm::ErrorOr<ClangTidyOptions>(
+                                     llvm::StringRef)>> ConfigFileHandler;
----------------
I think a ConfigFileHandler should be a proper class and have (at least) three methods:

  - parseFromFile(StringRef file) - Read configuration from file
  - configurationAsText() - Print configuration in input-like format
  - writeToFile(StringRef) - Save configuration to specific file, ignoring other bits in the file.

Not all of them have to be implemented for all handlers, but that seems like a set of useful functionality.


Also, at least the pair's string needs to be documented.



================
Comment at: clang-tidy/ClangTidyOptions.h:154
@@ +153,3 @@
+  ///
+  /// \param ConfigHandlers specifies a custom set of configuration file
+  /// handlers. Each handler is a pair of configuration file name and a function
----------------
This should document in which order config handlers are tried.

http://reviews.llvm.org/D5821