dgoldman created this revision.
dgoldman added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

While it currently still defaults to #include instead of #import,
this support can be used in the future to provide #import insertions
for headers with ObjC symbols once SymbolCollector is updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128677

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp

Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===================================================================
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -341,7 +341,8 @@
 }
 
 llvm::Optional<tooling::Replacement>
-HeaderIncludes::insert(llvm::StringRef IncludeName, bool IsAngled) const {
+HeaderIncludes::insert(llvm::StringRef IncludeName, bool IsAngled,
+                       bool IsImport) const {
   assert(IncludeName == trimInclude(IncludeName));
   // If a <header> ("header") already exists in code, "header" (<header>) with
   // different quotation will still be inserted.
@@ -371,7 +372,8 @@
   }
   assert(InsertOffset <= Code.size());
   std::string NewInclude =
-      std::string(llvm::formatv("#include {0}\n", QuotedName));
+      IsImport ? std::string(llvm::formatv("#import {0}\n", QuotedName))
+               : std::string(llvm::formatv("#include {0}\n", QuotedName));
   // When inserting headers at end of the code, also append '\n' to the code
   // if it does not end with '\n'.
   // FIXME: when inserting multiple #includes at the end of code, only one
Index: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
===================================================================
--- clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -51,9 +51,9 @@
   HeaderIncludes(llvm::StringRef FileName, llvm::StringRef Code,
                  const IncludeStyle &Style);
 
-  /// Inserts an #include directive of \p Header into the code. If \p IsAngled
-  /// is true, \p Header will be quoted with <> in the directive; otherwise, it
-  /// will be quoted with "".
+  /// Inserts an #include or #import directive of \p IncludeName into the code.
+  /// If \p IsAngled is true, \p IncludeName will be quoted with <> in the
+  /// directive; otherwise, it will be quoted with "".
   ///
   /// When searching for points to insert new header, this ignores #include's
   /// after the #include block(s) in the beginning of a file to avoid inserting
@@ -70,13 +70,13 @@
   /// same category in the code that should be sorted after \p IncludeName. If
   /// \p IncludeName already exists (with exactly the same spelling), this
   /// returns None.
-  llvm::Optional<tooling::Replacement> insert(llvm::StringRef Header,
-                                              bool IsAngled) const;
+  llvm::Optional<tooling::Replacement>
+  insert(llvm::StringRef Header, bool IsAngled, bool IsImport = false) const;
 
   /// Removes all existing #includes of \p Header quoted with <> if \p IsAngled
   /// is true or "" if \p IsAngled is false.
-  /// This doesn't resolve the header file path; it only deletes #includes with
-  /// exactly the same spelling.
+  /// This doesn't resolve the header file path; it only deletes #includes and
+  /// #imports with exactly the same spelling.
   tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
 
 private:
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -115,7 +115,8 @@
     return Path.value_or("");
   }
 
-  llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader) {
+  llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader,
+                                  bool ViaImport) {
     Clang = setupClang();
     PreprocessOnlyAction Action;
     EXPECT_TRUE(
@@ -124,7 +125,7 @@
     IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
                              CDB.getCompileCommand(MainFile)->Directory,
                              &Clang->getPreprocessor().getHeaderSearchInfo());
-    auto Edit = Inserter.insert(VerbatimHeader);
+    auto Edit = Inserter.insert(VerbatimHeader, ViaImport);
     Action.EndSourceFile();
     return Edit;
   }
@@ -328,9 +329,13 @@
 }
 
 TEST_F(HeadersTest, PreferInserted) {
-  auto Edit = insert("<y>");
+  auto Edit = insert("<y>", /*ViaImport=*/false);
   EXPECT_TRUE(Edit.hasValue());
-  EXPECT_TRUE(StringRef(Edit->newText).contains("<y>"));
+  EXPECT_EQ(Edit->newText, "#include <y>\n");
+
+  Edit = insert("\"header.h\"", /*ViaImport=*/true);
+  EXPECT_TRUE(Edit.hasValue());
+  EXPECT_EQ(Edit->newText, "#import \"header.h\"\n");
 }
 
 TEST(Headers, NoHeaderSearchInfo) {
Index: clang-tools-extra/clangd/IncludeFixer.h
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.h
+++ clang-tools-extra/clangd/IncludeFixer.h
@@ -55,7 +55,8 @@
   std::vector<Fix> fixesForSymbols(const SymbolSlab &Syms) const;
 
   llvm::Optional<Fix> insertHeader(llvm::StringRef Name,
-                                   llvm::StringRef Symbol = "") const;
+                                   llvm::StringRef Symbol = "",
+                                   bool ViaImport = false) const;
 
   struct UnresolvedName {
     std::string Name;   // E.g. "X" in foo::X.
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -248,10 +248,11 @@
 }
 
 llvm::Optional<Fix> IncludeFixer::insertHeader(llvm::StringRef Spelled,
-                                               llvm::StringRef Symbol) const {
+                                               llvm::StringRef Symbol,
+                                               bool ViaImport) const {
   Fix F;
 
-  if (auto Edit = Inserter->insert(Spelled))
+  if (auto Edit = Inserter->insert(Spelled, ViaImport))
     F.Edits.push_back(std::move(*Edit));
   else
     return llvm::None;
@@ -316,7 +317,7 @@
   llvm::StringSet<> InsertedHeaders;
   for (const auto &Sym : Syms) {
     for (const auto &Inc : getRankedIncludes(Sym)) {
-      if (auto ToInclude = Inserted(Sym, Inc)) {
+      if (auto ToInclude = Inserted(Sym, Inc.IncludeSpelling)) {
         if (ToInclude->second) {
           if (!InsertedHeaders.try_emplace(ToInclude->first).second)
             continue;
@@ -325,8 +326,8 @@
             Fixes.push_back(std::move(*Fix));
         }
       } else {
-        vlog("Failed to calculate include insertion for {0} into {1}: {2}", Inc,
-             File, ToInclude.takeError());
+        vlog("Failed to calculate include insertion for {0} into {1}: {2}",
+             Inc.IncludeSpelling, File, ToInclude.takeError());
       }
     }
   }
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -45,6 +45,13 @@
   bool valid() const;
 };
 
+/// Represents a spelled include or import.
+struct HeaderInclude {
+  llvm::StringRef IncludeSpelling;
+
+  Symbol::IncludeType IncludeType;
+};
+
 /// Creates a `HeaderFile` from \p Header which can be either a URI or a literal
 /// include.
 llvm::Expected<HeaderFile> toHeaderFile(llvm::StringRef Header,
@@ -52,7 +59,7 @@
 
 // Returns include headers for \p Sym sorted by popularity. If two headers are
 // equally popular, prefer the shorter one.
-llvm::SmallVector<llvm::StringRef, 1> getRankedIncludes(const Symbol &Sym);
+llvm::SmallVector<HeaderInclude, 1> getRankedIncludes(const Symbol &Sym);
 
 // An #include directive that we found in the main file.
 struct Inclusion {
@@ -239,7 +246,8 @@
 
   /// Calculates an edit that inserts \p VerbatimHeader into code. If the header
   /// is already included, this returns None.
-  llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader) const;
+  llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader,
+                                  bool ViaImport) const;
 
 private:
   StringRef FileName;
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -217,7 +217,7 @@
   return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
 }
 
-llvm::SmallVector<llvm::StringRef, 1> getRankedIncludes(const Symbol &Sym) {
+llvm::SmallVector<HeaderInclude, 1> getRankedIncludes(const Symbol &Sym) {
   auto Includes = Sym.IncludeHeaders;
   // Sort in descending order by reference count and header length.
   llvm::sort(Includes, [](const Symbol::IncludeHeaderWithReferences &LHS,
@@ -226,9 +226,9 @@
       return LHS.IncludeHeader.size() < RHS.IncludeHeader.size();
     return LHS.References > RHS.References;
   });
-  llvm::SmallVector<llvm::StringRef, 1> Headers;
+  llvm::SmallVector<HeaderInclude, 1> Headers;
   for (const auto &Include : Includes)
-    Headers.push_back(Include.IncludeHeader);
+    Headers.push_back({Include.IncludeHeader, Include.IncludeType});
   return Headers;
 }
 
@@ -350,11 +350,12 @@
   return Suggested;
 }
 
-llvm::Optional<TextEdit>
-IncludeInserter::insert(llvm::StringRef VerbatimHeader) const {
+llvm::Optional<TextEdit> IncludeInserter::insert(llvm::StringRef VerbatimHeader,
+                                                 bool ViaImport) const {
   llvm::Optional<TextEdit> Edit = None;
-  if (auto Insertion = Inserter.insert(VerbatimHeader.trim("\"<>"),
-                                       VerbatimHeader.startswith("<")))
+  if (auto Insertion =
+          Inserter.insert(VerbatimHeader.trim("\"<>"),
+                          VerbatimHeader.startswith("<"), ViaImport))
     Edit = replacementToEdit(Code, *Insertion);
   return Edit;
 }
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -186,7 +186,7 @@
   const CodeCompletionResult *SemaResult = nullptr;
   const Symbol *IndexResult = nullptr;
   const RawIdentifier *IdentifierResult = nullptr;
-  llvm::SmallVector<llvm::StringRef, 1> RankedIncludeHeaders;
+  llvm::SmallVector<HeaderInclude, 1> RankedIncludeHeaders;
 
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
@@ -262,7 +262,7 @@
         if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc())))
           return None;
     }
-    return RankedIncludeHeaders[0];
+    return RankedIncludeHeaders[0].IncludeSpelling;
   }
 
   using Bundle = llvm::SmallVector<CompletionCandidate, 4>;
@@ -378,17 +378,18 @@
     bool ShouldInsert = C.headerToInsertIfAllowed(Opts).has_value();
     // Calculate include paths and edits for all possible headers.
     for (const auto &Inc : C.RankedIncludeHeaders) {
-      if (auto ToInclude = Inserted(Inc)) {
+      if (auto ToInclude = Inserted(Inc.IncludeSpelling)) {
         CodeCompletion::IncludeCandidate Include;
         Include.Header = ToInclude->first;
         if (ToInclude->second && ShouldInsert)
-          Include.Insertion = Includes.insert(ToInclude->first);
+          Include.Insertion = Includes.insert(
+              ToInclude->first, Inc.IncludeType == Symbol::IncludeType::Import);
         Completion.Includes.push_back(std::move(Include));
       } else
         log("Failed to generate include insertion edits for adding header "
             "(FileURI='{0}', IncludeHeader='{1}') into {2}: {3}",
-            C.IndexResult->CanonicalDeclaration.FileURI, Inc, FileName,
-            ToInclude.takeError());
+            C.IndexResult->CanonicalDeclaration.FileURI, Inc.IncludeSpelling,
+            FileName, ToInclude.takeError());
     }
     // Prefer includes that do not need edits (i.e. already exist).
     std::stable_partition(Completion.Includes.begin(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to