From: Kadir Cetinkaya Date: Mon, 20 Mar 2023 17:57:40 +0000 (+0100) Subject: Revert "[clangd] Fix AddUsing in the face of typo-correction" X-Git-Tag: upstream/17.0.6~14249 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fb3f6a95393f33bc8d8550a5ac62c18e488a9b6f;p=platform%2Fupstream%2Fllvm.git 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 1e51d8f..103e13f 100644 --- 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 -#include -#include 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()) { 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()) { if (auto E = T->getAs()) { QualifierToRemove = E.getQualifierLoc(); + if (!QualifierToRemove) + return false; - SpelledNameRange = E.getSourceRange(); + auto NameRange = E.getSourceRange(); if (auto T = E.getNamedTypeLoc().getAs()) { // 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(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 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 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 86077c1..adfd018 100644 --- 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 namespace clang { namespace clangd { @@ -33,7 +30,7 @@ namespace one { void oo() {} template 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 foo; using one::vec; vec 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 EditedFiles; for (const auto &Case : Cases) { ExtraFiles["test.hpp"] = R"cpp(