[clangd] A code action to qualify an unqualified name

[ilya-biryukov]

• Add the code to actually instantiate a code action

[ilya-biryukov]

• Add some forgotten helpers

[ilya-biryukov]

This is a somewhat simple action to illustrate the use of code action APIs.
Still missing tests and trying to figure out what information we want to expose in order to avoid walking over ASTs in each of the actions, so this is not final. Should be a good reference point for how the code of the action would look like, though.

[jkorous]

Hi Ilya, I got here from reading your other patch (https://reviews.llvm.org/D56611). I'm wondering if we could make those range utility functions more understandable.

clangd/SourceCode.h

64

It seems to me the range is actually closed on both sides.
Do I get it right that the result is?

[begin of first token : end of last token]

Wouldn't some name like toWholeTokenRange be easier to understand?

81

I'd find it helpful to mention that the range is actually interpreted as closed-open (knowing which half is which).

[ilya-biryukov]

• Update after changes to parent revision

[ilya-biryukov]

clangd/SourceCode.h

64

The range is supposed to be half-open to allow representing points as empty range starting at the specified position.
This literally represents two byte offsets inside the same file.

My plan is to introduce a class to capture this abstraction, but let me see how the review goes.

81

Good point, will do.

[ilya-biryukov]

• Rebase after parent change

[ilya-biryukov]

• Update to reflect changes in parent revision

[ilya-biryukov]

• Remove the header file

[ilya-biryukov]

• Move to the monorepo
• Move out the source code helpers (they're now in swap-if-branches)

[JonasToth]

Is this for something like add const?
If yes, there is clang-tidy effort on that, see https://reviews.llvm.org/D54943 and https://reviews.llvm.org/D54395 for a similar effort. Would be best to share the code instead of reinventing it :)

[ilya-biryukov]

In D56610#1363408, @JonasToth wrote:

Is this for something like add const?
If yes, there is clang-tidy effort on that, see https://reviews.llvm.org/D54943 and https://reviews.llvm.org/D54395 for a similar effort. Would be best to share the code instead of reinventing it :)

No, this action is not about adding a type qualifier (like const or volatile), it adds a namespace qualifier, e.g.

using namespace std;

vector<int> foo; // --> std::vector<int> foo;

Moreover, it's just an example to illustrate how one could write a simple action like that in clangd.

[JonasToth]

In D56610#1363461, @ilya-biryukov wrote:

In D56610#1363408, @JonasToth wrote:

Is this for something like add const?
If yes, there is clang-tidy effort on that, see https://reviews.llvm.org/D54943 and https://reviews.llvm.org/D54395 for a similar effort. Would be best to share the code instead of reinventing it :)

No, this action is not about adding a type qualifier (like const or volatile), it adds a namespace qualifier, e.g.

using namespace std;

vector<int> foo; // --> std::vector<int> foo;

Moreover, it's just an example to illustrate how one could write a simple action like that in clangd.

I see, nvmd then :)

[ilya-biryukov]

• Rebase

[ilya-biryukov]

• Update license header

[sammccall]

(Grabbing this as discussed offline)

[sammccall]

Update matching to use Inputs.ASTSelection.
Cover some more cases of names (I think?)

Handle under-qualified names as well as unqualified ones.
The main benefit of this is it's a step closer to extracting all references to
qualified/unqualified names from the selection.

[hokein]

You'd need to rebase this patch, D57739 had some changes to the Tweak API.

clangd/SourceCode.h

 // Various code that examines C++ source code without using heavy AST machinery
 // (and often not even the lexer). To be used sparingly!
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #include "Protocol.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/SHA1.h"
 
 namespace clang {
 class SourceManager;
 

 /// FIXME: This should return an error if the location is invalid.
 Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
 
 /// Return the file location, corresponding to \p P. Note that one should take
 /// care to avoid comparing the result with expansion locations.
 llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
                                                         Position P);
 
+/// Turns a token range into a half-open range and checks its correctness.
+/// The resulting range will have only valid source location on both sides, both
+/// of which are file locations.
+llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &Mgr,
+                                                const LangOptions &LangOpts,
+                                                SourceRange R);
+
+/// Returns true iff all of the following conditions hold:
+///   - start and end locations are valid,
+///   - start and end locations are file locations from the same file
+///     (i.e. expansion locations are not taken into account).
+///   - start offset <= end offset.
+/// FIXME: introduce a type for source range with this invariant.
+bool isValidFileRange(const SourceManager &Mgr, SourceRange R);
+
+/// Returns true iff \p L is contained in \p R.
+/// Preconditions: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeContains(const SourceManager &Mgr, SourceRange R,
+                           SourceLocation L);
+
+/// Returns the source code covered by the source range.
+/// Preconditions: isValidRange(R) == true.
+llvm::StringRef toSourceCode(const SourceManager &SM, SourceRange R);
+
 // Converts a half-open clang source range to an LSP range.
 // Note that clang also uses closed source ranges, which this can't handle!
 Range halfOpenToRange(const SourceManager &SM, CharSourceRange R);
 
 // Converts an offset to a clang line/column (1-based, columns are bytes).
 // The offset must be in range [0, Code.size()].
 // Prefer to use SourceManager if one is available.
 std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code,

clangd/SourceCode.cpp

 //===--- SourceCode.h - Manipulating source code as strings -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "SourceCode.h"
 
 #include "Logger.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
 // Here be dragons. LSP positions use columns measured in *UTF-16 code units*!
Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
   if (!Invalid) {
     auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
     auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
     P.character = lspLength(LineSoFar);
   }
   return P;
 }
 
+bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {
+  if (!R.getBegin().isValid() || !R.getEnd().isValid())
+    return false;
+
+  FileID BeginFID;
+  size_t BeginOffset = 0;
+  std::tie(BeginFID, BeginOffset) = Mgr.getDecomposedLoc(R.getBegin());
+
+  FileID EndFID;
+  size_t EndOffset = 0;
+  std::tie(EndFID, EndOffset) = Mgr.getDecomposedLoc(R.getEnd());
+
+  return BeginFID.isValid() && BeginFID == EndFID && BeginOffset <= EndOffset;
+}
+
+bool halfOpenRangeContains(const SourceManager &Mgr, SourceRange R,
+                           SourceLocation L) {
+  assert(isValidFileRange(Mgr, R));
+
+  FileID BeginFID;
+  size_t BeginOffset = 0;
+  std::tie(BeginFID, BeginOffset) = Mgr.getDecomposedLoc(R.getBegin());
+  size_t EndOffset = Mgr.getFileOffset(R.getEnd());
+
+  FileID LFid;
+  size_t LOffset;
+  std::tie(LFid, LOffset) = Mgr.getDecomposedLoc(L);
+  return BeginFID == LFid && BeginOffset <= LOffset && LOffset < EndOffset;
+}
+
+llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &Mgr,
+                                                const LangOptions &LangOpts,
+                                                SourceRange R) {
+  auto Begin = Mgr.getFileLoc(R.getBegin());
+  if (Begin.isInvalid())
+    return llvm::None;
+  auto End = Mgr.getFileLoc(R.getEnd());
+  if (End.isInvalid())
+    return llvm::None;
+  End = Lexer::getLocForEndOfToken(End, 0, Mgr, LangOpts);
+
+  SourceRange Result(Begin, End);
+  if (!isValidFileRange(Mgr, Result))
+    return llvm::None;
+  return Result;
+}
+
+llvm::StringRef toSourceCode(const SourceManager &SM, SourceRange R) {
+  assert(isValidFileRange(SM, R));
+  bool Invalid = false;
+  auto *Buf = SM.getBuffer(SM.getFileID(R.getBegin()), &Invalid);
+  assert(!Invalid);
+
+  size_t BeginOffset = SM.getFileOffset(R.getBegin());
+  size_t EndOffset = SM.getFileOffset(R.getEnd());
+  return Buf->getBuffer().substr(BeginOffset, EndOffset - BeginOffset);
+}
+
 llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
                                                         Position P) {
   llvm::StringRef Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   auto Offset =
       positionToOffset(Code, P, /*AllowColumnBeyondLineLength=*/false);
   if (!Offset)
     return Offset.takeError();
   return SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(*Offset);
std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code,
   Offset = std::min(Code.size(), Offset);
   llvm::StringRef Before = Code.substr(0, Offset);
   int Lines = Before.count('\n');
   size_t PrevNL = Before.rfind('\n');
   size_t StartOfLine = (PrevNL == llvm::StringRef::npos) ? 0 : (PrevNL + 1);
   return {Lines + 1, Offset - StartOfLine + 1};
 }
 
-std::pair<llvm::StringRef, llvm::StringRef>
-splitQualifiedName(llvm::StringRef QName) {
+std::pair<StringRef, StringRef> splitQualifiedName(StringRef QName) {
   size_t Pos = QName.rfind("::");
   if (Pos == llvm::StringRef::npos)
     return {llvm::StringRef(), QName};
   return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)};
 }
 
 TextEdit replacementToEdit(llvm::StringRef Code,
                            const tooling::Replacement &R) {

clangd/refactor/tweaks/CMakeLists.txt

 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
 
 # A target containing all code tweaks (i.e. mini-refactorings) provided by
 # clangd.
 # Built as an object library to make sure linker does not remove global
 # constructors that register individual tweaks in a global registry.
 # To enable these tweaks in exectubales or shared libraries, add
 # $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of sources, see
 # clangd/tool/CMakeLists.txt for an example.
 add_clang_library(clangDaemonTweaks OBJECT
+  QualifyName.cpp
   )

clangd/refactor/tweaks/QualifyName.cpp

+//===--- QualifyName.cpp -----------------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Fully qualifies a name under a cursor.
+/// Before:
+///   using namespace std;
+///   ^vector<int> foo;
+/// After:
+///   std::vector<int> foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected<tooling::Replacements> apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+REGISTER_TWEAK(QualifyName);
+
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+    return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+    return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+    return nullptr; // FIXME: handle this situation, retrieve the thing
+                    // referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+                                           // TranslationUnitDecl.
+    return nullptr;
+    return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor<LocateInsertLoc> {
+public:
+  LocateInsertLoc(ASTContext &Ctx, SourceLocation CursorLoc,
+                  Reference &UnqualRef)
+      : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+    if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+      return false;
+    return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+    if (NNS.getPrefix())
+      return true; // we want only unqualified names.
+    auto &SM = Ctx.getSourceManager();
+    auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+    if (!Rng)
+      return true;
+    if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+      return true;
+    auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+    if (!Target)
+      return true; // continue traversal to recurse into types, if any.
+    UnqualRef.Begin = Rng->getBegin();
+    UnqualRef.Target = Target;
+    return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+    if (E->hasQualifier())
+      return true; // we want only unqualified names.
+    auto &SM = Ctx.getSourceManager();
+    auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+    if (!Rng)
+      return true;
+    if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+      return true;
+    UnqualRef.Begin = Rng->getBegin();
+    UnqualRef.Target = E->getFoundDecl();
+    return false;
+  }
+
+  bool VisitTagTypeLoc(TagTypeLoc Loc) {
+    auto &SM = Ctx.getSourceManager();
+    auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+    if (!Rng)
+      return true;
+    if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+      return true;
+    UnqualRef.Begin = Rng->getBegin();
+    UnqualRef.Target = Loc.getDecl();
+    return false;
+  }
+
+  bool VisitTypedefTypeLoc(TypedefTypeLoc Loc) {
+    auto &SM = Ctx.getSourceManager();
+    auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+    if (!Rng)
+      return true;
+    if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+      return true;
+    UnqualRef.Begin = Rng->getBegin();
+    UnqualRef.Target = Loc.getTypedefNameDecl();
+    return false;
+  }
+
+private:
+  ASTContext &Ctx;
+  SourceLocation CursorLoc;
+  Reference &UnqualRef;
+};
+
+bool QualifyName::prepare(const Selection &Inputs) {
+  auto &Ctx = Inputs.AST.getASTContext();
+  Reference Ref;
+  LocateInsertLoc(Ctx, Inputs.Cursor, Ref).TraverseAST(Ctx);
+  if (!Ref)
+    return false;
+  auto *Parent = Ref.Target->getDeclContext();
+  if (!Parent->isNamespace())
+    return false; // avoid complicated cases, qualify only with namespaces.
+  Qualifier = printNamespaceScope(*Parent);
+  if (Qualifier.empty())
+    return false; // happens for decls from global namespaces.
+  InsertLoc = Ref.Begin;
+  return true;
+}
+
+Expected<tooling::Replacements> QualifyName::apply(const Selection &Inputs) {
+  auto &Ctx = Inputs.AST.getASTContext();
+
+  tooling::Replacements R;
+  if (auto Err = R.add(tooling::Replacement(Ctx.getSourceManager(), InsertLoc,
+                                            0, Qualifier))) {
+    llvm::consumeError(std::move(Err));
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "building a replacement failed");
+  }
+  return R;
+}
+
+std::string QualifyName::title() const {
+  return llvm::formatv("Add '{0}' qualifier", Qualifier);
+}
+} // namespace
+} // namespace clangd
+} // namespace clang