Revert "[clangd] Fix AddUsing in the face of typo-correction"

This reverts commit 6f23fee4ef98a695062aa128a177478ba7d742d4.
Breaks windows buildbots

diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 1e51d8fb9a518cf87f2f0459c8485e9eb610b582..103e13f44d060688bd87e2acc5878497bce3aed3 100644 (file)
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -8,25 +8,10 @@
 
 #include "AST.h"
 #include "Config.h"
-#include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "support/Logger.h"
 #include "clang/AST/Decl.h"
-#include "clang/AST/Expr.h"
-#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/RecursiveASTVisitor.h"
-#include "clang/AST/Type.h"
-#include "clang/AST/TypeLoc.h"
-#include "clang/Basic/LLVM.h"
-#include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Syntax/Tokens.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/raw_ostream.h"
-#include <string>
-#include <tuple>
-#include <utility>
 
 namespace clang {
 namespace clangd {
@@ -60,12 +45,8 @@ private:
   // All of the following are set by prepare().
   // The qualifier to remove.
   NestedNameSpecifierLoc QualifierToRemove;
-  // Qualified name to use when spelling the using declaration. This might be
-  // different than SpelledQualifier in presence of error correction.
-  std::string QualifierToSpell;
-  // The name and qualifier as spelled in the code.
-  llvm::StringRef SpelledQualifier;
-  llvm::StringRef SpelledName;
+  // The name following QualifierToRemove.
+  llvm::StringRef Name;
   // If valid, the insertion point for "using" statement must come after this.
   // This is relevant when the type is defined in the main file, to make sure
   // the type/function is already defined at the point where "using" is added.
@@ -75,7 +56,7 @@ REGISTER_TWEAK(AddUsing)
 
 std::string AddUsing::title() const {
   return std::string(llvm::formatv(
-      "Add using-declaration for {0} and remove qualifier", SpelledName));
+      "Add using-declaration for {0} and remove qualifier", Name));
 }
 
 // Locates all "using" statements relevant to SelectionDeclContext.
@@ -288,23 +269,36 @@ bool AddUsing::prepare(const Selection &Inputs) {
   if (Node == nullptr)
     return false;
 
-  SourceRange SpelledNameRange;
   if (auto *D = Node->ASTNode.get<DeclRefExpr>()) {
     if (auto *II = D->getDecl()->getIdentifier()) {
       QualifierToRemove = D->getQualifierLoc();
-      SpelledNameRange = D->getSourceRange();
+      Name = II->getName();
       MustInsertAfterLoc = D->getDecl()->getBeginLoc();
     }
   } else if (auto *T = Node->ASTNode.get<TypeLoc>()) {
     if (auto E = T->getAs<ElaboratedTypeLoc>()) {
       QualifierToRemove = E.getQualifierLoc();
+      if (!QualifierToRemove)
+        return false;
 
-      SpelledNameRange = E.getSourceRange();
+      auto NameRange = E.getSourceRange();
       if (auto T = E.getNamedTypeLoc().getAs<TemplateSpecializationTypeLoc>()) {
         // Remove the template arguments from the name.
-        SpelledNameRange.setEnd(T.getLAngleLoc().getLocWithOffset(-1));
+        NameRange.setEnd(T.getLAngleLoc().getLocWithOffset(-1));
       }
 
+      auto SpelledTokens = TB.spelledForExpanded(TB.expandedTokens(NameRange));
+      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.
+
       if (const auto *ET = E.getTypePtr()) {
         if (const auto *TDT =
                 dyn_cast<TypedefType>(ET->getNamedType().getTypePtr())) {
@@ -315,14 +309,19 @@ bool AddUsing::prepare(const Selection &Inputs) {
       }
     }
   }
-  if (!QualifierToRemove ||
-      // FIXME: This only supports removing qualifiers that are made up of just
-      // namespace names. If qualifier contains a type, we could take the
-      // longest namespace prefix and remove that.
+
+  // FIXME: This only supports removing qualifiers that are made up of just
+  // namespace names. If qualifier contains a type, we could take the longest
+  // namespace prefix and remove that.
+  if (!QualifierToRemove.hasQualifier() ||
       !QualifierToRemove.getNestedNameSpecifier()->getAsNamespace() ||
-      // Respect user config.
-      isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier()))
+      Name.empty()) {
+    return false;
+  }
+
+  if (isNamespaceForbidden(Inputs, *QualifierToRemove.getNestedNameSpecifier()))
     return false;
+
   // Macros are difficult. We only want to offer code action when what's spelled
   // under the cursor is a namespace qualifier. If it's a macro that expands to
   // a qualifier, user would not know what code action will actually change.
@@ -334,35 +333,23 @@ bool AddUsing::prepare(const Selection &Inputs) {
     return false;
   }
 
-  auto SpelledTokens =
-      TB.spelledForExpanded(TB.expandedTokens(SpelledNameRange));
-  if (!SpelledTokens)
-    return false;
-  auto SpelledRange =
-      syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back());
-  // We only drop qualifiers that're namespaces, so this is safe.
-  std::tie(SpelledQualifier, SpelledName) =
-      splitQualifiedName(SpelledRange.text(SM));
-  QualifierToSpell = getNNSLAsString(
-      QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
-  if (!llvm::StringRef(QualifierToSpell).endswith(SpelledQualifier) ||
-      SpelledName.empty())
-    return false; // What's spelled doesn't match the qualifier.
   return true;
 }
 
 Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
 
+  std::string QualifierToRemoveStr = getNNSLAsString(
+      QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
   tooling::Replacements R;
   if (auto Err = R.add(tooling::Replacement(
           SM, SM.getSpellingLoc(QualifierToRemove.getBeginLoc()),
-          SpelledQualifier.size(), ""))) {
+          QualifierToRemoveStr.length(), ""))) {
     return std::move(Err);
   }
 
-  auto InsertionPoint = findInsertionPoint(Inputs, QualifierToRemove,
-                                           SpelledName, MustInsertAfterLoc);
+  auto InsertionPoint =
+      findInsertionPoint(Inputs, QualifierToRemove, Name, MustInsertAfterLoc);
   if (!InsertionPoint) {
     return InsertionPoint.takeError();
   }
@@ -375,7 +362,7 @@ Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
     if (InsertionPoint->AlwaysFullyQualify &&
         !isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
       UsingTextStream << "::";
-    UsingTextStream << QualifierToSpell << SpelledName << ";"
+    UsingTextStream << QualifierToRemoveStr << Name << ";"
                     << InsertionPoint->Suffix;
 
     assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID());

diff --git a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
index 86077c17f7555aeba0f29518b13f0c9b9ff67406..adfd018f56d2717746aa644148faba427e5746d7 100644 (file)
--- a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
@@ -8,11 +8,8 @@
 
 #include "Config.h"
 #include "TweakTesting.h"
-#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include <string>
 
 namespace clang {
 namespace clangd {
@@ -33,7 +30,7 @@ namespace one {
 void oo() {}
 template<typename TT> class tt {};
 namespace two {
-enum ee { ee_enum_value };
+enum ee {};
 void ff() {}
 class cc {
 public:
@@ -67,6 +64,9 @@ public:
   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;");
+
   // NestedNameSpecifier, but no namespace.
   EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;");
 
@@ -466,37 +466,7 @@ one::v^ec<int> foo;
 using one::vec;
 
 vec<int> foo;
-)cpp"},
-      // Typo correction.
-      {R"cpp(
-// error-ok
-#include "test.hpp"
-c^c C;
-)cpp",
-       R"cpp(
-// error-ok
-#include "test.hpp"
-using one::two::cc;
-
-cc C;
-)cpp"},
-      {R"cpp(
-// error-ok
-#include "test.hpp"
-void foo() {
-  switch(one::two::ee{}) { case two::ee_^one:break; }
-}
-)cpp",
-       R"cpp(
-// error-ok
-#include "test.hpp"
-using one::two::ee_one;
-
-void foo() {
-  switch(one::two::ee{}) { case ee_one:break; }
-}
-)cpp"},
-  };
+)cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {
     ExtraFiles["test.hpp"] = R"cpp(