[clang-tidy] Use vfs::FileSystem when getting config

[njames93]

Summary

The config providers that look for configuration files currently take a pointer to a FileSystem in the constructor.
For some reason this isn't actually used when trying to read those configuration files, Essentially it just follows the behaviour of the real filesystem.
Using clang-tidy standalone this doesn't cause any issue.
But if its used as a library and the user wishes to use say an InMemoryFileSystem it will try to read the files from the disc instead.

[njames93]

Added unittest for loading configs from an InMemoryFileSystem.

[njames93]

Small tweak to the tests.

[aaron.ballman]

LGTM aside from a nit with the test.

clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp

68

Can you add the newline to the end of the file?

[thakis]

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

366

This breaks bootstrap builds against the gcc 5.3 libstdc++: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8864218000842553296/+/steps/package_clang/0/stdout?format=raw

/b/s/w/ir/cache/builder/src/third_party/llvm/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:366:12: error: no matching constructor for initialization of 'clang::tidy::ClangTidyOptionsProvider::OptionsSource' (aka 'pair<clang::tidy::ClangTidyOptions, basic_string<char>>')
    return OptionsSource(*ParsedOptions, ConfigFile.str());
           ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can we undo the change on this line?

PTAL!

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

FileOptionsProvider::getRawOptions(StringRef FileName) {
   RawOptions.push_back(CommandLineOptions);
   return RawOptions;
 }
 
 llvm::Optional<OptionsSource>
 FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
   assert(!Directory.empty());
 
-  if (!llvm::sys::fs::is_directory(Directory)) {
+  llvm::ErrorOr<llvm::vfs::Status> DirectoryStatus = FS->status(Directory);
+
+  if (!DirectoryStatus || !DirectoryStatus->isDirectory()) {
     llvm::errs() << "Error reading configuration from " << Directory
                  << ": directory doesn't exist.\n";
     return llvm::None;
   }
 
   for (const ConfigFileHandler &ConfigHandler : ConfigHandlers) {
     SmallString<128> ConfigFile(Directory);
     llvm::sys::path::append(ConfigFile, ConfigHandler.first);
     LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
 
-    bool IsFile = false;
-    // Ignore errors from is_regular_file: we only need to know if we can read
-    // the file or not.
-    llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile);
-    if (!IsFile)
+    llvm::ErrorOr<llvm::vfs::Status> FileStatus = FS->status(ConfigFile);
+
+    if (!FileStatus || !FileStatus->isRegularFile())
       continue;
 
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
-        llvm::MemoryBuffer::getFile(ConfigFile.c_str());
+        FS->getBufferForFile(ConfigFile);
     if (std::error_code EC = Text.getError()) {
       llvm::errs() << "Can't read " << ConfigFile << ": " << EC.message()
                    << "\n";
       continue;
     }
 
     // Skip empty files, e.g. files opened for writing via shell output
     // redirection.
     if ((*Text)->getBuffer().empty())
       continue;
     llvm::ErrorOr<ClangTidyOptions> ParsedOptions =
         ConfigHandler.second((*Text)->getBuffer());
     if (!ParsedOptions) {
       if (ParsedOptions.getError())
         llvm::errs() << "Error parsing " << ConfigFile << ": "
                      << ParsedOptions.getError().message() << "\n";
       continue;
     }
-    return OptionsSource(*ParsedOptions, ConfigFile.c_str());
+    return OptionsSource(*ParsedOptions, ConfigFile.str());
   }
   return llvm::None;
 }
 
 /// 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);

clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

add_extra_unittest(ClangTidyTests
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
   GlobListTest.cpp
   GoogleModuleTest.cpp
   LLVMModuleTest.cpp
   NamespaceAliaserTest.cpp
   ObjCModuleTest.cpp
+  OptionsProviderTest.cpp
   OverlappingReplacementsTest.cpp
   UsingInserterTest.cpp
   ReadabilityModuleTest.cpp
   TransformerClangTidyCheckTest.cpp
   )
 
 clang_target_link_libraries(ClangTidyTests
   PRIVATE

clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp

+//===---- ObjCModuleTest.cpp - clang-tidy ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangTidyOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+TEST(ClangTidyOptionsProvider, InMemoryFileSystems) {
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FileSystem(
+      new llvm::vfs::InMemoryFileSystem);
+
+  StringRef BaseClangTidy = R"(
+    Checks: -*,clang-diagnostic-*
+  )";
+  StringRef Sub1ClangTidy = R"(
+    Checks: readability-*
+    InheritParentConfig: true
+  )";
+  StringRef Sub2ClangTidy = R"(
+    Checks: bugprone-*,misc-*,clang-diagnostic-*
+    InheritParentConfig: false
+    )";
+  FileSystem->addFile("ProjectRoot/.clang-tidy", 0,
+                      llvm::MemoryBuffer::getMemBuffer(BaseClangTidy));
+  FileSystem->addFile("ProjectRoot/SubDir1/.clang-tidy", 0,
+                      llvm::MemoryBuffer::getMemBuffer(Sub1ClangTidy));
+  FileSystem->addFile("ProjectRoot/SubDir1/File.cpp", 0,
+                      llvm::MemoryBuffer::getMemBuffer(""));
+  FileSystem->addFile("ProjectRoot/SubDir1/SubDir2/.clang-tidy", 0,
+                      llvm::MemoryBuffer::getMemBuffer(Sub2ClangTidy));
+  FileSystem->addFile("ProjectRoot/SubDir1/SubDir2/File.cpp", 0,
+                      llvm::MemoryBuffer::getMemBuffer(""));
+  FileSystem->addFile("ProjectRoot/SubDir1/SubDir2/SubDir3/File.cpp", 0,
+                      llvm::MemoryBuffer::getMemBuffer(""));
+
+  FileOptionsProvider FileOpt({}, {}, {}, FileSystem);
+
+  ClangTidyOptions File1Options =
+      FileOpt.getOptions("ProjectRoot/SubDir1/File.cpp");
+  ClangTidyOptions File2Options =
+      FileOpt.getOptions("ProjectRoot/SubDir1/SubDir2/File.cpp");
+  ClangTidyOptions File3Options =
+      FileOpt.getOptions("ProjectRoot/SubDir1/SubDir2/SubDir3/File.cpp");
+
+  ASSERT_TRUE(File1Options.Checks.hasValue());
+  EXPECT_EQ(*File1Options.Checks, "-*,clang-diagnostic-*,readability-*");
+  ASSERT_TRUE(File2Options.Checks.hasValue());
+  EXPECT_EQ(*File2Options.Checks, "bugprone-*,misc-*,clang-diagnostic-*");
+
+  // 2 and 3 should use the same config so these should also be the same.
+  EXPECT_EQ(File2Options.Checks, File3Options.Checks);
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang