[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

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


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 185690.
sammccall added a comment.
Herald added a project: clang.

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.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/Selection.cpp
  clangd/Selection.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp
  unittests/clangd/SelectionTests.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -185,6 +185,62 @@
   )cpp");
 }
 
+TEST(TweakTest, QualifyName) {
+  llvm::StringLiteral ID = "QualifyName";
+
+  const char *Input = R"cpp(
+namespace a { namespace b { namespace c { int D; } } }
+using namespace a;
+int X = ^b::c::D;
+  )cpp";
+  const char *Output = R"cpp(
+namespace a { namespace b { namespace c { int D; } } }
+using namespace a;
+int X = a::b::c::D;
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+namespace std { template  class vector; }
+using namespace std;
+vector<^vector>* v;
+  )cpp";
+  // FIXME: Outer vector is qualified rather than inner, because Selection
+  // doesn't include located template parameters.
+  Output = R"cpp(
+namespace std { template  class vector; }
+using namespace std;
+std::vector>* v;
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  checkAvailable(ID, R"cpp(
+namespace a { namespace b { namespace c { int D; class E{}; } } }
+using namespace a;
+^using ^b::^c::^D;
+int X = ^b::^c::^D;
+^b::^c::^E instance;
+  )cpp");
+  checkNotAvailable(ID, R"cpp(
+namespace a { namespace b { namespace c { int D; class E{}; } } }
+using namespace a;
+using b::c::D^;
+int ^X = b::c::D;
+b::c::E ^instance;
+  )cpp");
+  checkNotAvailable(ID, R"cpp(
+namespace a { namespace b { namespace c { int D; class E{}; } } }
+namespace { int x; }
+using namespace a;
+// Don't qualify fully-qualified things.
+using ^a::^b::c::^D;
+int y = ^x;
+
+// Don't add a:: while in namespace a.
+namespace a { namespace z { int X = ^b::c::^D; } }
+  )cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SelectionTests.cpp
===
--- unittests/clangd/SelectionTests.cpp
+++ unittests/clangd/SelectionTests.cpp
@@ -239,6 +239,17 @@
   }
 }
 
+TEST(SelectionTest, Context) {
+  const char *C = "namespace a { int ^x; }";
+  Annotations Test(C);
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto T = makeSelectionTree(C, AST);
+
+  ASSERT_TRUE(T.commonAncestor());
+  EXPECT_EQ(T.commonAncestor()->lexicalContext(),
+findDecl(AST, "a::x").getLexicalDeclContext());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,180 @@
+//===--- QualifyName.cpp -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "Logger.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 {
+// TODO: this tweak extracts references to names from the selection in an
+// efficient way. This should be extracted somewhere common.
+
+// Describes the qualifiers implied by a reference to a name in the source code.
+// This check may attempt to make these explicit.
+struct ImpliedQualifier {
+  // The position immediately before the name and any written qualifiers.
+  SourceLocation Begin;
+  // The scope that was implied, that contains the first qualifier or the name.
+  const DeclContext *Implied;
+};
+
+// Returns the qualifier implied before an already-qualified name. e.g.
+//   namespace a::b::c { int D; }
+//   using namespace a;
+//   int X = b::c::D;
+//   ^^
+// Here a:: could be inserted. It's the enclosing context of namespace b,
+// which is named by the first section of the nested name 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Sam McCall via Phabricator via cfe-commits]

sammccall commandeered this revision.
sammccall edited reviewers, added: ilya-biryukov; removed: sammccall.
sammccall added a comment.

(Grabbing this as discussed offline)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 184040.
ilya-biryukov added a comment.

- Update license header


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,179 @@
+//===--- QualifyName.cpp -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#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 foo;
+/// After:
+///   std::vector foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override final;
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) 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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+if (!Rng)
+  return true;
+if 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182871.
ilya-biryukov added a comment.

- Rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,180 @@
+//===--- 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 foo;
+/// After:
+///   std::vector foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override final;
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) 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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

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 foo; // --> std::vector foo;
>
>
> Moreover, it's just an example to illustrate how one could write a simple 
> action like that in clangd.


I see, nvmd then :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

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 foo; // --> std::vector foo;

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

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 :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182545.
ilya-biryukov added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,180 @@
+//===--- 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 foo;
+/// After:
+///   std::vector foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) 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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182320.
ilya-biryukov added a comment.

- Remove the header file


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp

Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,180 @@
+//===--- 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 foo;
+/// After:
+///   std::vector foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) 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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+if (!Rng)
+  return 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182316.
ilya-biryukov added a comment.

- Update to reflect changes in parent revision


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp
  clangd/refactor/tweaks/QualifyName.h

Index: clangd/refactor/tweaks/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.h
@@ -0,0 +1,42 @@
+//===--- QualifyName.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Fully qualifies a name under a cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+
+#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,163 @@
+//===--- 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 "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.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 {
+
+REGISTER_TWEAK(QualifyName);
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182273.
ilya-biryukov added a comment.

- Rebase after parent change


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp
  clangd/refactor/tweaks/QualifyName.h

Index: clangd/refactor/tweaks/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.h
@@ -0,0 +1,42 @@
+//===--- QualifyName.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Fully qualifies a name under a cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+
+#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public Tweak {
+public:
+  TweakID id() override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,163 @@
+//===--- 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 "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.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 {
+
+REGISTER_TWEAK(QualifyName);
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/SourceCode.h:64
 
+/// 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

jkorous wrote:
> 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?
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.



Comment at: clangd/SourceCode.h:81
+/// Preconditions: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeContains(const SourceManager , SourceRange R,
+   SourceLocation L);

jkorous wrote:
> I'd find it helpful to mention that the range is actually interpreted as 
> closed-open (knowing which half is which).
Good point, will do.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182079.
ilya-biryukov added a comment.

- Update after changes to parent revision


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/QualifyName.cpp
  clangd/refactor/tweaks/QualifyName.h

Index: clangd/refactor/tweaks/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.h
@@ -0,0 +1,42 @@
+//===--- QualifyName.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Fully qualifies a name under a cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+
+#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public Tweak {
+public:
+  TweakID id() override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,163 @@
+//===--- 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 "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.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 {
+
+REGISTER_TWEAK(QualifyName);
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

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.




Comment at: clangd/SourceCode.h:64
 
+/// 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

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?



Comment at: clangd/SourceCode.h:81
+/// Preconditions: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeContains(const SourceManager , SourceRange R,
+   SourceLocation L);

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


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

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.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181312.
ilya-biryukov added a comment.

- Add some forgotten helpers


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, 

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 181311.
ilya-biryukov added a comment.

- Add the code to actually instantiate a code action


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56610/new/

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/CodeActions.cpp
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, mgorny.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56610

Files:
  clangd/CMakeLists.txt
  clangd/refactor/actions/QualifyName.cpp
  clangd/refactor/actions/QualifyName.h

Index: clangd/refactor/actions/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.h
@@ -0,0 +1,32 @@
+//===--- QualifyNameAction.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// A code action that fully qualifies a name under cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_QUALIFYNAME_H
+
+#include "refactor/ActionProvider.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public ActionProvider {
+  llvm::Optional prepare(const ActionInputs ) override;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/actions/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/actions/QualifyName.cpp
@@ -0,0 +1,150 @@
+//===--- QualifyNameAction.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+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 {
+public:
+  LocateInsertLoc(ASTContext , SourceLocation CursorLoc,
+  Reference )
+  : 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  = 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  = 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 =