[clang-tools-extra] f6e5929 - [clangd] AddUsing: Used spelled text instead of type name.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-11-24T18:59:09+01:00
New Revision: f6e59294b63e1fd0b25720f24111cd17865004be

URL: https://github.com/llvm/llvm-project/commit/f6e59294b63e1fd0b25720f24111cd17865004be
DIFF: https://github.com/llvm/llvm-project/commit/f6e59294b63e1fd0b25720f24111cd17865004be.diff

LOG: [clangd] AddUsing: Used spelled text instead of type name.

This improves the behavior related to type aliases, as well as cases of
typo correction.

Differential Revision: https://reviews.llvm.org/D91966

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
    clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index cf8347f312a3..b53df446c732 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -43,9 +43,10 @@ class AddUsing : public Tweak {
   }
 
 private:
-  // The qualifier to remove. Set by prepare().
+  // All of the following are set by prepare().
+  // The qualifier to remove.
   NestedNameSpecifierLoc QualifierToRemove;
-  // The name following QualifierToRemove. Set by prepare().
+  // The name following QualifierToRemove.
   llvm::StringRef Name;
 };
 REGISTER_TWEAK(AddUsing)
@@ -206,8 +207,17 @@ bool isNamespaceForbidden(const Tweak::Selection &Inputs,
   return false;
 }
 
+std::string getNNSLAsString(NestedNameSpecifierLoc &NNSL,
+                            const PrintingPolicy &Policy) {
+  std::string Out;
+  llvm::raw_string_ostream OutStream(Out);
+  NNSL.getNestedNameSpecifier()->print(OutStream, Policy);
+  return OutStream.str();
+}
+
 bool AddUsing::prepare(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
+  const auto &TB = Inputs.AST->getTokens();
 
   // Do not suggest "using" in header files. That way madness lies.
   if (isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
@@ -247,11 +257,20 @@ bool AddUsing::prepare(const Selection &Inputs) {
     }
   } else if (auto *T = Node->ASTNode.get<TypeLoc>()) {
     if (auto E = T->getAs<ElaboratedTypeLoc>()) {
-      if (auto *BaseTypeIdentifier =
-              E.getType().getUnqualifiedType().getBaseTypeIdentifier()) {
-        Name = BaseTypeIdentifier->getName();
-        QualifierToRemove = E.getQualifierLoc();
-      }
+      QualifierToRemove = E.getQualifierLoc();
+
+      auto SpelledTokens =
+          TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
+      if (!SpelledTokens)
+        return false;
+      auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(),
+                                               SpelledTokens->back());
+      Name = SpelledRange.text(SM);
+
+      std::string QualifierToRemoveStr = getNNSLAsString(
+          QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
+      if (!Name.consume_front(QualifierToRemoveStr))
+        return false; // What's spelled doesn't match the qualifier.
     }
   }
 
@@ -283,20 +302,13 @@ bool AddUsing::prepare(const Selection &Inputs) {
 
 Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
-  auto &TB = Inputs.AST->getTokens();
 
-  // Determine the length of the qualifier under the cursor, then remove it.
-  auto SpelledTokens = TB.spelledForExpanded(
-      TB.expandedTokens(QualifierToRemove.getSourceRange()));
-  if (!SpelledTokens) {
-    return error("Could not determine length of the qualifier");
-  }
-  unsigned Length =
-      syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back())
-          .length();
+  std::string QualifierToRemoveStr = getNNSLAsString(
+      QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
   tooling::Replacements R;
   if (auto Err = R.add(tooling::Replacement(
-          SM, SpelledTokens->front().location(), Length, ""))) {
+          SM, SM.getSpellingLoc(QualifierToRemove.getBeginLoc()),
+          QualifierToRemoveStr.length(), ""))) {
     return std::move(Err);
   }
 
@@ -313,9 +325,8 @@ Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
     if (InsertionPoint->AlwaysFullyQualify &&
         !isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
       UsingTextStream << "::";
-    QualifierToRemove.getNestedNameSpecifier()->print(
-        UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
-    UsingTextStream << Name << ";" << InsertionPoint->Suffix;
+    UsingTextStream << QualifierToRemoveStr << Name << ";"
+                    << InsertionPoint->Suffix;
 
     assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID());
     if (auto Err = R.add(tooling::Replacement(SM, InsertionPoint->Loc, 0,

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 98d797aacd8f..edfaee779e38 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2520,6 +2520,9 @@ class cc {
   EXPECT_UNAVAILABLE(Header + "void fun() { ::ban::fo^o(); }");
   EXPECT_AVAILABLE(Header + "void fun() { banana::fo^o(); }");
 
+  // Do not offer code action on typo-corrections.
+  EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
@@ -2793,6 +2796,35 @@ using one::two::ff;using one::two::ee;
 
 void fun() {
   ff();
+})cpp"},
+            // using alias; insert using for the spelled name.
+            {R"cpp(
+#include "test.hpp"
+
+void fun() {
+  one::u^u u;
+})cpp",
+             R"cpp(
+#include "test.hpp"
+
+using one::uu;
+
+void fun() {
+  uu u;
+})cpp"},
+            // using namespace.
+            {R"cpp(
+#include "test.hpp"
+using namespace one;
+namespace {
+two::c^c C;
+})cpp",
+             R"cpp(
+#include "test.hpp"
+using namespace one;
+namespace {using two::cc;
+
+cc C;
 })cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {
@@ -2809,6 +2841,7 @@ class cc {
   static void mm() {}
 };
 }
+using uu = two::cc;
 })cpp";
       EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
     }