[clang] fc46d6e - [clang][Tooling] Add support for generating #import edits
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 6 10:48:08 PST 2022
Author: David Goldman
Date: 2022-12-06T13:47:07-05:00
New Revision: fc46d6e67fab06d54c8948ebf959d62984116bc3
URL: https://github.com/llvm/llvm-project/commit/fc46d6e67fab06d54c8948ebf959d62984116bc3
DIFF: https://github.com/llvm/llvm-project/commit/fc46d6e67fab06d54c8948ebf959d62984116bc3.diff
LOG: [clang][Tooling] Add support for generating #import edits
And make use of this from clangd's CodeComplete and IncludeFixer, although currently they are both restricted only to #include symbols.
Differential Revision: https://reviews.llvm.org/D128677
Added:
Modified:
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/Format/Format.cpp
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/unittests/Tooling/HeaderIncludesTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 32a80ac054cbe..b445854e57255 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -395,7 +395,8 @@ struct CodeCompletionBuilder {
CodeCompletion::IncludeCandidate Include;
Include.Header = ToInclude->first;
if (ToInclude->second && ShouldInsert)
- Include.Insertion = Includes.insert(ToInclude->first);
+ Include.Insertion = Includes.insert(
+ ToInclude->first, tooling::IncludeDirective::Include);
Completion.Includes.push_back(std::move(Include));
} else
log("Failed to generate include insertion edits for adding header "
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index 95813894d5aa3..4314ef15d3eb3 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -345,10 +345,12 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
}
llvm::Optional<TextEdit>
-IncludeInserter::insert(llvm::StringRef VerbatimHeader) const {
+IncludeInserter::insert(llvm::StringRef VerbatimHeader,
+ tooling::IncludeDirective Directive) const {
llvm::Optional<TextEdit> Edit;
- if (auto Insertion = Inserter.insert(VerbatimHeader.trim("\"<>"),
- VerbatimHeader.startswith("<")))
+ if (auto Insertion =
+ Inserter.insert(VerbatimHeader.trim("\"<>"),
+ VerbatimHeader.startswith("<"), Directive))
Edit = replacementToEdit(Code, *Insertion);
return Edit;
}
diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index 0901a23f4dd83..5611b32b11aa1 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -50,7 +50,7 @@ struct SymbolInclude {
/// The header to include. This is either a URI or a verbatim include which is
/// quoted with <> or "".
llvm::StringRef Header;
- /// The include directive to use, e.g. #import or #include.
+ /// The include directive(s) that can be used, e.g. #import and/or #include.
Symbol::IncludeDirective Directive;
};
@@ -248,7 +248,8 @@ class IncludeInserter {
/// Calculates an edit that inserts \p VerbatimHeader into code. If the header
/// is already included, this returns std::nullopt.
- llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader) const;
+ llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader,
+ tooling::IncludeDirective Directive) const;
private:
StringRef FileName;
diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp
index 5ba5a4e45ba04..45811e10e192a 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -249,18 +249,22 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
}
llvm::Optional<Fix> IncludeFixer::insertHeader(llvm::StringRef Spelled,
- llvm::StringRef Symbol) const {
+ llvm::StringRef Symbol,
+ tooling::IncludeDirective Directive) const {
Fix F;
- if (auto Edit = Inserter->insert(Spelled))
+ if (auto Edit = Inserter->insert(Spelled, Directive))
F.Edits.push_back(std::move(*Edit));
else
return std::nullopt;
+ llvm::StringRef DirectiveSpelling =
+ Directive == tooling::IncludeDirective::Include ? "Include" : "Import";
if (Symbol.empty())
- F.Message = llvm::formatv("Include {0}", Spelled);
+ F.Message = llvm::formatv("{0} {1}", DirectiveSpelling, Spelled);
else
- F.Message = llvm::formatv("Include {0} for symbol {1}", Spelled, Symbol);
+ F.Message = llvm::formatv("{0} {1} for symbol {2}",
+ DirectiveSpelling, Spelled, Symbol);
return F;
}
@@ -325,7 +329,8 @@ std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
if (!InsertedHeaders.try_emplace(ToInclude->first).second)
continue;
if (auto Fix =
- insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str()))
+ insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str(),
+ tooling::IncludeDirective::Include))
Fixes.push_back(std::move(*Fix));
}
} else {
diff --git a/clang-tools-extra/clangd/IncludeFixer.h b/clang-tools-extra/clangd/IncludeFixer.h
index 0756ee797e851..50237dd948e4a 100644
--- a/clang-tools-extra/clangd/IncludeFixer.h
+++ b/clang-tools-extra/clangd/IncludeFixer.h
@@ -17,6 +17,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Sema/ExternalSemaSource.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/Optional.h"
@@ -54,8 +55,10 @@ class IncludeFixer {
/// Generates header insertion fixes for all symbols. Fixes are deduplicated.
std::vector<Fix> fixesForSymbols(const SymbolSlab &Syms) const;
- llvm::Optional<Fix> insertHeader(llvm::StringRef Name,
- llvm::StringRef Symbol = "") const;
+ llvm::Optional<Fix>
+ insertHeader(llvm::StringRef Name, llvm::StringRef Symbol = "",
+ tooling::IncludeDirective Directive =
+ tooling::IncludeDirective::Include) const;
struct UnresolvedName {
std::string Name; // E.g. "X" in foo::X.
diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
index 4546b27dab08e..66caa5ebab95f 100644
--- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -16,6 +16,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
@@ -117,7 +118,8 @@ class HeadersTest : public ::testing::Test {
return Path.value_or("");
}
- llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader) {
+ llvm::Optional<TextEdit> insert(llvm::StringRef VerbatimHeader,
+ tooling::IncludeDirective Directive) {
Clang = setupClang();
PreprocessOnlyAction Action;
EXPECT_TRUE(
@@ -126,7 +128,7 @@ class HeadersTest : public ::testing::Test {
IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
CDB.getCompileCommand(MainFile)->Directory,
&Clang->getPreprocessor().getHeaderSearchInfo());
- auto Edit = Inserter.insert(VerbatimHeader);
+ auto Edit = Inserter.insert(VerbatimHeader, Directive);
Action.EndSourceFile();
return Edit;
}
@@ -330,9 +332,13 @@ TEST_F(HeadersTest, DontInsertDuplicateResolved) {
}
TEST_F(HeadersTest, PreferInserted) {
- auto Edit = insert("<y>");
- EXPECT_TRUE(Edit);
- EXPECT_TRUE(StringRef(Edit->newText).contains("<y>"));
+ auto Edit = insert("<y>", tooling::IncludeDirective::Include);
+ ASSERT_TRUE(Edit);
+ EXPECT_EQ(Edit->newText, "#include <y>\n");
+
+ Edit = insert("\"header.h\"", tooling::IncludeDirective::Import);
+ ASSERT_TRUE(Edit);
+ EXPECT_EQ(Edit->newText, "#import \"header.h\"\n");
}
TEST(Headers, NoHeaderSearchInfo) {
diff --git a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
index 0d9cb6a38e0a8..1466327598151 100644
--- a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -44,6 +44,8 @@ class IncludeCategoryManager {
SmallVector<llvm::Regex, 4> CategoryRegexs;
};
+enum class IncludeDirective { Include, Import };
+
/// Generates replacements for inserting or deleting #include directives in a
/// file.
class HeaderIncludes {
@@ -51,9 +53,9 @@ class HeaderIncludes {
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 Header into the code.
+ /// If \p IsAngled is true, \p Header 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
@@ -71,12 +73,13 @@ class HeaderIncludes {
/// \p IncludeName already exists (with exactly the same spelling), this
/// returns std::nullopt.
llvm::Optional<tooling::Replacement> insert(llvm::StringRef Header,
- bool IsAngled) const;
+ bool IsAngled,
+ IncludeDirective Directive) 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.
+ /// Removes all existing #includes and #imports 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 and
+ /// #imports with exactly the same spelling.
tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
// Matches a whole #include directive.
@@ -84,13 +87,16 @@ class HeaderIncludes {
private:
struct Include {
- Include(StringRef Name, tooling::Range R) : Name(Name), R(R) {}
+ Include(StringRef Name, tooling::Range R, IncludeDirective D)
+ : Name(Name), R(R), Directive(D) {}
// An include header quoted with either <> or "".
std::string Name;
// The range of the whole line of include directive including any leading
// whitespaces and trailing comment.
tooling::Range R;
+ // Either #include or #import.
+ IncludeDirective Directive;
};
void addExistingInclude(Include IncludeToAdd, unsigned NextLineOffset);
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 8e7bee94e1815..983246d6cca55 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3302,7 +3302,8 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
(void)Matched;
auto IncludeName = Matches[2];
auto Replace =
- Includes.insert(IncludeName.trim("\"<>"), IncludeName.startswith("<"));
+ Includes.insert(IncludeName.trim("\"<>"), IncludeName.startswith("<"),
+ tooling::IncludeDirective::Include);
if (Replace) {
auto Err = Result.add(*Replace);
if (Err) {
diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index dbe4f1e8827e9..665d67ea33c41 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -296,7 +296,9 @@ HeaderIncludes::HeaderIncludes(StringRef FileName, StringRef Code,
addExistingInclude(
Include(Matches[2],
tooling::Range(
- Offset, std::min(Line.size() + 1, Code.size() - Offset))),
+ Offset, std::min(Line.size() + 1, Code.size() - Offset)),
+ Matches[1] == "import" ? tooling::IncludeDirective::Import
+ : tooling::IncludeDirective::Include),
NextLineOffset);
}
Offset = NextLineOffset;
@@ -342,17 +344,20 @@ void HeaderIncludes::addExistingInclude(Include IncludeToAdd,
}
llvm::Optional<tooling::Replacement>
-HeaderIncludes::insert(llvm::StringRef IncludeName, bool IsAngled) const {
+HeaderIncludes::insert(llvm::StringRef IncludeName, bool IsAngled,
+ IncludeDirective Directive) const {
assert(IncludeName == trimInclude(IncludeName));
// If a <header> ("header") already exists in code, "header" (<header>) with
- //
diff erent quotation will still be inserted.
+ //
diff erent quotation and/or directive will still be inserted.
// FIXME: figure out if this is the best behavior.
auto It = ExistingIncludes.find(IncludeName);
- if (It != ExistingIncludes.end())
+ if (It != ExistingIncludes.end()) {
for (const auto &Inc : It->second)
- if ((IsAngled && StringRef(Inc.Name).startswith("<")) ||
- (!IsAngled && StringRef(Inc.Name).startswith("\"")))
+ if (Inc.Directive == Directive &&
+ ((IsAngled && StringRef(Inc.Name).startswith("<")) ||
+ (!IsAngled && StringRef(Inc.Name).startswith("\""))))
return std::nullopt;
+ }
std::string Quoted =
std::string(llvm::formatv(IsAngled ? "<{0}>" : "\"{0}\"", IncludeName));
StringRef QuotedName = Quoted;
@@ -371,8 +376,10 @@ HeaderIncludes::insert(llvm::StringRef IncludeName, bool IsAngled) const {
}
}
assert(InsertOffset <= Code.size());
+ llvm::StringRef DirectiveSpelling =
+ Directive == IncludeDirective::Include ? "include" : "import";
std::string NewInclude =
- std::string(llvm::formatv("#include {0}\n", QuotedName));
+ llvm::formatv("#{0} {1}\n", DirectiveSpelling, 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
diff --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp b/clang/unittests/Tooling/HeaderIncludesTest.cpp
index b7b958f0c914f..cd48d38cdb0a8 100644
--- a/clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -20,10 +20,12 @@ namespace {
class HeaderIncludesTest : public ::testing::Test {
protected:
- std::string insert(llvm::StringRef Code, llvm::StringRef Header) {
+ std::string insert(llvm::StringRef Code, llvm::StringRef Header,
+ IncludeDirective Directive = IncludeDirective::Include) {
HeaderIncludes Includes(FileName, Code, Style);
assert(Header.startswith("\"") || Header.startswith("<"));
- auto R = Includes.insert(Header.trim("\"<>"), Header.startswith("<"));
+ auto R =
+ Includes.insert(Header.trim("\"<>"), Header.startswith("<"), Directive);
if (!R)
return std::string(Code);
auto Result = applyAllReplacements(Code, Replacements(*R));
@@ -60,6 +62,25 @@ TEST_F(HeaderIncludesTest, RepeatedIncludes) {
EXPECT_EQ(Expected, insert(Code, "\"a2.h\""));
}
+TEST_F(HeaderIncludesTest, InsertImportWithSameInclude) {
+ std::string Code = "#include \"a.h\"\n";
+ std::string Expected = Code + "#import \"a.h\"\n";
+ EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import));
+}
+
+TEST_F(HeaderIncludesTest, DontInsertAlreadyImported) {
+ std::string Code = "#import \"a.h\"\n";
+ EXPECT_EQ(Code, insert(Code, "\"a.h\"", IncludeDirective::Import));
+}
+
+TEST_F(HeaderIncludesTest, DeleteImportAndSameInclude) {
+ std::string Code = R"cpp(
+#include <abc.h>
+#import <abc.h>
+int x;)cpp";
+ EXPECT_EQ("\nint x;", remove(Code, "<abc.h>"));
+}
+
TEST_F(HeaderIncludesTest, NoExistingIncludeWithDefine) {
std::string Code = "#ifndef A_H\n"
"#define A_H\n"
More information about the cfe-commits
mailing list