[clangd] Fix clang tidy provider when multiple config files exist in directory tree

[njames93]

Summary

Currently Clang tidy provider searches from the root directory up to the target directory, this is the opposite of how clang-tidy searches for config files.
The result of this is .clang-tidy files are ignored in any subdirectory of a directory containing a .clang-tidy file.

[njames93]

Newline at eof.

[njames93]

I have made an extra test case that compares the output of our provider to the output of a clang::tidy::FileOptionsProvider, This would ensure coherent behaviour between our implementation and clang-tidy implementation.
However I'm not sure it should be included as any changes made on the clang tidy side may break that test and we shouldn't be enforcing clang-tidy maintainers to also ensure compliance with clangd.

[njames93]

@sammccall I meant to ping this one sorry.

[sammccall]

Sorry about dropping this (and others, i'm trying to get through them now)

Agree about cherrypicking this.

Regarding the extra test case... I'm not sure either. It's a nice test, but the conceptual dependency is annoying.
If it's simple enough, feel free to add it and we can remove if it causes problems.

clang-tools-extra/clangd/TidyProvider.cpp

void apply(tidy::ClangTidyOptions &Result, PathRef AbsPath) {
     assert(path::is_absolute(AbsPath));
 
     // Compute absolute paths to all ancestors (substrings of P.Path).
     // Ensure cache entries for each ancestor exist in the map.
     llvm::StringRef Parent = path::parent_path(AbsPath);
     llvm::SmallVector<DotClangTidyCache *> Caches;
     {
       std::lock_guard<std::mutex> Lock(Mu);
-      for (auto I = path::begin(Parent), E = path::end(Parent); I != E; ++I) {
+      for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) {
         assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
                "Canonical path components should be substrings");
         llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
 #ifdef _WIN32
         // C:\ is an ancestor, but skip its (relative!) parent C:.
         if (Ancestor.size() == 2 && Ancestor.back() == ':')
           continue;
 #endif

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

add_unittest(ClangdUnitTests ClangdTests
   SymbolCollectorTests.cpp
   SymbolInfoTests.cpp
   SyncAPI.cpp
   TUSchedulerTests.cpp
   TestFS.cpp
   TestIndex.cpp
   TestTU.cpp
   TestWorkspace.cpp
+  TidyProviderTests.cpp
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
   ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp
 
   support/CancellationTests.cpp
   support/ContextTests.cpp
   support/FunctionTests.cpp

clang-tools-extra/clangd/unittests/TidyProviderTests.cpp

+//===-- TidyProviderTests.cpp - Clang tidy configuration provider tests ---===//
+//
+// 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 "TestFS.h"
+#include "TidyProvider.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+TEST(TidyProvider, NestedDirectories) {
+  MockFS FS;
+  FS.Files[testPath(".clang-tidy")] = R"yaml(
+  Checks: 'llvm-*'
+  CheckOptions:
+    - key: TestKey
+      value: 1
+)yaml";
+  FS.Files[testPath("sub1/.clang-tidy")] = R"yaml(
+  Checks: 'misc-*'
+  CheckOptions:
+    - key: TestKey
+      value: 2
+)yaml";
+  FS.Files[testPath("sub1/sub2/.clang-tidy")] = R"yaml(
+  Checks: 'bugprone-*'
+  CheckOptions:
+    - key: TestKey
+      value: 3
+  InheritParentConfig: true
+)yaml";
+
+  TidyProvider Provider = provideClangTidyFiles(FS);
+
+  auto BaseOptions = getTidyOptionsForFile(Provider, testPath("File.cpp"));
+  ASSERT_TRUE(BaseOptions.Checks.hasValue());
+  EXPECT_EQ(*BaseOptions.Checks, "llvm-*");
+  EXPECT_EQ(BaseOptions.CheckOptions.lookup("TestKey").Value, "1");
+
+  auto Sub1Options = getTidyOptionsForFile(Provider, testPath("sub1/File.cpp"));
+  ASSERT_TRUE(Sub1Options.Checks.hasValue());
+  EXPECT_EQ(*Sub1Options.Checks, "misc-*");
+  EXPECT_EQ(Sub1Options.CheckOptions.lookup("TestKey").Value, "2");
+
+  auto Sub2Options =
+      getTidyOptionsForFile(Provider, testPath("sub1/sub2/File.cpp"));
+  ASSERT_TRUE(Sub2Options.Checks.hasValue());
+  EXPECT_EQ(*Sub2Options.Checks, "misc-*,bugprone-*");
+  EXPECT_EQ(Sub2Options.CheckOptions.lookup("TestKey").Value, "3");
+}
+} // namespace
+} // namespace clangd
+} // namespace clang