[clangd] DefineInline action availability checks

[kadircet]

Summary

Introduces DefineInline action and initial version of availability
checks.

[kadircet]

• Rebase

[sammccall]

clang-tools-extra/clangd/unittests/TweakTests.cpp

53

This function, and all the other helpers in this file, are actually imminently going away in favor of TweakTesting.h.
Sorry it's been stuck in my backlog for a couple of weeks...

We should work out how to do that stuff there.
One approach is to give TweakTest a StringMap<string> ExtraFiles and StringMap<string> EditedFiles that excludes main, so you'd write

ExtraFiles["foo.h"] = "...";
EXPECT_EQ(apply("..."), "...");
EXPECT_THAT(EditedFiles, ElementsAre(Pair(testPath("foo.h"), "...");

or something like this.

(Whatever we do, we should bear in mind the fact that out-line will mean we need to supply an index too)

545

I don't think so

567

nit: NoForwardDecl

597

FWIW, I find these params rather cryptic - it'd be nice to have an API for the helpers that is explicit about this being files/filenames

[kadircet]

• Rebase and address comments

[arphaman]

When fully implemented, will define inline tweak work with C++ methods in classes as well?
E.g.

HEADER:
class Foo { void foo(); }
CPP:
#include "Header.h"
void Foo::foo() {}

becomes:

HEADER:
class Foo { void foo() { } }
CPP:
#include "Header.h"

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

192

Sorry, I'm not really familiar with design of tweaks, so this might be a bad question: Is it possible to change the title of the tweak on a per-TU basis. More specifically, some tweaks might want to have different titles for Objective-C actions compared to their C++ siblings.

[kadircet]

In D65433#1661865, @arphaman wrote:

When fully implemented, will define inline tweak work with C++ methods in classes as well?

Yes you can see an example of it in the tests provided in TweakTests.cpp

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

192

Current design returns a constant string per tweak, but we can change that behaviour easily by making this a function of language options. You can take a look at ClangdServer::enumerateTweaks for details.

[sammccall]

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

192

This isn't quite true: id() must be constant but title() need not be. See ExpandMacro for an example.

The only constraint is it shouldn't be terribly expensive (this is why ExpandAuto doesn't include the type in the title, due to an AST bug it requires a new traversal).

More specifically, some tweaks might want to have different titles for Objective-C actions compared to their C++ siblings.

I think this is possible and the right idea. But for completeness I should mention: having multiple Tweak subclasses that share implementation is also possible. In particular in cases where you have two related actions and may want to provide *both* as options (maybe extract function vs method?), they'd need to be separate subclasses. (Or we'd need to change the design a bit)

[arphaman]

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

192

Oh yes, I think it's probably better to use the Tweak subclasses like you said. This will be better for ObjC++, which could have either a C++ or an ObjC tweak available depending on the current AST selection.

[hokein]

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

65

nit: maybe use auto?

78

thinking this a bit more, the function is non-trivial, we probably want unit test for it.

105

IIUC, I believe this function relies on the flag "Opts.IndexFunctionLocals"? could we spell it explicitly in the code even its default value false fits our use case.

113

hmm, looks like we don't use TargetContext in the implemenation at all. Is the comment out-of-date now?

141

we are distinguishing two cases here: normal functions and the class methods. Could we make the code handle these two cases separately? e.g.

if (Class) {
   // check whether they are in the same class;
} else { // handle normal function
  // call isBeforeInTranslation
}

160

function template is tricky, I may be not aware of the context, what's our plan for supporting it? could you give an concrete example? it seems that the current unit test doesn't cover it.

172

I believe it also works for class methods?

183

now we get a potential ODR violation in this example, maybe choose a different example?

224

The tweak is under development, do we need to hide it until it is completed?

clang-tools-extra/clangd/unittests/TweakTests.cpp

25

the header seems irrelevant to me.

709

The a.h seems to be non-existed unless you explicitly set it via ExtraFiles["a.h"]?

[kadircet]

• Address comments

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

78

agreed, sent out D67748

113

we are still checking its visibility in target context, but without making use of declcontext. Updating the comment.

160

Yes we plan to support them, added a test case thanks for pointing it out.

This basically ensures that we select the correct decl in the case of template spec decls, because canonical decl is function template itself, instead of the specialization decl.

183

You are right, but we cannot get away from it by changing the example. Adding an "inline " instead, and I believe that's what we should do if we are moving a function definition to a header.

224

I wasn't planning to land it before all of its dependencies are LGTM'd, but marking it as hidden just in case.

[hokein]

mostly good to me, a few more comments.

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

156

maybe name it findTarget? I think the function is used to find a potential target decl which the definition will be moved to, Canonical doesn't provide much information here.

160

We may get a nullptr if FD is the first declaration, I think we need to handle the nullptr case, otherwise the check will crash.

183

I think not all cases will need inline, e.g. class method, or function template(?). Fix these problems is out-scope of the tweak (there is a clang-tidy check handling this case), and probably add complexity to the implementation. I'm leaning on not fixing it.

Maybe a better example is class method?

185

I'm not sure Inline function definition is a widely-known refactoring term, I think "Move function definition to declaration" is probably easier for normal C++ developers to understand?

clang-tools-extra/clangd/unittests/TweakTests.cpp

713

I think we missing a basic test where a declaration is in the header and it can be moved?

Header = "void foo();"
Main = "void f^oo() { // no dependency code }"

[kadircet]

• Address comments

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

160

theoretically, a template specialization cannot be the first decl, as one needs to declare function template first.
making this more clear by changing this into a loop that finds the first forward declaration for the specialization.

183

yes clang-tidy can generate those fixes but I believe a code-action should not generate code that relies on a clang-tidy check to fix it.

clang-tools-extra/clangd/unittests/TweakTests.cpp

713

isn't it covered by the next testcase ?

[hokein]

looks good with a few nits.

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

154

nit: the comment is out of date with the new change?

183

I'm still not convinced that we should fix it in the check. but we don't have to solve it out in this patch, could we remove the inline from the sample? (adding it now seems to confuse users, we don't support it yet).

clang-tools-extra/clangd/unittests/TweakTesting.h

52

nit: add a comment // file name => file content.

clang-tools-extra/clangd/unittests/TweakTests.cpp

713

ah, sorry, I missed that.

[kadircet]

• Rebase on top of D67748

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

183

we basically support nothing in that sample in this patch, it is just to reflect the end-state of the code-action.
but as you wish; deleting it now, and instead adding with the next patch(that implements apply).

[kadircet]

• Rebase and update testhelpers

[kadircet]

• Revert miscommit

[kadircet]

• Use canonical decl when checking visibility

[kadircet]

• Rebase and bail out on methods inside templated classes

[thakis]

The test fails in Windows: http://45.33.8.238/win/1112/step_7.txt

Ptal, and if it takes a while to investigate please revert while you look.

clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

 //===--- DefineInline.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 "AST.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "XRefs.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"

 #include <string>
 #include <unordered_map>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
+// Lexes from end of \p FD until it finds a semicolon.
+llvm::Optional<SourceLocation> getSemiColonForDecl(const FunctionDecl *FD) {
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  const LangOptions &LangOpts = FD->getASTContext().getLangOpts();
+
+  SourceLocation SemiColon =
+      Lexer::getLocForEndOfToken(FD->getEndLoc(), 0, SM, LangOpts);
+  llvm::StringRef BufData = SM.getBufferData(SM.getFileID(SemiColon));
+  Lexer RawLexer(SM.getLocForStartOfFile(SM.getFileID(SemiColon)), LangOpts,
+                 BufData.begin(), SM.getCharacterData(SemiColon),
+                 BufData.end());
+  Token CurToken;
+  while (!CurToken.is(tok::semi)) {
+    if (RawLexer.LexFromRawLexer(CurToken))
+      return llvm::None;
+  }
+  SemiColon = CurToken.getLocation();
+  return SemiColon;
+}
+
 // Deduces the FunctionDecl from a selection. Requires either the function body
 // or the function decl to be selected. Returns null if none of the above
 // criteria is met.
 const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
   const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode;
   if (const FunctionDecl *FD = AstNode.get<FunctionDecl>())
     return FD;
   if (AstNode.get<CompoundStmt>() &&
       SelNode->Selected == SelectionTree::Complete) {
     if (const SelectionTree::Node *P = SelNode->Parent)
       return P->ASTNode.get<FunctionDecl>();
   }
   return nullptr;
 }
 
 // Checks the decls mentioned in Source are visible in the context of Target.
 // Achives that by checking declarations occur before target location in
 // translation unit or declared in the same class.
 bool checkDeclsAreVisible(const llvm::DenseSet<const Decl *> &DeclRefs,
                           const FunctionDecl *Target, const SourceManager &SM) {
   SourceLocation TargetLoc = Target->getLocation();
   // To be used in visibility check below, decls in a class are visible
   // independent of order.
   const RecordDecl *Class = nullptr;
   if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(Target))
     Class = MD->getParent();
 
   for (const auto *D : DeclRefs) {
     if (D == Target)
       continue;
     SourceLocation DeclLoc = D->getLocation();
 
     // FIXME: Allow declarations from different files with include insertion.
     if (!SM.isWrittenInSameFile(DeclLoc, TargetLoc))
       return false;
 
     // If declaration is before target, then it is visible.
     if (SM.isBeforeInTranslationUnit(DeclLoc, TargetLoc))
       continue;
 
     // Otherwise they need to be in same class
     if (!Class)
       return false;
     const RecordDecl *Parent = nullptr;
     if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(D))
       Parent = MD->getParent();
     else if (const auto *FD = llvm::dyn_cast<FieldDecl>(D))
       Parent = FD->getParent();
     if (Parent != Class)
       return false;
   }
   return true;
 }
 
