VitaNuo updated this revision to Diff 527452.
VitaNuo added a comment.

Remove newline at file end.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150185/new/

https://reviews.llvm.org/D150185

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  
clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
  clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
@@ -0,0 +1,105 @@
+//===--- IncludeSpellerTest.cpp
+//-------------------------------------------------===//
+//
+// 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 "clang-include-cleaner/IncludeSpeller.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Testing/TestAST.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+#include <assert.h>
+#include <initializer_list>
+#include <string>
+namespace clang::include_cleaner {
+namespace {
+
+const char *testRoot() {
+#ifdef _WIN32
+  return "C:\\include-cleaner-test";
+#else
+  return "/include-cleaner-test";
+#endif
+}
+
+std::string testPath(llvm::StringRef File) {
+  assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
+
+  llvm::SmallString<32> NativeFile = File;
+  llvm::sys::path::native(NativeFile, llvm::sys::path::Style::native);
+  llvm::SmallString<32> Path;
+  llvm::sys::path::append(Path, llvm::sys::path::Style::native, testRoot(),
+                          NativeFile);
+  return std::string(Path.str());
+}
+
+std::string
+appendPathFileSystemIndependent(std::initializer_list<std::string> Segments,
+                                bool IsAbsolute) {
+  llvm::SmallString<32> Result;
+  if (IsAbsolute)
+    llvm::sys::path::append(Result, llvm::sys::path::Style::native,
+                            llvm::sys::path::get_separator());
+  for (const auto &Segment : Segments)
+    llvm::sys::path::append(Result, llvm::sys::path::Style::native, Segment);
+  return std::string(Result.str());
+}
+
+class DummyIncludeSpeller : public IncludeSpeller {
+public:
+  std::string operator()(const IncludeSpellerInput &Input) const override {
+    llvm::StringRef AbsolutePath = Input.H.physical()->tryGetRealPathName();
+    std::string RootWithSeparator{testRoot()};
+    RootWithSeparator += llvm::sys::path::get_separator();
+    bool Result =
+        AbsolutePath.consume_front(llvm::StringRef{RootWithSeparator});
+    if (!Result)
+      return "";
+    return AbsolutePath.str();
+  }
+};
+
+TEST(IncludeSpeller, IsRelativeToTestRoot) {
+  TestInputs Inputs;
+  Inputs.FileName = testPath("foo.h");
+  Inputs.ExtraFiles["bar.h"] = "";
+  Inputs.ExtraFiles[testPath(
+      appendPathFileSystemIndependent({"foo", "baz.h"}, false))] = "";
+  Inputs.ExtraFiles[appendPathFileSystemIndependent({"foo", "bar.h"}, true)] =
+      "";
+  TestAST AST{Inputs};
+
+  auto &FM = AST.fileManager();
+  auto &HS = AST.preprocessor().getHeaderSearchInfo();
+  const auto *MainFile = AST.sourceManager().getFileEntryForID(
+      AST.sourceManager().getMainFileID());
+
+  EXPECT_EQ("foo.h", spellHeader({Header{*FM.getFile(testPath("foo.h"))}, HS,
+                                  MainFile}));
+  EXPECT_EQ("\"bar.h\"",
+            spellHeader({Header{*FM.getFile("bar.h")}, HS, MainFile}));
+  EXPECT_EQ(
+      appendPathFileSystemIndependent({"foo", "baz.h"}, false),
+      spellHeader({Header{*FM.getFile(testPath(appendPathFileSystemIndependent(
+                       {"foo", "baz.h"}, false)))},
+                   HS, MainFile}));
+  EXPECT_EQ("\"" + appendPathFileSystemIndependent({"foo", "bar.h"}, true) +
+                "\"",
+            spellHeader({Header{*FM.getFile(appendPathFileSystemIndependent(
+                             {"foo", "bar.h"}, true))},
+                         HS, MainFile}));
+}
+
+IncludeSpellingStrategy::Add<DummyIncludeSpeller>
+    Speller("dummy", "Dummy Include Speller");
+
+} // namespace
+} // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
+++ clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
@@ -7,6 +7,7 @@
 add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
   AnalysisTest.cpp
   FindHeadersTest.cpp
+  IncludeSpellerTest.cpp
   LocateSymbolTest.cpp
   RecordTest.cpp
   TypesTest.cpp
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -8,10 +8,12 @@
 
 #include "clang-include-cleaner/Analysis.h"
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/IncludeSpeller.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -20,14 +22,39 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Registry.h"
+#include <memory>
 #include <string>
 
+LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy)
+
 namespace clang::include_cleaner {
 
+std::string mapHeader(const IncludeSpellerInput &Input) {
+  static auto Spellers = [] {
+    auto Result =
+        llvm::SmallVector<std::unique_ptr<include_cleaner::IncludeSpeller>>{};
+    for (const auto &Strategy :
+         include_cleaner::IncludeSpellingStrategy::entries())
+      Result.push_back(Strategy.instantiate());
+    return Result;
+  }();
+
+  std::string Spelling;
+  for (const auto &Speller : Spellers) {
+    Spelling = (*Speller)(Input);
+    if (!Spelling.empty())
+      break;
+  }
+  return Spelling;
+}
+
 void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
               llvm::ArrayRef<SymbolReference> MacroRefs,
               const PragmaIncludes *PI, const SourceManager &SM,
@@ -53,19 +80,24 @@
   }
 }
 
-std::string spellHeader(const Header &H, HeaderSearch &HS,
-                        const FileEntry *Main) {
+std::string spellHeader(const IncludeSpellerInput &Input) {
+  const Header &H = Input.H;
   switch (H.kind()) {
-  case Header::Physical: {
-    bool IsSystem = false;
-    std::string Path = HS.suggestPathToFileForDiagnostics(
-        H.physical(), Main->tryGetRealPathName(), &IsSystem);
-    return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
-  }
   case Header::Standard:
     return H.standard().name().str();
   case Header::Verbatim:
     return H.verbatim().str();
+  case Header::Physical:
+    // Spelling physical headers allows for various plug-in strategies.
+    std::string FinalSpelling = mapHeader(Input);
+    if (!FinalSpelling.empty())
+      return FinalSpelling;
+
+    // Fallback to default spelling via header search.
+    bool IsSystem = false;
+    FinalSpelling = Input.HS.suggestPathToFileForDiagnostics(
+        H.physical(), Input.Main->tryGetRealPathName(), &IsSystem);
+    return IsSystem ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\"";
   }
   llvm_unreachable("Unknown Header kind");
 }
@@ -90,7 +122,7 @@
              }
              if (!Satisfied && !Providers.empty() &&
                  Ref.RT == RefType::Explicit)
-               Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+               Missing.insert(spellHeader({Providers.front(), HS, MainFile}));
            });
 
   AnalysisResults Results;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
===================================================================
--- /dev/null
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
@@ -0,0 +1,39 @@
+//===--- IncludeSpeller.h - Spelling strategies for headers.-------- 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
+//
+//===----------------------------------------------------------------------===//
+// An extension point to let applications introduce custom spelling
+// strategies for physical headers.
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_INCLUDE_CLEANER_INCLUDESPELLER_H
+#define CLANG_INCLUDE_CLEANER_INCLUDESPELLER_H
+
+#include "clang-include-cleaner/Types.h"
+#include "clang/Lex/HeaderSearch.h"
+#include <string>
+
+namespace clang::include_cleaner {
+
+/// Provides the necessary information for custom spelling computations.
+struct IncludeSpellerInput {
+  const Header &H;
+  HeaderSearch &HS;
+  const FileEntry *Main;
+};
+
+class IncludeSpeller {
+public:
+  virtual ~IncludeSpeller() = default;
+
+  /// Takes in absolute path to a source file and should return a verbatim
+  /// include spelling (with angles/quotes) or an empty string to indicate no
+  /// customizations are needed.
+  virtual std::string operator()(const IncludeSpellerInput &Input) const = 0;
+};
+} // namespace clang::include_cleaner
+
+#endif
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -11,14 +11,17 @@
 #ifndef CLANG_INCLUDE_CLEANER_ANALYSIS_H
 #define CLANG_INCLUDE_CLEANER_ANALYSIS_H
 
+#include "clang-include-cleaner/IncludeSpeller.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/MemoryBufferRef.h"
-#include <variant>
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+#include <string>
 
 namespace clang {
 class SourceLocation;
@@ -75,9 +78,6 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
                         const format::FormatStyle &IncludeStyle);
 
-std::string spellHeader(const Header &H, HeaderSearch &HS,
-                        const FileEntry *Main);
-
 /// Gets all the providers for a symbol by traversing each location.
 /// Returned headers are sorted by relevance, first element is the most
 /// likely provider for the symbol.
@@ -85,6 +85,13 @@
                                            const SourceManager &SM,
                                            const PragmaIncludes *PI);
 
+using IncludeSpellingStrategy = llvm::Registry<IncludeSpeller>;
+
+/// Generates a spelling for `H` that can be directly included in `Main`.
+/// When `H` is a physical header, prefers the spelling provided by custom llvm
+/// strategies, if any. Otherwise, uses header search info to generate
+/// shortest spelling.
+std::string spellHeader(const IncludeSpellerInput &Input);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -74,11 +74,6 @@
 convertIncludes(const SourceManager &SM,
                 const llvm::ArrayRef<Inclusion> Includes);
 
-/// Determines the header spelling of an include-cleaner header
-/// representation. The spelling contains the ""<> characters.
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
-                        include_cleaner::Header Provider);
-
 std::vector<include_cleaner::SymbolReference>
 collectMacroReferences(ParsedAST &AST);
 
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,8 +197,9 @@
       continue;
     }
 
-    std::string Spelling =
-        spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front());
+    std::string Spelling = include_cleaner::spellHeader(
+        {SymbolWithMissingInclude.Providers.front(),
+         AST.getPreprocessor().getHeaderSearchInfo(), MainFile});
     llvm::StringRef HeaderRef{Spelling};
     bool Angled = HeaderRef.starts_with("<");
     // We might suggest insertion of an existing include in edge cases, e.g.,
@@ -332,21 +333,6 @@
   return ConvertedIncludes;
 }
 
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
-                        include_cleaner::Header Provider) {
-  if (Provider.kind() == include_cleaner::Header::Physical) {
-    if (auto CanonicalPath = getCanonicalPath(Provider.physical()->getLastRef(),
-                                              AST.getSourceManager())) {
-      std::string SpelledHeader =
-          llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
-      if (!SpelledHeader.empty())
-        return SpelledHeader;
-    }
-  }
-  return include_cleaner::spellHeader(
-      Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
-}
-
 std::vector<const Inclusion *>
 getUnused(ParsedAST &AST,
           const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1222,7 +1222,9 @@
     // on local variables, etc.
     return;
 
-  HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H);
+  HI.Provider = include_cleaner::spellHeader(
+      {H, AST.getPreprocessor().getHeaderSearchInfo(),
+       SM.getFileEntryForID(SM.getMainFileID())});
 }
 
 // FIXME: similar functions are present in FindHeaders.cpp (symbolName)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to