kuhnel updated this revision to Diff 207037.
kuhnel marked 10 inline comments as done.
kuhnel added a comment.
fixed code review comments from sam
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62855/new/
https://reviews.llvm.org/D62855
Files:
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.h
clang-tools-extra/clangd/test/code-action-request.test
clang-tools-extra/clangd/unittests/TweakTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2108,6 +2108,28 @@
}
}
+TEST(GetDeductedType, KwAutoExpansion) {
+ struct Test {
+ StringRef AnnotatedCode;
+ const char *DeductedType;
+ } Tests[] = {
+ {"^auto i = 0;", "int"},
+ {"^auto f(){ return 1;};", "int"}
+ };
+ for (Test T : Tests) {
+ Annotations File(T.AnnotatedCode);
+ auto AST = TestTU::withCode(File.code()).build();
+ ASSERT_TRUE(AST.getDiagnostics().empty()) << AST.getDiagnostics().begin()->Message;
+ SourceManagerForFile SM("foo.cpp", File.code());
+
+ for (Position Pos : File.points()) {
+ auto Location = sourceLocationInMainFile(SM.get(), Pos);
+ auto DeducedType = getDeducedType(AST, *Location);
+ EXPECT_EQ(DeducedType->getAsString(), T.DeductedType);
+ }
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -11,6 +11,7 @@
#include "TestTU.h"
#include "refactor/Tweak.h"
#include "clang/AST/Expr.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/StringRef.h"
@@ -19,6 +20,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <cassert>
+#include "refactor/tweaks/ExpandAutoType.h"
using llvm::Failed;
using llvm::Succeeded;
@@ -126,6 +128,19 @@
EXPECT_EQ(Output, std::string(*Result)) << Input;
}
+/// Check if apply returns an error and that the @ErrorMessage is contained
+/// in that error
+void checkApplyContainsError(llvm::StringRef ID, llvm::StringRef Input,
+ const std::string& ErrorMessage) {
+ auto Result = apply(ID, Input);
+ EXPECT_FALSE(Result) << "expected error message:\n " << ErrorMessage <<
+ "\non input:" << Input;
+ EXPECT_NE(std::string::npos,
+ llvm::toString(Result.takeError()).find(ErrorMessage))
+ << "Wrong error message:\n " << llvm::toString(Result.takeError())
+ << "\nexpected:\n " << ErrorMessage;
+}
+
TEST(TweakTest, SwapIfBranches) {
llvm::StringLiteral ID = "SwapIfBranches";
@@ -278,6 +293,164 @@
EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 | int x"));
}
+TEST(TweakTest, ExpandAutoType) {
+ llvm::StringLiteral ID = "ExpandAutoType";
+
+ checkAvailable(ID, R"cpp(
+ ^a^u^t^o^ i = 0;
+ )cpp");
+
+ checkNotAvailable(ID, R"cpp(
+ auto ^i^ ^=^ ^0^;^
+ )cpp");
+
+ llvm::StringLiteral Input = R"cpp(
+ [[auto]] i = 0;
+ )cpp";
+ llvm::StringLiteral Output = R"cpp(
+ int i = 0;
+ )cpp";
+ checkTransform(ID, Input, Output);
+
+ // check primitive type
+ Input = R"cpp(
+ au^to i = 0;
+ )cpp";
+ Output = R"cpp(
+ int i = 0;
+ )cpp";
+ checkTransform(ID, Input, Output);
+
+ // check classes and namespaces
+ Input = R"cpp(
+ namespace testns {
+ class TestClass {
+ class SubClass {};
+ };
+ }
+ ^auto C = testns::TestClass::SubClass();
+ )cpp";
+ Output = R"cpp(
+ namespace testns {
+ class TestClass {
+ class SubClass {};
+ };
+ }
+ testns::TestClass::SubClass C = testns::TestClass::SubClass();
+ )cpp";
+ checkTransform(ID, Input, Output);
+
+ // check that namespaces are shortened
+ Input = R"cpp(
+ namespace testns {
+ class TestClass {
+ };
+ void func() { ^auto C = TestClass(); }
+ }
+ )cpp";
+ Output = R"cpp(
+ namespace testns {
+ class TestClass {
+ };
+ void func() { TestClass C = TestClass(); }
+ }
+ )cpp";
+ checkTransform(ID, Input, Output);
+
+ // unknown types in a template should not be replaced
+ Input = R"cpp(
+ template <typename T> void x() {
+ ^auto y = T::z();
+ }
+ )cpp";
+ checkApplyContainsError(ID, Input, "Could not deduct type for 'auto' type");
+
+ // undefined functions should not be replaced
+ Input = R"cpp(
+ a^uto x = doesnt_exist();
+ )cpp";
+ checkApplyContainsError(ID, Input, "Could not deduct type for 'auto' type");
+
+ // lambda types are not replaced
+ Input = R"cpp(
+ au^to x = []{};
+ )cpp";
+ checkApplyContainsError(ID, Input,
+ "Could not expand type of lambda expression");
+
+ // local class
+ Input = R"cpp(
+ namespace x {
+ void y() {
+ struct S{};
+ a^uto z = S();
+ }}
+ )cpp";
+ Output = R"cpp(
+ namespace x {
+ void y() {
+ struct S{};
+ S z = S();
+ }}
+ )cpp";
+ checkTransform(ID, Input, Output);
+}
+
+TEST(ExpandAutoType, GetNamespaceString) {
+ struct EATWrapper : ExpandAutoType {
+ // to access the protected method
+ using ExpandAutoType::getNamespaceString;
+ };
+
+ auto GetNamespaceString = [](std::string CppCode) {
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ Annotations Code(CppCode);
+ TU.Code = Code.code();
+ ParsedAST AST = TU.build();
+ assert(AST.getDiagnostics().empty());
+ auto Tree = SelectionTree(
+ AST.getASTContext(),
+ static_cast<unsigned int>(*positionToOffset(Code.code(),
+ Code.point())));
+ return EATWrapper::getNamespaceString(Tree.commonAncestor());
+ };
+
+ ASSERT_EQ("firstns::secondns",
+ GetNamespaceString(
+ "namespace firstns{namespace secondns{ au^to i = 0;} }"));
+}
+
+TEST(ExpandAutoType, ShortenNamespace) {
+ struct EATWrapper : ExpandAutoType {
+ // to access the protected method
+ using ExpandAutoType::shortenNamespace;
+ };
+
+ ASSERT_EQ("TestClass", EATWrapper::shortenNamespace("TestClass", ""));
+
+ ASSERT_EQ("TestClass", EATWrapper::shortenNamespace(
+ "testnamespace::TestClass", "testnamespace"));
+
+ ASSERT_EQ(
+ "namespace1::TestClass",
+ EATWrapper::shortenNamespace("namespace1::TestClass", "namespace2"));
+
+ ASSERT_EQ("TestClass",
+ EATWrapper::shortenNamespace("testns1::testns2::TestClass",
+ "testns1::testns2"));
+
+ ASSERT_EQ(
+ "testns2::TestClass",
+ EATWrapper::shortenNamespace("testns1::testns2::TestClass", "testns1"));
+
+ // FIXME: template types are ignored right now, we should add this in the
+ // future
+ ASSERT_EQ("TestClass<testns1::OtherClass>",
+ EATWrapper::shortenNamespace(
+ "testns1::TestClass<testns1::OtherClass>", "testns1"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/test/code-action-request.test
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/code-action-request.test
@@ -0,0 +1,70 @@
+# RUN: clangd -log=verbose -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}}
+---
+{
+ "jsonrpc": "2.0",
+ "id": 1,
+ "method": "textDocument/codeAction",
+ "params": {
+ "textDocument": {
+ "uri": "test:///main.cpp"
+ },
+ "range": {
+ "start": {
+ "line": 0,
+ "character": 0
+ },
+ "end": {
+ "line": 0,
+ "character": 4
+ }
+ },
+ "context": {
+ "diagnostics": []
+ }
+ }
+}
+# CHECK: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "arguments": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "file": "file:///clangd-test/main.cpp",
+# CHECK-NEXT: "selection": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "tweakID": "ExpandAutoType"
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "command": "clangd.applyTweak",
+# CHECK-NEXT: "title": "expand auto type"
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+# CHECK: "newText": "int",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+---
\ No newline at end of file
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.h
@@ -0,0 +1,64 @@
+//===--- ExpandAutoType.h ----------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAKS_EXPANDAUTO_TYPE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAKS_EXPANDAUTO_TYPE_H
+#include "refactor/Tweak.h"
+
+namespace clang {
+namespace clangd {
+/// Expand the "auto" type to the derived type
+/// Before:
+/// auto x = Something();
+/// ^^^^
+/// After:
+/// MyClass x = Something();
+/// ^^^^^^^
+/// FIXME: Handle decltype as well
+class ExpandAutoType : public Tweak {
+public:
+ const char *id() const final;
+ Intent intent() const override { return Intent::Refactor;}
+ bool prepare(const Selection &Inputs) override;
+ Expected<Effect> apply(const Selection &Inputs) override;
+ std::string title() const override;
+
+protected:
+ /// Walk up the AST from the given Node to find the next AutoTypeLoc,
+ /// if it exists
+ const llvm::Optional<AutoTypeLoc>
+ findAutoType(const SelectionTree::Node *Node);
+
+ /// Get the namespaces of a node as string, e.g.
+ /// outernamespace::innernamespace.
+ static std::string getNamespaceString(const SelectionTree::Node *Node);
+
+ /// Try to shorten the OriginalName by removing namespaces from the left of
+ /// the string that are redundant in the CurrentNamespace. This way the type
+ /// idenfier become shorter and easier to read.
+ /// Limitation: It only handles the qualifier of the type itself, not that of
+ /// templates.
+ /// Example: shortenNamespace("ns1::MyClass<ns1::OtherClass>", "ns1")
+ /// --> "MyClass<ns1::OtherClass>"
+ static std::string shortenNamespace(llvm::StringRef OriginalName,
+ llvm::StringRef CurrentNamespace);
+
+private:
+ /// Cache the AutoTypeLoc, so that we do not need to search twice.
+ llvm::Optional<clang::AutoTypeLoc> CachedLocation;
+
+ // Create an error message with filename and line number in it
+ llvm::Error createErrorMessage(const std::string& Message,
+ const Selection &Inputs);
+
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -0,0 +1,134 @@
+//===--- ReplaceAutoType.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 "ExpandAutoType.h"
+
+#include "Logger.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
+#include <climits>
+#include <memory>
+#include <string>
+#include "XRefs.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(ExpandAutoType)
+
+bool ExpandAutoType::prepare(const Selection& Inputs) {
+ auto Node = Inputs.ASTSelection.commonAncestor();
+ CachedLocation = findAutoType(Node);
+ return CachedLocation != llvm::None;
+}
+
+Expected<Tweak::Effect> ExpandAutoType::apply(const Selection& Inputs) {
+ auto& SrcMgr = Inputs.AST.getASTContext().getSourceManager();
+
+ llvm::Optional<clang::QualType> DeductedType =
+ getDeducedType(Inputs.AST, CachedLocation->getBeginLoc());
+
+ // if we can't resolve the type, return an error message
+ if (DeductedType == llvm::None || DeductedType->isNull()) {
+ return createErrorMessage("Could not deduct type for 'auto' type", Inputs);
+ }
+
+ // if it's a lambda expression, return an error message
+ if (isa<RecordType>(*DeductedType) and
+ dyn_cast<RecordType>(*DeductedType)->getDecl()->isLambda()) {
+ return createErrorMessage("Could not expand type of lambda expression",
+ Inputs);
+ }
+
+ SourceRange OriginalRange(CachedLocation->getBeginLoc(),
+ CachedLocation->getEndLoc());
+ PrintingPolicy PP(Inputs.AST.getASTContext().getPrintingPolicy());
+ PP.SuppressTagKeyword = 1;
+ std::string PrettyTypeName = shortenNamespace(
+ DeductedType->getAsString(PP),
+ getNamespaceString(Inputs.ASTSelection.commonAncestor()));
+ tooling::Replacement Expansion(SrcMgr, CharSourceRange(OriginalRange, true),
+ PrettyTypeName);
+
+ return Tweak::Effect::applyEdit(tooling::Replacements(Expansion));
+}
+
+// try to find an 'auto' type location from the Selection
+const llvm::Optional<AutoTypeLoc>
+ExpandAutoType::findAutoType(const SelectionTree::Node* StartNode) {
+ auto Node = StartNode;
+ while (Node != nullptr) {
+ auto TypeNode = Node->ASTNode.get<TypeLoc>();
+ if (TypeNode) {
+ if (const AutoTypeLoc Result = TypeNode->getAs<AutoTypeLoc>()) {
+ return Result;
+ }
+ }
+ Node = Node->Parent;
+ }
+ return llvm::None;
+}
+
+std::string ExpandAutoType::title() const { return "expand auto type"; }
+
+std::string
+ExpandAutoType::shortenNamespace(const llvm::StringRef OriginalName,
+ const llvm::StringRef CurrentNamespace) {
+ llvm::SmallVector<llvm::StringRef, 8> OriginalParts;
+ llvm::SmallVector<llvm::StringRef, 8> CurrentParts;
+ llvm::SmallVector<llvm::StringRef, 8> Result;
+ OriginalName.split(OriginalParts, "::");
+ CurrentNamespace.split(CurrentParts, "::");
+ auto MinLength = std::min(CurrentParts.size(), OriginalParts.size());
+
+ u_int DifferentAt = 0;
+ while (DifferentAt < MinLength &&
+ CurrentParts[DifferentAt] == OriginalParts[DifferentAt]) {
+ DifferentAt++;
+ }
+
+ for (u_int i = DifferentAt; i < OriginalParts.size(); ++i) {
+ Result.push_back(OriginalParts[i]);
+ }
+ return join(Result, "::");
+}
+
+std::string ExpandAutoType::getNamespaceString(
+ const SelectionTree::Node* StartNode) {
+ auto Node = StartNode;
+ while (Node != nullptr) {
+ LLVM_DEBUG(Node->ASTNode.print());
+ if (const Decl* Current = Node->ASTNode.get<Decl>()) {
+ if (auto CurrentNameSpace = dyn_cast<NamespaceDecl>(Current)) {
+ return CurrentNameSpace->getQualifiedNameAsString();
+ }
+ }
+ Node = Node->Parent;
+ }
+ return "";
+}
+
+llvm::Error ExpandAutoType::createErrorMessage(const std::string& Message,
+ const Selection& Inputs) {
+ auto& SrcMgr = Inputs.AST.getASTContext().getSourceManager();
+ std::string ErrorMessage =
+ Message + ": " +
+ SrcMgr.getFilename(Inputs.Cursor).str() + " Line " +
+ std::to_string(SrcMgr.getExpansionLineNumber(Inputs.Cursor));
+
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ ErrorMessage.c_str());
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -15,6 +15,7 @@
DumpAST.cpp
RawStringLiteral.cpp
SwapIfBranches.cpp
+ ExpandAutoType.cpp
LINK_LIBS
clangAST
Index: clang-tools-extra/clangd/XRefs.h
===================================================================
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -140,6 +140,16 @@
ParsedAST &AST, Position Pos, int Resolve, TypeHierarchyDirection Direction,
const SymbolIndex *Index = nullptr, PathRef TUPath = PathRef{});
+/// Retrieves the deduced type at a given location (auto, decltype).
+/// SourceLocationBeg must point to the first character of the token
+llvm::Optional<QualType> getDeducedType(ParsedAST &AST,
+ SourceLocation SourceLocationBeg);
+
+/// Retrieves the deduced type at a given location (auto, decltype).
+/// SourceLocationBeg must point to the first character of the token
+llvm::Optional<QualType> getDeducedType(ParsedAST &AST,
+ SourceLocation SourceLocationBeg);
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -868,7 +868,9 @@
} // namespace
/// Retrieves the deduced type at a given location (auto, decltype).
-bool hasDeducedType(ParsedAST &AST, SourceLocation SourceLocationBeg) {
+/// SourceLocationBeg must point to the first character of the token
+llvm::Optional<QualType> getDeducedType(ParsedAST &AST,
+ SourceLocation SourceLocationBeg) {
Token Tok;
auto &ASTCtx = AST.getASTContext();
// Only try to find a deduced type if the token is auto or decltype.
@@ -876,12 +878,20 @@
Lexer::getRawToken(SourceLocationBeg, Tok, ASTCtx.getSourceManager(),
ASTCtx.getLangOpts(), false) ||
!Tok.is(tok::raw_identifier)) {
- return false;
+ return {};
}
AST.getPreprocessor().LookUpIdentifierInfo(Tok);
if (!(Tok.is(tok::kw_auto) || Tok.is(tok::kw_decltype)))
- return false;
- return true;
+ return {};
+
+ DeducedTypeVisitor V(SourceLocationBeg);
+ V.TraverseAST(AST.getASTContext());
+ return V.DeducedType;
+}
+
+/// Retrieves the deduced type at a given location (auto, decltype).
+bool hasDeducedType(ParsedAST &AST, SourceLocation SourceLocationBeg) {
+ return (bool) getDeducedType(AST, SourceLocationBeg);
}
llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -17,6 +17,7 @@
#include "clang/AST/Decl.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/MacroInfo.h"
+#include "clang/AST/RecursiveASTVisitor.h"
namespace clang {
class SourceManager;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
