Skip to content

Commit b857a50

Browse files
committed
[clangd] Support include-fixer inside macro arguments.
Motivating case: EXPECT_EQ(42, missingFunction(bar)); Differential Revision: https://reviews.llvm.org/D120619
1 parent f5b6866 commit b857a50

File tree

2 files changed

+44
-19
lines changed

2 files changed

+44
-19
lines changed

clang-tools-extra/clangd/IncludeFixer.cpp

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,8 @@ llvm::Optional<std::string> qualifiedByUnresolved(const SourceManager &SM,
343343
SourceLocation Loc,
344344
const LangOptions &LangOpts) {
345345
std::string Result;
346-
347-
SourceLocation NextLoc = Loc;
346+
// Accept qualifier written within macro arguments, but not macro bodies.
347+
SourceLocation NextLoc = SM.getTopMacroCallerLoc(Loc);
348348
while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) {
349349
if (!CCTok->is(tok::coloncolon))
350350
break;
@@ -373,40 +373,46 @@ struct CheapUnresolvedName {
373373
llvm::Optional<std::string> UnresolvedScope;
374374
};
375375

376+
llvm::Optional<std::string> getSpelledSpecifier(const CXXScopeSpec &SS,
377+
const SourceManager &SM) {
378+
// Support specifiers written within a single macro argument.
379+
if (!SM.isWrittenInSameFile(SS.getBeginLoc(), SS.getEndLoc()))
380+
return llvm::None;
381+
SourceRange Range(SM.getTopMacroCallerLoc(SS.getBeginLoc()), SM.getTopMacroCallerLoc(SS.getEndLoc()));
382+
if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID())
383+
return llvm::None;
384+
385+
return (toSourceCode(SM, Range) + "::").str();
386+
}
387+
376388
// Extracts unresolved name and scope information around \p Unresolved.
377389
// FIXME: try to merge this with the scope-wrangling code in CodeComplete.
378390
llvm::Optional<CheapUnresolvedName> extractUnresolvedNameCheaply(
379391
const SourceManager &SM, const DeclarationNameInfo &Unresolved,
380392
CXXScopeSpec *SS, const LangOptions &LangOpts, bool UnresolvedIsSpecifier) {
381-
bool Invalid = false;
382-
llvm::StringRef Code = SM.getBufferData(
383-
SM.getDecomposedLoc(Unresolved.getBeginLoc()).first, &Invalid);
384-
if (Invalid)
385-
return llvm::None;
386393
CheapUnresolvedName Result;
387394
Result.Name = Unresolved.getAsString();
388395
if (SS && SS->isNotEmpty()) { // "::" or "ns::"
389396
if (auto *Nested = SS->getScopeRep()) {
390-
if (Nested->getKind() == NestedNameSpecifier::Global)
397+
if (Nested->getKind() == NestedNameSpecifier::Global) {
391398
Result.ResolvedScope = "";
392-
else if (const auto *NS = Nested->getAsNamespace()) {
393-
auto SpecifiedNS = printNamespaceScope(*NS);
399+
} else if (const auto *NS = Nested->getAsNamespace()) {
400+
std::string SpecifiedNS = printNamespaceScope(*NS);
401+
llvm::Optional<std::string> Spelling = getSpelledSpecifier(*SS, SM);
394402

395403
// Check the specifier spelled in the source.
396-
// If the resolved scope doesn't end with the spelled scope. The
397-
// resolved scope can come from a sema typo correction. For example,
404+
// If the resolved scope doesn't end with the spelled scope, the
405+
// resolved scope may come from a sema typo correction. For example,
398406
// sema assumes that "clangd::" is a typo of "clang::" and uses
399407
// "clang::" as the specified scope in:
400408
// namespace clang { clangd::X; }
401409
// In this case, we use the "typo" specifier as extra scope instead
402410
// of using the scope assumed by sema.
403-
auto B = SM.getFileOffset(SS->getBeginLoc());
404-
auto E = SM.getFileOffset(SS->getEndLoc());
405-
std::string Spelling = (Code.substr(B, E - B) + "::").str();
406-
if (llvm::StringRef(SpecifiedNS).endswith(Spelling))
407-
Result.ResolvedScope = SpecifiedNS;
408-
else
409-
Result.UnresolvedScope = Spelling;
411+
if (!Spelling || llvm::StringRef(SpecifiedNS).endswith(*Spelling)) {
412+
Result.ResolvedScope = std::move(SpecifiedNS);
413+
} else {
414+
Result.UnresolvedScope = std::move(*Spelling);
415+
}
410416
} else if (const auto *ANS = Nested->getAsNamespaceAlias()) {
411417
Result.ResolvedScope = printNamespaceScope(*ANS->getNamespace());
412418
} else {

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ namespace {
3636
using ::testing::_;
3737
using ::testing::AllOf;
3838
using ::testing::Contains;
39+
using ::testing::Each;
3940
using ::testing::ElementsAre;
4041
using ::testing::Field;
4142
using ::testing::IsEmpty;
@@ -1094,6 +1095,24 @@ using Type = ns::$template[[Foo]]<int>;
10941095
"Include \"x.h\" for symbol ns::X")))));
10951096
}
10961097

1098+
TEST(IncludeFixerTest, TypoInMacro) {
1099+
auto TU = TestTU::withCode(R"cpp(// error-ok
1100+
#define ID(T) T
1101+
X a1;
1102+
ID(X a2);
1103+
ns::X a3;
1104+
ID(ns::X a4);
1105+
namespace ns{};
1106+
ns::X a5;
1107+
ID(ns::X a6);
1108+
)cpp");
1109+
auto Index = buildIndexWithSymbol(
1110+
{SymbolWithHeader{"X", "unittest:///x.h", "\"x.h\""},
1111+
SymbolWithHeader{"ns::X", "unittest:///ns.h", "\"x.h\""}});
1112+
TU.ExternalIndex = Index.get();
1113+
EXPECT_THAT(*TU.build().getDiagnostics(), Each(withFix(_)));
1114+
}
1115+
10971116
TEST(IncludeFixerTest, MultipleMatchedSymbols) {
10981117
Annotations Test(R"cpp(// error-ok
10991118
$insert[[]]namespace na {

0 commit comments

Comments
 (0)