+// Rewrites body of FD to fully-qualify all of the decls inside.
+llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD) {
+  // There are three types of spellings that needs to be qualified in a function
+  // body:
+  // - Types:       Foo                 -> ns::Foo
+  // - DeclRefExpr: ns2::foo()          -> ns1::ns2::foo();
+  // - UsingDecls:
+  //    using ns2::foo      -> using ns1::ns2::foo
+  //    using namespace ns2 -> using namespace ns1::ns2
+  //    using ns3 = ns2     -> using ns3 = ns1::ns2
+  //
+  // Go over all references inside a function body to generate replacements that
+  // will fully qualify those. So that body can be moved into an arbitrary file.
+  // We perform the qualification by qualyfying the first type/decl in a
+  // (un)qualified name. e.g:
+  //    namespace a { namespace b { class Bar{}; void foo(); } }
+  //    b::Bar x; -> a::b::Bar x;
+  //    foo(); -> a::b::foo();
+  // FIXME: Instead of fully qualyfying we should try deducing visible scopes at
+  // target location and generate minimal edits.
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  tooling::Replacements Replacements;
+  findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {
+    // Since we want to qualify only the first qualifier, skip names with a
+    // qualifier.
+    if (Ref.Qualifier.hasQualifier())
+      return;
+    // There might be no decl in dependent contexts, there's nothing much we can
+    // do in such cases.
+    if (Ref.Targets.empty())
+      return;
+
+    // All Targets should be in the same nested name scope, so we can safely
+    // chose any one of them.
+    const NamedDecl *ND = Ref.Targets.front();
+    // Skip local symbols.
+    if (index::isFunctionLocalSymbol(ND))
+      return;
+    // Skip members as qualyfying the left side is enough.
+    if (ND->isCXXInstanceMember())
+      return;
+
+    std::string Qualifier = printNamespaceScope(*ND->getDeclContext());
+
+    if (auto Err = Replacements.add(
+            tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
+      elog("Failed to add qualifier: {0}", std::move(Err));
+  });
+
+  auto QualifiedFunc = tooling::applyAllReplacements(
+      SM.getBufferData(SM.getFileID(FD->getLocation())), Replacements);
+  if (!QualifiedFunc)
+    return QualifiedFunc.takeError();
+
+  // Get new begin and end positions for the qualified body.
+  SourceRange OrigFuncRange = FD->getBody()->getSourceRange();
+  unsigned BodyBegin = SM.getFileOffset(OrigFuncRange.getBegin());
+  unsigned BodyEnd = Replacements.getShiftedCodePosition(
+      SM.getFileOffset(OrigFuncRange.getEnd()));
+
+  // Trim the result to function body.
+  return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
+}
+
 // Returns the canonical declaration for the given FunctionDecl. This will
 // usually be the first declaration in current translation unit with the
 // exception of template specialization.
 // For those we return first declaration different than the canonical one.
 // Because canonical declaration points to template decl instead of
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
   if (!FD->isFunctionTemplateSpecialization())
     return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find
   // the first forward decl for this specialization.
   auto PrevDecl = FD;
   while (PrevDecl->getPreviousDecl() != CanonDecl) {
     PrevDecl = PrevDecl->getPreviousDecl();
     assert(PrevDecl && "Found specialization without template decl");
   }
   return PrevDecl;
 }
 
