[Sema] Maintain the implicitness of call argument tuple/parens in coerceCallArguments

If any arguments were defaulted the tuple/paren was made implicit, even though
the original tuple/paren was present in the source.

This prevented some sourcekit ASTWalkers from considering them, making
refactorings, documentation info, jump-to-definition and other features
unavailable when queried via their argument labels.

Resolves rdar://problem/62118957

Author: Nathan Hawes

include/swift/AST/Expr.h

@@ -4133,9 +4133,9 @@ class DefaultArgumentExpr final : public Expr {
       DefaultArgsOwner(defaultArgsOwner), ParamIndex(paramIndex), Loc(loc),
       ContextOrCallerSideExpr(dc) { }
 
-  SourceRange getSourceRange() const {
-    return Loc;
-  }
+  SourceRange getSourceRange() const { return {}; }
+
+  SourceLoc getArgumentListLoc() const { return Loc; }
 
   ConcreteDeclRef getDefaultArgsOwner() const {
     return DefaultArgsOwner;

lib/IDE/SyntaxModel.cpp

@@ -547,22 +547,24 @@ std::pair<bool, Expr *> ModelASTWalker::walkToExprPre(Expr *E) {
     return {false, E};
 
   auto addCallArgExpr = [&](Expr *Elem, TupleExpr *ParentTupleExpr) {
-    if (isCurrentCallArgExpr(ParentTupleExpr)) {
-      CharSourceRange NR = parameterNameRangeOfCallArg(ParentTupleExpr, Elem);
-      SyntaxStructureNode SN;
-      SN.Kind = SyntaxStructureKind::Argument;
-      SN.NameRange = NR;
-      SN.BodyRange = charSourceRangeFromSourceRange(SM, Elem->getSourceRange());
-      if (NR.isValid()) {
-        SN.Range = charSourceRangeFromSourceRange(SM, SourceRange(NR.getStart(),
-                                                            Elem->getEndLoc()));
-        passTokenNodesUntil(NR.getStart(), ExcludeNodeAtLocation);
-      }
-      else
-        SN.Range = SN.BodyRange;
+    if (isa<DefaultArgumentExpr>(Elem) ||
+        !isCurrentCallArgExpr(ParentTupleExpr))
+      return;
 
-      pushStructureNode(SN, Elem);
+    CharSourceRange NR = parameterNameRangeOfCallArg(ParentTupleExpr, Elem);
+    SyntaxStructureNode SN;
+    SN.Kind = SyntaxStructureKind::Argument;
+    SN.NameRange = NR;
+    SN.BodyRange = charSourceRangeFromSourceRange(SM, Elem->getSourceRange());
+    if (NR.isValid()) {
+      SN.Range = charSourceRangeFromSourceRange(SM, SourceRange(NR.getStart(),
+                                                          Elem->getEndLoc()));
+      passTokenNodesUntil(NR.getStart(), ExcludeNodeAtLocation);
     }
+    else
+      SN.Range = SN.BodyRange;
+
+    pushStructureNode(SN, Elem);
   };
 
   if (auto *ParentTupleExpr = dyn_cast_or_null<TupleExpr>(Parent.getAsExpr())) {

lib/Sema/CSApply.cpp

@@ -5759,9 +5759,10 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType,
       // We already had a ParenExpr, so replace it's sub-expression.
       argParen->setSubExpr(newArgs[0]);
     } else {
+      bool isImplicit = arg->isImplicit();
       arg = new (ctx)
           ParenExpr(lParenLoc, newArgs[0], rParenLoc, hasTrailingClosure);
-      arg->setImplicit();
+      arg->setImplicit(isImplicit);
     }
   } else {
     assert(isa<TupleType>(paramType.getPointer()));
@@ -5776,7 +5777,7 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType,
       // Build a new TupleExpr, re-using source location information.
       arg = TupleExpr::create(ctx, lParenLoc, newArgs, newLabels, newLabelLocs,
                               rParenLoc, hasTrailingClosure,
-                              /*implicit=*/true);
+                              /*implicit=*/arg->isImplicit());
     }
   }
 

lib/Sema/TypeCheckExpr.cpp

@@ -788,7 +788,8 @@ Expr *CallerSideDefaultArgExprRequest::evaluate(
   auto paramTy = defaultExpr->getType();
 
   // Re-create the default argument using the location info of the call site.
-  auto *initExpr = synthesizeCallerSideDefault(param, defaultExpr->getLoc());
+  auto *initExpr =
+      synthesizeCallerSideDefault(param, defaultExpr->getArgumentListLoc());
   auto *dc = defaultExpr->ContextOrCallerSideExpr.get<DeclContext *>();
   assert(dc && "Expected a DeclContext before type-checking caller-side arg");
 

test/SourceKit/Refactoring/basic.swift

@@ -106,6 +106,13 @@ struct TestDefaultedParen {
 
 TestDefaultedParen.init()
 
+struct HasInitWithDefaultArgs {
+  init(x: Int = 10, y: Int = 20, z: Int = 10)
+}
+
+HasInitWithDefaultArgs(z: 45)
+HasInitWithDefaultArgs(y: 45, z: 89)
+
 // RUN: %sourcekitd-test -req=cursor -pos=3:1 -end-pos=5:13 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK1
 
 // CHECK1: ACTIONS BEGIN
@@ -139,6 +146,11 @@ TestDefaultedParen.init()
 
 // RUN: %sourcekitd-test -req=cursor -pos=107:20  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-NORENAME
 
+// RUN: %sourcekitd-test -req=cursor -pos=113:24  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
+// RUN: %sourcekitd-test -req=cursor -pos=114:24  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
+// RUN: %sourcekitd-test -req=cursor -pos=114:31  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
+// RUN: %sourcekitd-test -req=cursor -pos=114:31  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
+
 // RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
 // RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT