[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 ↗

(On Diff #181312)

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 ↗

(On Diff #181312)

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 ↗

(On Diff #181312)

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 ↗

(On Diff #181312)

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.

clang-tools-extra/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
   )

clang-tools-extra/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