[clang-tools-extra] c5c3cdb - [clangd] Populate-switch triggers when the whole condition is selected.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-08-08T21:06:08+02:00
New Revision: c5c3cdb9c92895a63993cee70d2dd776ff9519c3

URL: https://github.com/llvm/llvm-project/commit/c5c3cdb9c92895a63993cee70d2dd776ff9519c3
DIFF: https://github.com/llvm/llvm-project/commit/c5c3cdb9c92895a63993cee70d2dd776ff9519c3.diff

LOG: [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.

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
    clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
index bae80cdecf590..40af43fef4aaf 100644
--- 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 41518108a4f8d..13277c99cc314 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -95,6 +95,12 @@ TEST_F(PopulateSwitchTest, Test) {
           R""(enum Enum {A}; switch (^A) {})"",
           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,