[indexer] Fix crash in initVarRefIndexSymbols by handling references from 'import var ...'

lib/AST/SourceEntityWalker.cpp

@@ -365,6 +365,7 @@ bool SemaAnnotator::handleImports(ImportDecl *Import) {
   auto Decls = Import->getDecls();
   if (Decls.size() == 1) {
     // FIXME: ImportDecl should store a DeclNameLoc.
+    // FIXME: Handle overloaded funcs too by passing a reference for each?
     if (!passReference(Decls.front(), Type(), DeclNameLoc(Import->getEndLoc()),
         SemaReferenceKind::DeclRef))
       return false;

lib/Index/Index.cpp

@@ -361,7 +361,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
   bool initIndexSymbol(ValueDecl *D, SourceLoc Loc, bool IsRef,
                        IndexSymbol &Info);
   bool initFuncDeclIndexSymbol(ValueDecl *D, IndexSymbol &Info);
-  bool initCallRefIndexSymbol(Expr *CurrentE, Expr *ParentE, ValueDecl *D,
+  bool initFuncRefIndexSymbol(Expr *CurrentE, Expr *ParentE, ValueDecl *D,
                               SourceLoc Loc, IndexSymbol &Info);
   bool initVarRefIndexSymbols(Expr *CurrentE, ValueDecl *D, SourceLoc Loc,
                               IndexSymbol &Info);
@@ -662,15 +662,15 @@ bool IndexSwiftASTWalker::reportPseudoAccessor(AbstractStorageDecl *D,
   if (IsRef) {
     IndexSymbol Info;
 
-    // initCallRefIndexSymbol uses the top of the entities stack as the caller,
+    // initFuncRefIndexSymbol uses the top of the entities stack as the caller,
     // but in this case the top of the stack is the referenced
     // AbstractStorageDecl.
     assert(getParentDecl() == D);
     auto PreviousTop = EntitiesStack.pop_back_val();
-    bool initCallFailed = initCallRefIndexSymbol(ExprStack.back(), getParentExpr(), D, Loc, Info);
+    bool initFailed = initFuncRefIndexSymbol(getCurrentExpr(), getParentExpr(), D, Loc, Info);
     EntitiesStack.push_back(PreviousTop);
 
-    if (initCallFailed)
+    if (initFailed)
       return true; // continue walking.
     if (updateInfo(Info))
       return true;
@@ -793,7 +793,7 @@ bool IndexSwiftASTWalker::reportRef(ValueDecl *D, SourceLoc Loc,
     return true; // keep walking
 
   if (isa<AbstractFunctionDecl>(D)) {
-    if (initCallRefIndexSymbol(getCurrentExpr(), getParentExpr(), D, Loc, Info))
+    if (initFuncRefIndexSymbol(getCurrentExpr(), getParentExpr(), D, Loc, Info))
       return true;
   } else if (isa<AbstractStorageDecl>(D)) {
     if (initVarRefIndexSymbols(getCurrentExpr(), D, Loc, Info))
@@ -809,11 +809,10 @@ bool IndexSwiftASTWalker::reportRef(ValueDecl *D, SourceLoc Loc,
   }
 
   // Report the accessors that were utilized.
-  if (isa<AbstractStorageDecl>(D)) {
+  if (AbstractStorageDecl *ASD = dyn_cast<AbstractStorageDecl>(D)) {
     bool UsesGetter = Info.roles & (SymbolRoleSet)SymbolRole::Read;
     bool UsesSetter = Info.roles & (SymbolRoleSet)SymbolRole::Write;
 
-    AbstractStorageDecl *ASD = cast<AbstractStorageDecl>(D);
     if (UsesGetter)
       if (!reportPseudoAccessor(ASD, AccessorKind::IsGetter, /*IsRef=*/true,
                                 Loc))
@@ -947,15 +946,23 @@ static bool isDynamicCall(Expr *BaseE, ValueDecl *D) {
   return true;
 }
 
-bool IndexSwiftASTWalker::initCallRefIndexSymbol(Expr *CurrentE, Expr *ParentE,
+bool IndexSwiftASTWalker::initFuncRefIndexSymbol(Expr *CurrentE, Expr *ParentE,
                                                  ValueDecl *D, SourceLoc Loc,
                                                  IndexSymbol &Info) {
-  if (!ParentE)
-    return true;
 
   if (initIndexSymbol(D, Loc, /*IsRef=*/true, Info))
     return true;
 
+  if (!CurrentE)
+    return false;
+
+  // FIXME: the below check maintains existing indexing behaviour with
+  // pseudo/accessor output but seems incorrect. E.g otherGlobal in:
+  // let global = otherGlobal
+  // will not have a parent expression so no accessor call is reported
+  if (!ParentE)
+    return true;
+
   Info.roles |= (unsigned)SymbolRole::Call;
 
   Decl *ParentD = getParentDecl();
@@ -995,10 +1002,13 @@ bool IndexSwiftASTWalker::initCallRefIndexSymbol(Expr *CurrentE, Expr *ParentE,
 
 bool IndexSwiftASTWalker::initVarRefIndexSymbols(Expr *CurrentE, ValueDecl *D, SourceLoc Loc, IndexSymbol &Info) {
 
-  if (!(CurrentE->getReferencedDecl() == D))
+  if (initIndexSymbol(D, Loc, /*IsRef=*/true, Info))
     return true;
 
-  if (initIndexSymbol(D, Loc, /*IsRef=*/true, Info))
+  if (!CurrentE)
+    return false;
+
+  if (!(CurrentE->getReferencedDecl() == D))
     return true;
 
   AccessKind Kind = CurrentE->hasLValueAccessKind() ? CurrentE->getLValueAccessKind() : AccessKind::Read;

test/Index/Inputs/imported_swift_module.swift

@@ -0,0 +1,2 @@
+public func importedFunc() {}
+public var importedGlobal: Int = 0

test/Index/roles.swift

@@ -1,4 +1,13 @@
-// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck %s
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+//
+// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/imported_swift_module.swift
+// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s -I %t | %FileCheck %s
+
+import func imported_swift_module.importedFunc
+// CHECK: [[@LINE-1]]:35 | function/Swift | importedFunc() | s:F21imported_swift_module12importedFuncFT_T_ | Ref | rel: 0
+import var imported_swift_module.importedGlobal
+// CHECK: [[@LINE-1]]:34 | variable/Swift | importedGlobal | s:v21imported_swift_module14importedGlobalSi | Ref | rel: 0
 
 // Definition
 let x = 2

test/SourceKit/Indexing/index_func_import.swift.response

@@ -25,6 +25,13 @@
     }
   ],
   key.entities: [
+    {
+      key.kind: source.lang.swift.ref.function.free,
+      key.name: "globalFunc()",
+      key.usr: "s:F11test_module10globalFuncFT_T_",
+      key.line: 8,
+      key.column: 25
+    },
     {
       key.kind: source.lang.swift.decl.function.free,
       key.name: "test()",