[clangd] Move the existing heder-source-switch implemenation out of clangdServer.

[hokein]

Summary

This is a NFC change.

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/HeaderSourceSwitch.cpp

21

this patch is fixing it ?

clang-tools-extra/clangd/HeaderSourceSwitch.h

15

nit: s/a best/the best

21

nit: s/a best/the best

[hokein]

address comments.

clang-tools-extra/clangd/ClangdServer.cpp

 //===--- ClangdServer.cpp - Main clangd server code --------------*- C++-*-===//
 //
 // 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 "ClangdServer.h"
 #include "CodeComplete.h"
 #include "FindSymbols.h"
 #include "Format.h"
 #include "FormattedString.h"
+#include "HeaderSourceSwitch.h"
 #include "Headers.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SemanticSelection.h"
 #include "SourceCode.h"
if (!InpAST)
       return CB(InpAST.takeError());
     CB(clangd::locateSymbolAt(InpAST->AST, Pos, Index));
   };
 
   WorkScheduler.runWithAST("Definitions", File, std::move(Action));
 }
 
 llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
-  // FIXME: move the file heuristic to HeaderSourceSwitch.h.
-  llvm::StringRef SourceExtensions[] = {".cpp", ".c", ".cc", ".cxx",
-                                        ".c++", ".m", ".mm"};
-  llvm::StringRef HeaderExtensions[] = {".h", ".hh", ".hpp", ".hxx", ".inc"};
-
-  llvm::StringRef PathExt = llvm::sys::path::extension(Path);
-
-  // Lookup in a list of known extensions.
-  auto SourceIter =
-      llvm::find_if(SourceExtensions, [&PathExt](PathRef SourceExt) {
-        return SourceExt.equals_lower(PathExt);
-      });
-  bool IsSource = SourceIter != std::end(SourceExtensions);
-
-  auto HeaderIter =
-      llvm::find_if(HeaderExtensions, [&PathExt](PathRef HeaderExt) {
-        return HeaderExt.equals_lower(PathExt);
-      });
-
-  bool IsHeader = HeaderIter != std::end(HeaderExtensions);
-
-  // We can only switch between the known extensions.
-  if (!IsSource && !IsHeader)
-    return None;
-
-  // Array to lookup extensions for the switch. An opposite of where original
-  // extension was found.
-  llvm::ArrayRef<llvm::StringRef> NewExts;
-  if (IsSource)
-    NewExts = HeaderExtensions;
-  else
-    NewExts = SourceExtensions;
-
-  // Storage for the new path.
-  llvm::SmallString<128> NewPath = llvm::StringRef(Path);
-
-  // Instance of vfs::FileSystem, used for file existence checks.
-  auto FS = FSProvider.getFileSystem();
-
-  // Loop through switched extension candidates.
-  for (llvm::StringRef NewExt : NewExts) {
-    llvm::sys::path::replace_extension(NewPath, NewExt);
-    if (FS->exists(NewPath))
-      return NewPath.str().str(); // First str() to convert from SmallString to
-                                  // StringRef, second to convert from StringRef
-                                  // to std::string
-
-    // Also check NewExt in upper-case, just in case.
-    llvm::sys::path::replace_extension(NewPath, NewExt.upper());
-    if (FS->exists(NewPath))
-      return NewPath.str().str();
-  }
-
-  return None;
+  return getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem());
 }
 
 llvm::Expected<tooling::Replacements>
 ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
                          llvm::ArrayRef<tooling::Range> Ranges) {
   // Call clang-format.
   format::FormatStyle Style =
       getFormatStyleForFile(File, Code, FSProvider.getFileSystem().get());

clang-tools-extra/clangd/HeaderSourceSwitch.h

 //===--- HeaderSourceSwitch.h - ----------------------------------*- C++-*-===//
 //
 // 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 "ParsedAST.h"
 #include "llvm/ADT/Optional.h"
 
 namespace clang {
 namespace clangd {
 
 /// Given a header file, returns a best matching source file, and vice visa.
+/// It only uses the filename heuristics to do the inference.
+llvm::Optional<Path> getCorrespondingHeaderOrSource(
+    const Path &OriginalFile,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS);
+
+/// Given a header file, returns a best matching source file, and vice visa.
 /// The heuristics incorporate with the AST and the index (if provided).
 llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
                                                     ParsedAST &AST,
                                                     const SymbolIndex *Index);
 
 /// Returns all indexable decls that are present in the main file of the AST.
 /// Exposed for unittests.
 std::vector<const Decl *> getIndexableLocalDecls(ParsedAST &AST);
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/HeaderSourceSwitch.cpp

 #include "AST.h"
 #include "Logger.h"
 #include "index/SymbolCollector.h"
 #include "clang/AST/Decl.h"
 
 namespace clang {
 namespace clangd {
 
+llvm::Optional<Path> getCorrespondingHeaderOrSource(
+    const Path &OriginalFile,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+  // FIXME: move the file heuristic to HeaderSourceSwitch.h.
+  llvm::StringRef SourceExtensions[] = {".cpp", ".c", ".cc", ".cxx",
+                                        ".c++", ".m", ".mm"};
+  llvm::StringRef HeaderExtensions[] = {".h", ".hh", ".hpp", ".hxx", ".inc"};
+
+  llvm::StringRef PathExt = llvm::sys::path::extension(OriginalFile);
+
+  // Lookup in a list of known extensions.
+  auto SourceIter =
+      llvm::find_if(SourceExtensions, [&PathExt](PathRef SourceExt) {
+        return SourceExt.equals_lower(PathExt);
+      });
+  bool IsSource = SourceIter != std::end(SourceExtensions);
+
+  auto HeaderIter =
+      llvm::find_if(HeaderExtensions, [&PathExt](PathRef HeaderExt) {
+        return HeaderExt.equals_lower(PathExt);
+      });
+  bool IsHeader = HeaderIter != std::end(HeaderExtensions);
+
+  // We can only switch between the known extensions.
+  if (!IsSource && !IsHeader)
+    return None;
+
+  // Array to lookup extensions for the switch. An opposite of where original
+  // extension was found.
+  llvm::ArrayRef<llvm::StringRef> NewExts;
+  if (IsSource)
+    NewExts = HeaderExtensions;
+  else
+    NewExts = SourceExtensions;
+
+  // Storage for the new path.
+  llvm::SmallString<128> NewPath = llvm::StringRef(OriginalFile);
+
+  // Loop through switched extension candidates.
+  for (llvm::StringRef NewExt : NewExts) {
+    llvm::sys::path::replace_extension(NewPath, NewExt);
+    if (VFS->exists(NewPath))
+      return NewPath.str().str(); // First str() to convert from SmallString to
+                                  // StringRef, second to convert from StringRef
+                                  // to std::string
+
+    // Also check NewExt in upper-case, just in case.
+    llvm::sys::path::replace_extension(NewPath, NewExt.upper());
+    if (VFS->exists(NewPath))
+      return NewPath.str().str();
+  }
+  return None;
+}
+
 llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
                                                     ParsedAST &AST,
                                                     const SymbolIndex *Index) {
   if (!Index) {
     // FIXME: use the AST to do the inference.
     return None;
   }
   LookupRequest Request;

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

if (!ReqStats[I].FileIsRemoved) {
                 ReqStats[I].LastContentsHadErrors);
     }
 
     ASSERT_LE(Stats[I].HitsWithErrors, ReqStats[I].RequestsWithErrors);
     ASSERT_LE(Stats[I].HitsWithoutErrors, ReqStats[I].RequestsWithoutErrors);
   }
 }
 
-TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) {
-  MockFSProvider FS;
-  ErrorCheckingDiagConsumer DiagConsumer;
-  MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
-
-  auto SourceContents = R"cpp(
-  #include "foo.h"
-  int b = a;
-  )cpp";
-
-  auto FooCpp = testPath("foo.cpp");
-  auto FooH = testPath("foo.h");
-  auto Invalid = testPath("main.cpp");
-
-  FS.Files[FooCpp] = SourceContents;
-  FS.Files[FooH] = "int a;";
-  FS.Files[Invalid] = "int main() { \n return 0; \n }";
-
-  Optional<Path> PathResult = Server.switchSourceHeader(FooCpp);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooH);
-
-  PathResult = Server.switchSourceHeader(FooH);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooCpp);
-
-  SourceContents = R"c(
-  #include "foo.HH"
-  int b = a;
-  )c";
-
-  // Test with header file in capital letters and different extension, source
-  // file with different extension
-  auto FooC = testPath("bar.c");
-  auto FooHH = testPath("bar.HH");
-
-  FS.Files[FooC] = SourceContents;
-  FS.Files[FooHH] = "int a;";
-
-  PathResult = Server.switchSourceHeader(FooC);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), FooHH);
-
-  // Test with both capital letters
-  auto Foo2C = testPath("foo2.C");
-  auto Foo2HH = testPath("foo2.HH");
-  FS.Files[Foo2C] = SourceContents;
-  FS.Files[Foo2HH] = "int a;";
-
-  PathResult = Server.switchSourceHeader(Foo2C);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), Foo2HH);
-
-  // Test with source file as capital letter and .hxx header file
-  auto Foo3C = testPath("foo3.C");
-  auto Foo3HXX = testPath("foo3.hxx");
-
-  SourceContents = R"c(
-  #include "foo3.hxx"
-  int b = a;
-  )c";
-
-  FS.Files[Foo3C] = SourceContents;
-  FS.Files[Foo3HXX] = "int a;";
-
-  PathResult = Server.switchSourceHeader(Foo3C);
-  EXPECT_TRUE(PathResult.hasValue());
-  ASSERT_EQ(PathResult.getValue(), Foo3HXX);
-
-  // Test if asking for a corresponding file that doesn't exist returns an empty
-  // string.
-  PathResult = Server.switchSourceHeader(Invalid);
-  EXPECT_FALSE(PathResult.hasValue());
-}
-
 TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
   class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
   public:
     std::atomic<int> Count = {0};
 
     NoConcurrentAccessDiagConsumer(std::promise<void> StartSecondReparse)
         : StartSecondReparse(std::move(StartSecondReparse)) {}
 

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

 
 MATCHER_P(DeclNamed, Name, "") {
   if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg))
     if (ND->getQualifiedNameAsString() == Name)
       return true;
   return false;
 }
 
+TEST(HeaderSourceSwitchTest, FileHeuristic) {
+  MockFSProvider FS;
+  auto FooCpp = testPath("foo.cpp");
+  auto FooH = testPath("foo.h");
+  auto Invalid = testPath("main.cpp");
+
+  FS.Files[FooCpp];
+  FS.Files[FooH];
+  FS.Files[Invalid];
+  Optional<Path> PathResult =
+      getCorrespondingHeaderOrSource(FooCpp, FS.getFileSystem());
+  EXPECT_TRUE(PathResult.hasValue());
+  ASSERT_EQ(PathResult.getValue(), FooH);
+
+  PathResult = getCorrespondingHeaderOrSource(FooH, FS.getFileSystem());
+  EXPECT_TRUE(PathResult.hasValue());
+  ASSERT_EQ(PathResult.getValue(), FooCpp);
+
+  // Test with header file in capital letters and different extension, source
+  // file with different extension
+  auto FooC = testPath("bar.c");
+  auto FooHH = testPath("bar.HH");
+
+  FS.Files[FooC];
+  FS.Files[FooHH];
+  PathResult = getCorrespondingHeaderOrSource(FooC, FS.getFileSystem());
+  EXPECT_TRUE(PathResult.hasValue());
+  ASSERT_EQ(PathResult.getValue(), FooHH);
+
+  // Test with both capital letters
+  auto Foo2C = testPath("foo2.C");
+  auto Foo2HH = testPath("foo2.HH");
+  FS.Files[Foo2C];
+  FS.Files[Foo2HH];
+  PathResult = getCorrespondingHeaderOrSource(Foo2C, FS.getFileSystem());
+  EXPECT_TRUE(PathResult.hasValue());
+  ASSERT_EQ(PathResult.getValue(), Foo2HH);
+
+  // Test with source file as capital letter and .hxx header file
+  auto Foo3C = testPath("foo3.C");
+  auto Foo3HXX = testPath("foo3.hxx");
+
+  FS.Files[Foo3C];
+  FS.Files[Foo3HXX];
+  PathResult = getCorrespondingHeaderOrSource(Foo3C, FS.getFileSystem());
+  EXPECT_TRUE(PathResult.hasValue());
+  ASSERT_EQ(PathResult.getValue(), Foo3HXX);
+
+  // Test if asking for a corresponding file that doesn't exist returns an empty
+  // string.
+  PathResult = getCorrespondingHeaderOrSource(Invalid, FS.getFileSystem());
+  EXPECT_FALSE(PathResult.hasValue());
+}
+
 TEST(HeaderSourceSwitchTest, GetLocalDecls) {
   TestTU TU;
   TU.HeaderCode = R"cpp(
   void HeaderOnly();
   )cpp";
   TU.Code = R"cpp(
   void MainF1();
   class Foo {};