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

[Alexander Kornienko]

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

================
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");
 
----------------
djasper wrote:
> 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.
These are valid points. I've addressed these using the opposite approach: made TryReadConfigFile try all configuration file handlers.

================
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;
----------------
djasper wrote:
> 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.
> 
> 
I don't feel that these methods will be immediately useful. We can add them (or alternatives) once we're have the need to surface relevant functionality (e.g. listing supported configuration formats and choosing the format for -dump-config) in the command-line interface. 

For now, I suggest leaving it as a pair. I'm adding a comment describing its content and usage.

================
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
----------------
djasper wrote:
> This should document in which order config handlers are tried.
Done.

http://reviews.llvm.org/D5821