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

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-11-07T19:18:02Z
New Revision: d725f1ce5318f8aca12632d3b7cfb3a5968e5d37

URL: https://github.com/llvm/llvm-project/commit/d725f1ce5318f8aca12632d3b7cfb3a5968e5d37
DIFF: https://github.com/llvm/llvm-project/commit/d725f1ce5318f8aca12632d3b7cfb3a5968e5d37.diff

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

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.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D90992

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

Modified: 
    clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
    clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 8ff22041d725..2ac4bbbf36b3 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -325,7 +325,9 @@ 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;
@@ -336,15 +338,13 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef 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";
@@ -363,7 +363,7 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
                      << ParsedOptions.getError().message() << "\n";
       continue;
     }
-    return OptionsSource(*ParsedOptions, ConfigFile.c_str());
+    return OptionsSource(*ParsedOptions, ConfigFile.str());
   }
   return llvm::None;
 }

diff  --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
index 016cb9d0c244..5b0cbac3a61e 100644
--- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -17,6 +17,7 @@ add_extra_unittest(ClangTidyTests
   LLVMModuleTest.cpp
   NamespaceAliaserTest.cpp
   ObjCModuleTest.cpp
+  OptionsProviderTest.cpp
   OverlappingReplacementsTest.cpp
   UsingInserterTest.cpp
   ReadabilityModuleTest.cpp

diff  --git a/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
new file mode 100644
index 000000000000..b99d0781e3f7
--- /dev/null
+++ b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
@@ -0,0 +1,67 @@
+//===---- 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