Merge pull request #58893 from ahoppen/pr-5.7/shorthand-closure-info

ahoppen · web-flow · commit 95b1176f5b9b · 2022-05-16T21:28:59.000+02:00
[5.7][CursorInfo] Report values shadowed using shorthand syntax as secondary results
diff --git a/include/swift/IDE/Utils.h b/include/swift/IDE/Utils.h
@@ -151,6 +151,14 @@ struct ResolvedCursorInfo {
   ValueDecl *ValueD = nullptr;
   TypeDecl *CtorTyRef = nullptr;
   ExtensionDecl *ExtTyRef = nullptr;
+  /// Declarations that were shadowed by \c ValueD using a shorthand syntax that
+  /// names both the newly declared variable and the referenced variable by the
+  /// same identifier in the source text. This includes shorthand closure
+  /// captures (`[foo]`) and shorthand if captures
+  /// (`if let foo {`).
+  /// Decls that are shadowed using shorthand syntax should be reported as
+  /// additional cursor info results.
+  SmallVector<ValueDecl *, 2> ShorthandShadowedDecls;
   ModuleEntity Mod;
   bool IsRef = true;
   bool IsKeywordArgument = false;
diff --git a/include/swift/Sema/IDETypeChecking.h b/include/swift/Sema/IDETypeChecking.h
@@ -27,6 +27,7 @@
 namespace swift {
   class AbstractFunctionDecl;
   class ASTContext;
+  class CaptureListExpr;
   class ConcreteDeclRef;
   class Decl;
   class DeclContext;
@@ -35,6 +36,7 @@ namespace swift {
   class Expr;
   class ExtensionDecl;
   class FunctionType;
+  class LabeledConditionalStmt;
   class LookupResult;
   class NominalTypeDecl;
   class PatternBindingDecl;
@@ -319,6 +321,19 @@ namespace swift {
 
   /// Just a proxy to swift::contextUsesConcurrencyFeatures() from lib/IDE code.
   bool completionContextUsesConcurrencyFeatures(const DeclContext *dc);
+
+  /// If the capture list shadows any declarations using shorthand syntax, i.e.
+  /// syntax that names both the newly declared variable and the referenced
+  /// variable by the same identifier in the source text, i.e. `[foo]`, return
+  /// these shorthand shadows.
+  /// The first element in the pair is the implicitly declared variable and the
+  /// second variable is the shadowed one.
+  SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
+  getShorthandShadows(CaptureListExpr *CaptureList);
+
+  /// Same as above but for shorthand `if let foo {` syntax.
+  SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
+  getShorthandShadows(LabeledConditionalStmt *CondStmt);
 }
 
 #endif
diff --git a/lib/IDE/IDERequests.cpp b/lib/IDE/IDERequests.cpp
@@ -62,6 +62,12 @@ class CursorInfoResolver : public SourceEntityWalker {
   Type ContainerType;
   Expr *OutermostCursorExpr;
   llvm::SmallVector<Expr*, 8> ExprStack;
+  /// If a decl shadows another decl using shorthand syntax (`[foo]` or
+  /// `if let foo {`), this maps the re-declared variable to the one that is
+  /// being shadowed.
+  /// The transitive closure of shorthand shadowed decls should be reported as
+  /// additional results in cursor info.
+  llvm::DenseMap<ValueDecl *, ValueDecl *> ShorthandShadowedDecls;
 
 public:
   explicit CursorInfoResolver(SourceFile &SrcFile) :
@@ -167,6 +173,11 @@ ResolvedCursorInfo CursorInfoResolver::resolve(SourceLoc Loc) {
   LocToResolve = Loc;
   CursorInfo.Loc = Loc;
   walk(SrcFile);
+  auto ShorthandShadowedDecl = ShorthandShadowedDecls[CursorInfo.ValueD];
+  while (ShorthandShadowedDecl) {
+    CursorInfo.ShorthandShadowedDecls.push_back(ShorthandShadowedDecl);
+    ShorthandShadowedDecl = ShorthandShadowedDecls[ShorthandShadowedDecl];
+  }
   return CursorInfo;
 }
 
@@ -199,6 +210,14 @@ bool CursorInfoResolver::walkToStmtPre(Stmt *S) {
   // there is a token location inside the string, it will seem as if it is out
   // of the source range, unless we convert to character range.
 
+  if (auto CondStmt = dyn_cast<LabeledConditionalStmt>(S)) {
+    for (auto ShorthandShadow : getShorthandShadows(CondStmt)) {
+      assert(ShorthandShadowedDecls.count(ShorthandShadow.first) == 0);
+      ShorthandShadowedDecls[ShorthandShadow.first] =
+          ShorthandShadow.second;
+    }
+  }
+
   // FIXME: Even implicit Stmts should have proper ranges that include any
   // non-implicit Stmts (fix Stmts created for lazy vars).
   if (!S->isImplicit() &&
@@ -250,6 +269,14 @@ bool CursorInfoResolver::walkToExprPre(Expr *E) {
   if (isDone())
     return true;
 
+  if (auto CaptureList = dyn_cast<CaptureListExpr>(E)) {
+    for (auto ShorthandShadows : getShorthandShadows(CaptureList)) {
+      assert(ShorthandShadowedDecls.count(ShorthandShadows.first) == 0);
+      ShorthandShadowedDecls[ShorthandShadows.first] =
+          ShorthandShadows.second;
+    }
+  }
+
   if (auto SAE = dyn_cast<SelfApplyExpr>(E)) {
     if (SAE->getFn()->getStartLoc() == LocToResolve) {
       ContainerType = SAE->getBase()->getType();
diff --git a/lib/IDE/IDETypeChecking.cpp b/lib/IDE/IDETypeChecking.cpp
@@ -906,3 +906,61 @@ Type swift::getResultTypeOfKeypathDynamicMember(SubscriptDecl *SD) {
     RootAndResultTypeOfKeypathDynamicMemberRequest{SD}, TypePair()).
       SecondTy;
 }
+
+SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
+swift::getShorthandShadows(CaptureListExpr *CaptureList) {
+  SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1> Result;
+  for (auto Capture : CaptureList->getCaptureList()) {
+    if (Capture.PBD->getPatternList().size() != 1) {
+      continue;
+    }
+    auto *DRE = dyn_cast_or_null<DeclRefExpr>(Capture.PBD->getInit(0));
+    if (!DRE) {
+      continue;
+    }
+
+    auto DeclaredVar = Capture.getVar();
+    if (DeclaredVar->getLoc() != DRE->getLoc()) {
+      // We have a capture like `[foo]` if the declared var and the
+      // reference share the same location.
+      continue;
+    }
+
+    auto *ReferencedVar = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+    if (!ReferencedVar) {
+      continue;
+    }
+
+    assert(DeclaredVar->getName() == ReferencedVar->getName());
+
+    Result.emplace_back(std::make_pair(DeclaredVar, ReferencedVar));
+  }
+  return Result;
+}
+
+SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1>
+swift::getShorthandShadows(LabeledConditionalStmt *CondStmt) {
+  SmallVector<std::pair<ValueDecl *, ValueDecl *>, 1> Result;
+  for (const StmtConditionElement &Cond : CondStmt->getCond()) {
+    if (Cond.getKind() != StmtConditionElement::CK_PatternBinding) {
+      continue;
+    }
+    auto Init = dyn_cast<DeclRefExpr>(Cond.getInitializer());
+    if (!Init) {
+      continue;
+    }
+    auto ReferencedVar = dyn_cast_or_null<VarDecl>(Init->getDecl());
+    if (!ReferencedVar) {
+      continue;
+    }
+
+    Cond.getPattern()->forEachVariable([&](VarDecl *DeclaredVar) {
+      if (DeclaredVar->getLoc() != Init->getLoc()) {
+        return;
+      }
+      assert(DeclaredVar->getName() == ReferencedVar->getName());
+      Result.emplace_back(std::make_pair(DeclaredVar, ReferencedVar));
+    });
+  }
+  return Result;
+}
diff --git a/test/SourceKit/CursorInfo/closure_capture.swift b/test/SourceKit/CursorInfo/closure_capture.swift
@@ -0,0 +1,78 @@
+func takeClosure(_ closure: () -> Void) {}
+
+func simple(bar: Int) {
+  takeClosure { [bar] in
+    print(bar)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=3:13 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_PARAM
+// SIMPLE_PARAM: source.lang.swift.decl.var.parameter (3:13-3:16)
+// SIMPLE_PARAM: SECONDARY SYMBOLS BEGIN
+// There should be no secondary symbols
+// SIMPLE_PARAM-NEXT: SECONDARY SYMBOLS END
+
+// RUN: %sourcekitd-test -req=cursor -pos=4:18 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_CAPTURE
+// SIMPLE_CAPTURE: source.lang.swift.decl.var.local (4:18-4:21)
+// SIMPLE_CAPTURE: SECONDARY SYMBOLS BEGIN
+// SIMPLE_CAPTURE: source.lang.swift.ref.var.local (3:13-3:16)
+
+// RUN: %sourcekitd-test -req=cursor -pos=5:11 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_REF
+// SIMPLE_REF: source.lang.swift.ref.var.local (4:18-4:21)
+// SIMPLE_REF: SECONDARY SYMBOLS BEGIN
+// SIMPLE_REF: source.lang.swift.ref.var.local (3:13-3:16)
+
+func doubleNested(bar: Int) {
+  takeClosure { [bar] in
+    takeClosure { [bar] in
+      print(bar)
+    }
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=26:18 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_FIRST_CAPTURE
+// DOUBLE_NESTED_FIRST_CAPTURE: source.lang.swift.decl.var.local (26:18-26:21)
+// DOUBLE_NESTED_FIRST_CAPTURE: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_FIRST_CAPTURE: source.lang.swift.ref.var.local (25:19-25:22)
+
+// RUN: %sourcekitd-test -req=cursor -pos=27:20 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_SECOND_CAPTURE
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.decl.var.local (27:20-27:23)
+// DOUBLE_NESTED_SECOND_CAPTURE: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.ref.var.local (26:18-26:21)
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.ref.var.local (25:19-25:22)
+
+// RUN: %sourcekitd-test -req=cursor -pos=28:13 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_REF
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (27:20-27:23)
+// DOUBLE_NESTED_REF: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (26:18-26:21)
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (25:19-25:22)
+
+// Make sure we don't report secondary symbols if the variable is captured explicitly using '='
+func explicitCapture(bar: Int) {
+  takeClosure { [bar = bar] in
+    print(bar)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=53:11 %s -- %s | %FileCheck %s --check-prefix=EXPLICIT_CAPTURE_REF
+// EXPLICIT_CAPTURE_REF: source.lang.swift.ref.var.local (52:18-52:21)
+// EXPLICIT_CAPTURE_REF: SECONDARY SYMBOLS BEGIN
+// There should be no secondary symbols
+// EXPLICIT_CAPTURE_REF-NEXT: SECONDARY SYMBOLS END
+
+func multipleCaptures(bar: Int, baz: Int) {
+  takeClosure { [bar, baz] in
+    print(bar)
+    print(baz)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=65:11 %s -- %s | %FileCheck %s --check-prefix=MULTIPLE_CAPTURES_BAR
+// MULTIPLE_CAPTURES_BAR: source.lang.swift.ref.var.local (64:18-64:21)
+// MULTIPLE_CAPTURES_BAR: SECONDARY SYMBOLS BEGIN
+// MULTIPLE_CAPTURES_BAR.lang.swift.ref.var.local (63:23-63:26)
+
+// RUN: %sourcekitd-test -req=cursor -pos=66:11 %s -- %s | %FileCheck %s --check-prefix=MULTIPLE_CAPTURES_BAZ
+// MULTIPLE_CAPTURES_BAZ: source.lang.swift.ref.var.local (64:23-64:26)
+// MULTIPLE_CAPTURES_BAZ: SECONDARY SYMBOLS BEGIN
+// MULTIPLE_CAPTURES_BAZ.lang.swift.ref.var.local (63:33-63:36)
diff --git a/test/SourceKit/CursorInfo/shorthand_if_let.swift b/test/SourceKit/CursorInfo/shorthand_if_let.swift
@@ -0,0 +1,108 @@
+func simple(bar: Int?) {
+  if let bar {
+    print(bar)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=1:13 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_PARAM
+// SIMPLE_PARAM: source.lang.swift.decl.var.parameter (1:13-1:16)
+// SIMPLE_PARAM: Int
+// SIMPLE_PARAM: SECONDARY SYMBOLS BEGIN
+// There should be no secondary symbols
+// SIMPLE_PARAM-NEXT: SECONDARY SYMBOLS END
+
+// RUN: %sourcekitd-test -req=cursor -pos=2:10 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_CAPTURE
+// SIMPLE_CAPTURE: source.lang.swift.decl.var.local (2:10-2:13)
+// SIMPLE_CAPTURE: Int
+// SIMPLE_CAPTURE: SECONDARY SYMBOLS BEGIN
+// SIMPLE_CAPTURE: source.lang.swift.ref.var.local (1:13-1:16)
+// SIMPLE_CAPTURE: Int?
+
+// RUN: %sourcekitd-test -req=cursor -pos=3:11 %s -- %s | %FileCheck %s --check-prefix=SIMPLE_REF
+// SIMPLE_REF: source.lang.swift.ref.var.local (2:10-2:13)
+// SIMPLE_REF: Int
+// SIMPLE_REF: SECONDARY SYMBOLS BEGIN
+// SIMPLE_REF: source.lang.swift.ref.var.local (1:13-1:16)
+// SIMPLE_REF: Int?
+
+func doubleNested(bar: Int??) {
+  if let bar {
+    if let bar {
+      print(bar)
+    }
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=29:10 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_FIRST_CAPTURE
+// DOUBLE_NESTED_FIRST_CAPTURE: source.lang.swift.decl.var.local (29:10-29:13)
+// DOUBLE_NESTED_FIRST_CAPTURE: Int?
+// DOUBLE_NESTED_FIRST_CAPTURE: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_FIRST_CAPTURE: source.lang.swift.ref.var.local (28:19-28:22)
+// DOUBLE_NESTED_FIRST_CAPTURE: Int??
+
+// RUN: %sourcekitd-test -req=cursor -pos=30:12 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_SECOND_CAPTURE
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.decl.var.local (30:12-30:15)
+// DOUBLE_NESTED_SECOND_CAPTURE: Int
+// DOUBLE_NESTED_SECOND_CAPTURE: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.ref.var.local (29:10-29:13)
+// DOUBLE_NESTED_SECOND_CAPTURE: Int?
+// DOUBLE_NESTED_SECOND_CAPTURE: source.lang.swift.ref.var.local (28:19-28:22)
+// DOUBLE_NESTED_SECOND_CAPTURE: Int??
+
+// RUN: %sourcekitd-test -req=cursor -pos=31:13 %s -- %s | %FileCheck %s --check-prefix=DOUBLE_NESTED_REF
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (30:12-30:15)
+// DOUBLE_NESTED_REF: Int
+// DOUBLE_NESTED_REF: SECONDARY SYMBOLS BEGIN
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (29:10-29:13)
+// DOUBLE_NESTED_REF: Int?
+// DOUBLE_NESTED_REF: source.lang.swift.ref.var.local (28:19-28:22)
+// DOUBLE_NESTED_REF: Int??
+
+// Make sure we don't report secondary symbols if the variable is captured explicitly using '='
+func explicitCapture(bar: Int?) {
+  if let bar = bar {
+    print(bar)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=64:11 %s -- %s | %FileCheck %s --check-prefix=EXPLICIT_CAPTURE_REF
+// EXPLICIT_CAPTURE_REF: source.lang.swift.ref.var.local (63:10-63:13)
+// EXPLICIT_CAPTURE_REF: Int
+// EXPLICIT_CAPTURE_REF: SECONDARY SYMBOLS BEGIN
+// There should be no secondary symbols
+// EXPLICIT_CAPTURE_REF-NEXT: SECONDARY SYMBOLS END
+
+func multipleShorthand(bar: Int?, baz: Int?) {
+  if let bar, let baz {
+    print(bar)
+    print(baz)
+  }
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=77:11 %s -- %s | %FileCheck %s --check-prefix=MULTIPLE_SHORTHAND_BAR
+// MULTIPLE_SHORTHAND_BAR: source.lang.swift.ref.var.local (76:10-76:13)
+// MULTIPLE_SHORTHAND_BAR: Int
+// MULTIPLE_SHORTHAND_BAR: SECONDARY SYMBOLS BEGIN
+// MULTIPLE_SHORTHAND_BAR.lang.swift.ref.var.local (75:23-75:26)
+// MULTIPLE_SHORTHAND_BAR: Int?
+
+// RUN: %sourcekitd-test -req=cursor -pos=78:11 %s -- %s | %FileCheck %s --check-prefix=MULTIPLE_SHORTHAND_BAZ
+// MULTIPLE_SHORTHAND_BAZ: source.lang.swift.ref.var.local (76:19-76:22)
+// MULTIPLE_SHORTHAND_BAZ: Int
+// MULTIPLE_SHORTHAND_BAZ: SECONDARY SYMBOLS BEGIN
+// MULTIPLE_SHORTHAND_BAZ.lang.swift.ref.var.local (63:33-63:36)
+// MULTIPLE_SHORTHAND_BAZ: Int?
+
+func guardLet(bar: Int?) {
+  guard let bar else {
+    return
+  }
+  print(bar)
+}
+
+// RUN: %sourcekitd-test -req=cursor -pos=100:9 %s -- %s | %FileCheck %s --check-prefix=GUARD_LET
+// GUARD_LET: source.lang.swift.ref.var.local (97:13-97:16)
+// GUARD_LET: Int
+// GUARD_LET: SECONDARY SYMBOLS BEGIN
+// GUARD_LET.lang.swift.ref.var.local (96:15-96:18)
+// GUARD_LET: Int?
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
