[clangd] No ExtractFunction on empty selections.

[kadircet]

this one is actually specific to vardecl's and has a pending fix in D66872

I would rather wait for it to land, instead of adding another branch. WDYT?

[hokein]

In D67651#1672403, @kadircet wrote:

this one is actually specific to vardecl's and has a pending fix in D66872

I would rather wait for it to land, instead of adding another branch. WDYT?

fair enough, didn't notice that fix before, the testcase here probably doesn't work after that patch.

[kadircet]

In D67651#1672489, @hokein wrote:

fair enough, didn't notice that fix before, the testcase here probably doesn't work after that patch.

yes it shouldn't work after that one.

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

 
 tooling::Replacement createFunctionDefinition(const NewFunction &ExtractedFunc,
                                               const SourceManager &SM) {
   std::string FunctionDef = ExtractedFunc.renderDefinition(SM);
   return tooling::Replacement(SM, ExtractedFunc.InsertionPoint, 0, FunctionDef);
 }
 
 bool ExtractFunction::prepare(const Selection &Inputs) {
+  // Don't trigger the tweak on an empty selection.
+  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
+    return false;
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST.getSourceManager();
   const LangOptions &LangOpts = Inputs.AST.getASTContext().getLangOpts();
   if (auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts)) {
     ExtZone = std::move(*MaybeExtZone);
     return true;
   }
   return false;

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

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");
+  // no selection.
+  EXPECT_EQ(apply("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