[clang-tools-extra] fb3f6a9 - Revert "[clangd] Fix AddUsing in the face of typo-correction"
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 20 10:58:06 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-03-20T18:57:53+01:00
New Revision: fb3f6a95393f33bc8d8550a5ac62c18e488a9b6f
URL: https://github.com/llvm/llvm-project/commit/fb3f6a95393f33bc8d8550a5ac62c18e488a9b6f
DIFF: https://github.com/llvm/llvm-project/commit/fb3f6a95393f33bc8d8550a5ac62c18e488a9b6f.diff
LOG: Revert "[clangd] Fix AddUsing in the face of typo-correction"
This reverts commit 6f23fee4ef98a695062aa128a177478ba7d742d4.
Breaks windows buildbots
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 1e51d8fb9a518..103e13f44d060 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 <string>
-#include <tuple>
-#include <utility>
namespace clang {
namespace clangd {
@@ -60,12 +45,8 @@ class AddUsing : public Tweak {
// 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
- //
diff erent 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
diff icult. 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 86077c17f7555..adfd018f56d27 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 <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 @@ 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;");
+
// 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(
More information about the cfe-commits
mailing list