+// Returns the begining location for a FunctionDecl. Returns location of
+// template keyword for templated functions.
+const SourceLocation getBeginLoc(const FunctionDecl *FD) {
+  // Include template parameter list.
+  if (auto *FTD = FD->getDescribedFunctionTemplate())
+    return FTD->getBeginLoc();
+  return FD->getBeginLoc();
+}
+
 /// Moves definition of a function/method to its declaration location.
 /// Before:
 /// a.h:
 ///   void foo();
 ///
 /// a.cc:
 ///   void foo() { return; }
 ///
bool prepare(const Selection &Sel) override {
     if (!checkDeclsAreVisible(getNonLocalDeclRefs(Sel.AST, Source), Target,
                               Sel.AST.getSourceManager()))
       return false;
 
     return true;
   }
 
   Expected<Effect> apply(const Selection &Sel) override {
-    return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                   "Not implemented yet");
+    const ASTContext &AST = Sel.AST.getASTContext();
+    const SourceManager &SM = AST.getSourceManager();
+
+    auto QualifiedBody = qualifyAllDecls(Source);
+    if (!QualifiedBody)
+      return QualifiedBody.takeError();
+
+    auto SemiColon = getSemiColonForDecl(Target);
+    if (!SemiColon) {
+      return llvm::createStringError(
+          llvm::inconvertibleErrorCode(),
+          "Couldn't find semicolon for target declaration");
+    }
+
+    tooling::Replacements Replacements;
+    auto Err = Replacements.add(
+        tooling::Replacement(SM, *SemiColon, 1, *QualifiedBody));
+    if (Err)
+      return std::move(Err);
+    Effect E;
+
+    // We need to generate two edits if the Source and Target are in different
+    // files.
+    if (!SM.isWrittenInSameFile(Source->getBeginLoc(), Target->getBeginLoc())) {
+      auto FE = Effect::fileEdit(SM, SM.getFileID(*SemiColon),
+                                 std::move(Replacements));
+      if (!FE)
+        return FE.takeError();
+      E.ApplyEdits.try_emplace(FE->first, std::move(FE->second));
+      Replacements.clear();
+    }
+
+    SourceLocation BeginLoc = getBeginLoc(Source);
+    unsigned int SourceLen =
+        SM.getFileOffset(Source->getEndLoc()) - SM.getFileOffset(BeginLoc) + 1;
+    Err = Replacements.add(tooling::Replacement(SM, BeginLoc, SourceLen, ""));
+    if (Err)
+      return std::move(Err);
+
+    auto FE =
+        Effect::fileEdit(SM, SM.getFileID(Sel.Cursor), std::move(Replacements));
+    if (!FE)
+      return FE.takeError();
+    E.ApplyEdits.try_emplace(FE->first, std::move(FE->second));
+    return E;
   }
 
 private:
   const FunctionDecl *Source = nullptr;
   const FunctionDecl *Target = nullptr;
 };
 
 REGISTER_TWEAK(DefineInline);
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/TweakTesting.h

enum CodeContext {
     // Code snippet is placed directly into the source file. e.g. a declaration.
     File,
     // Snippet will appear within a function body. e.g. a statement.
     Function,
     // Snippet is an expression.
     Expression,
   };
 
   // Mapping from file name to contents.
   llvm::StringMap<std::string> ExtraFiles;
