[clangd] Populate-switch triggers when the whole condition is selected.

This allows vscode to find it as a diagnostic quickfix for -Wswitch.

While here, group the code into chunks and add a couple more comments.

diff --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
index bae80cd..40af43f 100644 (file)
--- a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -88,47 +88,39 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   if (!CA)
     return false;
 
-  const Stmt *CAStmt = CA->ASTNode.get<Stmt>();
-  if (!CAStmt)
-    return false;
-
-  // Go up a level if we see a compound statement.
-  // switch (value) {}
-  //                ^^
-  if (isa<CompoundStmt>(CAStmt)) {
-    CA = CA->Parent;
-    if (!CA)
-      return false;
-
-    CAStmt = CA->ASTNode.get<Stmt>();
-    if (!CAStmt)
+  // Support targeting
+  //  - the switch statement itself (keyword, parens)
+  //  - the whole expression (possibly wrapped in implicit casts)
+  //  - the outer body (typically CompoundStmt)
+  // Selections *within* the expression or body don't trigger.
+  // direct child (the 
+  Switch = CA->ASTNode.get<SwitchStmt>();
+  if (!Switch) {
+    if (const SelectionTree::Node *Parent = CA->outerImplicit().Parent)
+      Switch = Parent->ASTNode.get<SwitchStmt>();
+    if (!Switch)
       return false;
   }
-
-  DeclCtx = &CA->getDeclContext();
-  Switch = dyn_cast<SwitchStmt>(CAStmt);
-  if (!Switch)
-    return false;
-
-  Body = dyn_cast<CompoundStmt>(Switch->getBody());
+  // Body need not be a CompoundStmt! But that's all we support editing.
+  Body = llvm::dyn_cast_or_null<CompoundStmt>(Switch->getBody());
   if (!Body)
     return false;
+  DeclCtx = &CA->getDeclContext();
 
+  // Examine the condition of the switch statement to see if it's an enum.
   const Expr *Cond = Switch->getCond();
   if (!Cond)
     return false;
-
   // Ignore implicit casts, since enums implicitly cast to integer types.
   Cond = Cond->IgnoreParenImpCasts();
-
   EnumT = Cond->getType()->getAsAdjusted<EnumType>();
   if (!EnumT)
     return false;
-
   EnumD = EnumT->getDecl();
   if (!EnumD || EnumD->isDependentType())
     return false;
 
+  // Finally, check which cases exist and which are covered.
   // We trigger if there are any values in the enum that aren't covered by the
   // switch.
 

diff --git a/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
index 4151810..13277c9 100644 (file)
--- a/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -96,6 +96,12 @@ TEST_F(PopulateSwitchTest, Test) {
           R""(enum Enum {A}; switch (A) {case A:break;})"",
       },
       {
+          // Selection of whole switch condition
+          Function,
+          R""(enum Enum {A}; switch ([[A]]) {})"",
+          R""(enum Enum {A}; switch (A) {case A:break;})"",
+      },
+      {
           // Selection in switch body
           Function,
           R""(enum Enum {A}; switch (A) {^})"",