+  llvm::StringMap<std::string> EditedFiles;
 
 protected:
   TweakTest(const char *TweakID) : TweakID(TweakID) {}
 
   // Contents of a header file to be implicitly included.
   // This typically contains declarations that will be used for a set of related
   // testcases.
   std::string Header;
 
   // Context in which snippets of code should be placed to run tweaks.
   CodeContext Context = File;
 
   // Apply the current tweak to the range (or point) in MarkedCode.
   // MarkedCode will be wrapped according to the Context.
-  //  - if the tweak produces edits, returns the edited code (without markings).
+  //  - if the tweak produces edits, returns the edited code (without markings)
+  //    for the main file. Populates \p EditedFiles if there were any other
+  //    changes.
   //    The context added to MarkedCode will be stripped away before returning,
   //    unless the tweak edited it.
   //  - if the tweak produces a message, returns "message:\n<message>"
   //  - if prepare() returns false, returns "unavailable"
   //  - if apply() returns an error, returns "fail: <message>"
-  std::string apply(llvm::StringRef MarkedCode) const;
+  std::string apply(llvm::StringRef MarkedCode);
 
   // Accepts a code snippet with many ranges (or points) marked, and returns a
   // list of snippets with one range marked each.
   // Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
   static std::vector<std::string> expandCases(llvm::StringRef MarkedCode);
 
   // Returns a matcher that accepts marked code snippets where the tweak is
   // available at the marked range.

clang-tools-extra/clangd/unittests/TweakTesting.cpp

 //===-- TweakTesting.cpp ------------------------------------------------*-===//
 //
 // 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 "TweakTesting.h"
 
 #include "Annotations.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
+#include <string>
 
 namespace clang {
 namespace clangd {
 namespace {
 using Context = TweakTest::CodeContext;
 
 std::pair<llvm::StringRef, llvm::StringRef> wrapping(Context Ctx) {
   switch (Ctx) {
MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles,
   if (PrepareResult)
     return true;
   llvm::consumeError(PrepareResult.takeError());
   return false;
 }
 
 } // namespace
 
-std::string TweakTest::apply(llvm::StringRef MarkedCode) const {
+std::string TweakTest::apply(llvm::StringRef MarkedCode) {
   std::string WrappedCode = wrap(Context, MarkedCode);
   Annotations Input(WrappedCode);
   auto Selection = rangeOrPoint(Input);
   TestTU TU;
   TU.HeaderCode = Header;
+  TU.AdditionalFiles = std::move(ExtraFiles);
   TU.Code = Input.code();
   ParsedAST AST = TU.build();
   Tweak::Selection S(AST, Selection.first, Selection.second);
 
   auto T = prepareTweak(TweakID, S);
   if (!T) {
     llvm::consumeError(T.takeError());
     return "unavailable";
   }
   llvm::Expected<Tweak::Effect> Result = (*T)->apply(S);
   if (!Result)
     return "fail: " + llvm::toString(Result.takeError());
   if (Result->ShowMessage)
     return "message:\n" + *Result->ShowMessage;
   if (Result->ApplyEdits.empty())
     return "no effect";
-  if (Result->ApplyEdits.size() > 1)
-    return "received multi-file edits";
 
-  auto ApplyEdit = Result->ApplyEdits.begin()->second;
-  if (auto NewText = ApplyEdit.apply())
-    return unwrap(Context, *NewText);
-  else
+  std::string EditedMainFile;
+  for (auto &It : Result->ApplyEdits) {
+    auto NewText = It.second.apply();
+    if (!NewText)
       return "bad edits: " + llvm::toString(NewText.takeError());
+    llvm::StringRef Unwrapped = unwrap(Context, *NewText);
+    if (It.first() == testPath(TU.Filename))
+      EditedMainFile = Unwrapped;
+    else
+      EditedFiles.try_emplace(It.first(), Unwrapped.str());
+  }
+  return EditedMainFile;
 }
 
 ::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
   return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header,
                           ExtraFiles);
 }
 
 std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {

clang-tools-extra/clangd/unittests/TweakTests.cpp

 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cassert>
 #include <string>
 #include <vector>
 
 using ::testing::AllOf;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
 
 namespace clang {
 namespace clangd {
 namespace {
 
 TEST(FileEdits, AbsolutePath) {
   auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
 
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> MemFS(
       new llvm::vfs::InMemoryFileSystem);
   MemFS->setCurrentWorkingDirectory(testRoot());
   for (auto Path : RelPaths)
     MemFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("", Path));
   FileManager FM(FileSystemOptions(), MemFS);
   DiagnosticsEngine DE(new DiagnosticIDs, new DiagnosticOptions);
   SourceManager SM(DE, FM);
 
   for (auto Path : RelPaths) {

 TEST_F(RawStringLiteralTest, Test) {
   Context = Expression;
   EXPECT_AVAILABLE(R"cpp(^"^f^o^o^\^n^")cpp");
   EXPECT_AVAILABLE(R"cpp(R"(multi )" ^"token " "str\ning")cpp");
   EXPECT_UNAVAILABLE(R"cpp(^"f^o^o^o")cpp"); // no chars need escaping
   EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
   EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
   EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
-  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp");           // forbidden escape char
 
   const char *Input = R"cpp(R"(multi
 token)" "\nst^ring\n" "literal")cpp";
   const char *Output = R"cpp(R"(multi
 token
 string
 literal)")cpp";
   EXPECT_EQ(apply(Input), Output);

 TWEAK_TEST(ExtractFunction);
 TEST_F(ExtractFunctionTest, FunctionTest) {
   Context = Function;
 
   // Root statements should have common parent.
   EXPECT_EQ(apply("for(;;) [[1+2; 1+2;]]"), "unavailable");
   // Expressions aren't extracted.
   EXPECT_EQ(apply("int x = 0; [[x++;]]"), "unavailable");
   // We don't support extraction from lambdas.
   EXPECT_EQ(apply("auto lam = [](){ [[int x;]] }; "), "unavailable");
 
   // Ensure that end of Zone and Beginning of PostZone being adjacent doesn't
   // lead to break being included in the extraction zone.
   EXPECT_THAT(apply("for(;;) { [[int x;]]break; }"), HasSubstr("extracted"));
   // FIXME: This should be unavailable since partially selected but
   // selectionTree doesn't always work correctly for VarDecls.
   EXPECT_THAT(apply("int [[x = 0]];"), HasSubstr("extracted"));
   // FIXME: ExtractFunction should be unavailable inside loop construct
   // initalizer/condition.
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
   // Don't extract because needs hoisting.
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
   // Check all parameters are in order
   std::string ParameterCheckInput = R"cpp(
 struct Foo {
   int x;
 };
 void f(int a) {
   int b;
   int *ptr = &a;
   Foo foo;
   [[a += foo.x + b;
   *ptr++;]]
 })cpp";

 })cpp";
   EXPECT_EQ(apply(ParameterCheckInput), ParameterCheckOutput);
 
   // Check const qualifier
   std::string ConstCheckInput = R"cpp(
 void f(const int c) {
   [[while(c) {}]]
 })cpp";
   std::string ConstCheckOutput = R"cpp(
 void extracted(const int &c) {
 while(c) {}
 }
 void f(const int c) {
   extracted(c);
 })cpp";
   EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
 
EXPECT_AVAILABLE(R"cpp(
     void fo^o(int baz) {
       int x = 10;
       bar();
     })cpp");
 
   // Internal symbol usage.
   Header = "void foo(int test);";
   EXPECT_UNAVAILABLE(R"cpp(
     void bar();
     void fo^o(int baz) {
       int x = 10;
       bar();
     })cpp");
 
   // Becomes available after making symbol visible.
   Header = "void bar();" + Header;
   EXPECT_AVAILABLE(R"cpp(
     void fo^o(int baz) {
       int x = 10;
       bar();
     })cpp");
EXPECT_AVAILABLE(R"cpp(
     void bar();
     template <> void foo<int>();
 
     template<> void f^oo<int>() {
       bar();
     })cpp");
 }
 
+TEST_F(DefineInlineTest, TransformUsings) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+  void bar();
+  namespace b {
+  void baz();
+  namespace c {
+  void aux();
+  }
+  }
+  }
+
+  void foo();
+  void f^oo() {
+    using namespace a;
+
+    using namespace b;
+    using namespace a::b;
+
+    using namespace c;
+    using namespace b::c;
+    using namespace a::b::c;
+
+    using a::bar;
+
+    using b::baz;
+    using a::b::baz;
+
+    using c::aux;
+    using b::c::aux;
+    using a::b::c::aux;
+
+    namespace d = c;
+    namespace d = b::c;
+  }
+  )cpp"),
+            R"cpp(
+  namespace a {
+  void bar();
+  namespace b {
+  void baz();
+  namespace c {
+  void aux();
+  }
+  }
+  }
+
+  void foo(){
+    using namespace a;
+
+    using namespace a::b;
+    using namespace a::b;
+
+    using namespace a::b::c;
+    using namespace a::b::c;
+    using namespace a::b::c;
+
+    using a::bar;
+
+    using a::b::baz;
+    using a::b::baz;
+
+    using a::b::c::aux;
+    using a::b::c::aux;
+    using a::b::c::aux;
+
+    namespace d = a::b::c;
+    namespace d = a::b::c;
+  }
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  void foo()    /*Comment -_-*/  ;
+
+  void f^oo() {
+    class Foo {
+    public:
+      void foo();
+      int x;
+      static int y;
+    };
+    Foo::y = 0;
+
+    enum En { Zero, One };
+    En x = Zero;
+
+    enum class EnClass { Zero, One };
+    EnClass y = EnClass::Zero;
+
+    template <typename T> class Bar {};
+    Bar<int> z;
+  }
+  )cpp"),
+            R"cpp(
+  void foo()    /*Comment -_-*/  {
+    class Foo {
+    public:
+      void foo();
+      int x;
+      static int y;
+    };
+    Foo::y = 0;
+
+    enum En { Zero, One };
+    En x = Zero;
+
+    enum class EnClass { Zero, One };
+    EnClass y = EnClass::Zero;
+
+    template <typename T> class Bar {};
+    Bar<int> z;
+  }
+
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      void bar();
+    };
+    template <typename T> T bar;
+    template <typename T> void aux() {}
+  }
+
+  void foo()    /*Comment -_-*/  ;
+
+  void f^oo() {
+    using namespace a;
+    bar<Bar<int>>.bar();
+    aux<Bar<int>>();
+  }
+  )cpp"),
+            R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      void bar();
+    };
+    template <typename T> T bar;
+    template <typename T> void aux() {}
+  }
+
+  void foo()    /*Comment -_-*/  {
+    using namespace a;
+    a::bar<a::Bar<int>>.bar();
+    a::aux<a::Bar<int>>();
+  }
+
+  
+  )cpp");
+}
+
+MATCHER_P2(FileWithContents, FileName, Contents, "") {
+  return arg.first() == FileName && arg.second == Contents;
+}
+
+TEST_F(DefineInlineTest, TransformMembers) {
+  EXPECT_EQ(apply(R"cpp(
+  class Foo {
+    void foo()    /*Comment -_-*/  ;
+  };
+
+  void Foo::f^oo() {
+    return;
+  }
+  )cpp"),
+            R"cpp(
+  class Foo {
+    void foo()    /*Comment -_-*/  {
+    return;
+  }
+  };
+
+  
+  )cpp");
+
+  ExtraFiles["a.h"] = R"cpp(
+  class Foo {
+    void foo()    /*Comment -_-*/  ;
+  };)cpp";
+  EXPECT_EQ(apply(R"cpp(
+  #include "a.h"
+  void Foo::f^oo() {
+    return;
+  })cpp"),
+            R"cpp(
+  #include "a.h"
+  )cpp");
+  EXPECT_THAT(EditedFiles,
+              testing::ElementsAre(FileWithContents(testPath("a.h"),
+                                                    R"cpp(
+  class Foo {
+    void foo()    /*Comment -_-*/  {
+    return;
+  }
+  };)cpp")));
+}
+
+TEST_F(DefineInlineTest, TransformDependentTypes) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+    template <typename T> class Bar {};
+  }
+  using namespace a;
+
+  template <typename T>
+  void foo()    /*Comment -_-*/  ;
+
+  template <typename T>
+  void f^oo() {
+    Bar<T> B;
+    Bar<Bar<T>> q;
+  }
+  )cpp"),
+            R"cpp(
+  namespace a {
+    template <typename T> class Bar {};
+  }
+  using namespace a;
+
+  template <typename T>
+  void foo()    /*Comment -_-*/  {
+    a::Bar<T> B;
+    a::Bar<a::Bar<T>> q;
+  }
+
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformFunctionTempls) {
+  EXPECT_EQ(apply(R"cpp(
+  template <typename T>
+  void foo(T p);
+
+  template <>
+  void foo<int>(int p);
+
+  template <>
+  void foo<char>(char p);
+
+  template <>
+  void fo^o<int>(int p) {
+    return;
+  }
+  )cpp"),
+            R"cpp(
+  template <typename T>
+  void foo(T p);
+
+  template <>
+  void foo<int>(int p){
+    return;
+  }
+
+  template <>
+  void foo<char>(char p);
+
+  
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  template <typename T>
+  void foo(T p);
+
+  template <>
+  void foo<int>(int p);
+
+  template <>
+  void foo<char>(char p);
+
+  template <>
+  void fo^o<char>(char p) {
+    return;
+  }
+  )cpp"),
+            R"cpp(
+  template <typename T>
+  void foo(T p);
+
+  template <>
+  void foo<int>(int p);
+
+  template <>
+  void foo<char>(char p){
+    return;
+  }
+
+  
+  )cpp");
+
+  EXPECT_EQ(apply(R"cpp(
+  template <typename T>
+  void foo(T p);
+
+  template <>
+  void foo<int>(int p);
+
+  template <typename T>
+  void fo^o(T p) {
+    return;
+  }
+  )cpp"),
+            R"cpp(
+  template <typename T>
+  void foo(T p){
+    return;
+  }
+
+  template <>
+  void foo<int>(int p);
+
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTypeLocs) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      template <typename Q> class Baz {};
+    };
+    namespace b{
+    class Foo{};
+    };
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+
+  void foo()    /*Comment -_-*/  ;
+
+  void f^oo() {
+    Bar<int> B;
+    b::Foo foo;
+    a::Bar<Bar<int>>::Baz<Bar<int>> q;
+  }
+  )cpp"),
+            R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      template <typename Q> class Baz {};
+    };
+    namespace b{
+    class Foo{};
+    };
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+
+  void foo()    /*Comment -_-*/  {
+    a::Bar<int> B;
+    a::b::Foo foo;
+    a::Bar<a::Bar<int>>::Baz<a::Bar<int>> q;
+  }
+
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDeclRefs) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      void foo();
+      static void bar();
+      int x;
+      static int y;
+    };
+    void bar();
+    namespace b {
+    class Foo{};
+    namespace c {
+    void test();
+    }
+    }
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+
+  void foo()    /*Comment -_-*/  ;
+
+  void f^oo() {
+    a::Bar<int> B;
+    B.foo();
+    a::bar();
+    Bar<Bar<int>>::bar();
+    a::Bar<int>::bar();
+    B.x = Bar<int>::y;
+    Bar<int>::y = 3;
+    bar();
+    c::test();
+  }
+  )cpp"),
+            R"cpp(
+  namespace a {
+    template <typename T> class Bar {
+    public:
+      void foo();
+      static void bar();
+      int x;
+      static int y;
+    };
+    void bar();
+    namespace b {
+    class Foo{};
+    namespace c {
+    void test();
+    }
+    }
+  }
+  using namespace a;
+  using namespace b;
+  using namespace c;
+
+  void foo()    /*Comment -_-*/  {
+    a::Bar<int> B;
+    B.foo();
+    a::bar();
+    a::Bar<a::Bar<int>>::bar();
+    a::Bar<int>::bar();
+    B.x = a::Bar<int>::y;
+    a::Bar<int>::y = 3;
+    a::bar();
+    a::b::c::test();
+  }
+
+  
+  )cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang