[CodeCompletion] Rename isSingleExpressionBody to isImplicitSingleExpressionReturn for clarity

This bool was true only for single-expression closure and function bodies that
do not have an explicit `return` in the source. Rename it and its associated
methods, to avoid confusion with the hasSingleExpressionBody methods on
ClosureExpr and AbstractFuncDecl. Those don't care whether the expression was
explicitly written in the source or not.

Author: Nathan Hawes

include/swift/Sema/CodeCompletionTypeChecking.h

@@ -54,7 +54,7 @@ namespace swift {
       SmallVector<Type, 4> ExpectedTypes;
       bool ExpectsNonVoid;
       bool BaseIsStaticMetaType;
-      bool IsSingleExpressionBody;
+      bool IsImplicitSingleExpressionReturn;
     };
 
   private:
@@ -91,7 +91,7 @@ namespace swift {
   public:
     struct Result {
       Type ExpectedTy;
-      bool IsSingleExpressionBody;
+      bool IsImplicitSingleExpressionReturn;
     };
 
   private:

lib/IDE/CodeCompletion.cpp

@@ -1171,14 +1171,14 @@ calculateMaxTypeRelation(Type Ty, const ExpectedTypeContext &typeContext,
     //
     //     { ... -> Int in x }        // x must be Int
     //     { ... -> ()  in return x } // x must be Void
-    if (typeContext.isSingleExpressionBody && expectedTy->isVoid())
+    if (typeContext.isImplicitSingleExpressionReturn && expectedTy->isVoid())
       continue;
 
     Result = std::max(Result, calculateTypeRelation(Ty, expectedTy, DC));
 
     // Map invalid -> unrelated when in a single-expression body, since the
     // input may be incomplete.
-    if (typeContext.isSingleExpressionBody &&
+    if (typeContext.isImplicitSingleExpressionReturn &&
         Result == CodeCompletionResult::ExpectedTypeRelation::Invalid)
       Result = CodeCompletionResult::ExpectedTypeRelation::Unrelated;
   }
@@ -1958,9 +1958,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     IsStaticMetatype = value;
   }
 
-  void setExpectedTypes(ArrayRef<Type> Types, bool isSingleExpressionBody,
+  void setExpectedTypes(ArrayRef<Type> Types,
+                        bool isImplicitSingleExpressionReturn,
                         bool preferNonVoid = false) {
-    expectedTypeContext.isSingleExpressionBody = isSingleExpressionBody;
+    expectedTypeContext.isImplicitSingleExpressionReturn =
+        isImplicitSingleExpressionReturn;
     expectedTypeContext.preferNonVoid = preferNonVoid;
     expectedTypeContext.possibleTypes.clear();
     expectedTypeContext.possibleTypes.reserve(Types.size());
@@ -1976,7 +1978,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
   CodeCompletionContext::TypeContextKind typeContextKind() const {
     if (expectedTypeContext.empty() && !expectedTypeContext.preferNonVoid) {
       return CodeCompletionContext::TypeContextKind::None;
-    } else if (expectedTypeContext.isSingleExpressionBody) {
+    } else if (expectedTypeContext.isImplicitSingleExpressionReturn) {
       return CodeCompletionContext::TypeContextKind::SingleExpressionBody;
     } else {
       return CodeCompletionContext::TypeContextKind::Required;
@@ -6105,7 +6107,7 @@ void deliverUnresolvedMemberResults(
 
   for (auto &Result: Results) {
     Lookup.setExpectedTypes({Result.ExpectedTy},
-                            Result.IsSingleExpressionBody,
+                            Result.IsImplicitSingleExpressionReturn,
                             /*expectsNonVoid*/true);
     Lookup.setIdealExpectedType(Result.ExpectedTy);
 
@@ -6154,7 +6156,7 @@ void deliverDotExprResults(
     Lookup.setIsStaticMetatype(Result.BaseIsStaticMetaType);
     Lookup.getPostfixKeywordCompletions(Result.BaseTy, BaseExpr);
     Lookup.setExpectedTypes(Result.ExpectedTypes,
-                            Result.IsSingleExpressionBody,
+                            Result.IsImplicitSingleExpressionReturn,
                             Result.ExpectsNonVoid);
     if (isDynamicLookup(Result.BaseTy))
       Lookup.setIsDynamicLookup();
@@ -6395,7 +6397,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
   case CompletionKind::PostfixExprBeginning: {
     ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr);
     Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                            ContextInfo.isSingleExpressionBody());
+                            ContextInfo.isImplicitSingleExpressionReturn());
     DoPostfixExprBeginning();
     break;
   }
@@ -6417,8 +6419,9 @@ void CodeCompletionCallbacksImpl::doneParsing() {
 
     if (ShouldCompleteCallPatternAfterParen) {
       ExprContextInfo ParentContextInfo(CurDeclContext, ParsedExpr);
-      Lookup.setExpectedTypes(ParentContextInfo.getPossibleTypes(),
-                              ParentContextInfo.isSingleExpressionBody());
+      Lookup.setExpectedTypes(
+          ParentContextInfo.getPossibleTypes(),
+          ParentContextInfo.isImplicitSingleExpressionReturn());
       if (!ContextInfo.getPossibleCallees().empty()) {
         for (auto &typeAndDecl : ContextInfo.getPossibleCallees())
           Lookup.tryFunctionCallCompletions(typeAndDecl.Type, typeAndDecl.Decl,
@@ -6436,7 +6439,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
         (Lookup.FoundFunctionCalls &&
          Lookup.FoundFunctionsWithoutFirstKeyword)) {
       Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                              ContextInfo.isSingleExpressionBody());
+                              ContextInfo.isImplicitSingleExpressionReturn());
       Lookup.setHaveLParen(false);
       DoPostfixExprBeginning();
     }
@@ -6485,7 +6488,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
   case CompletionKind::CaseStmtBeginning: {
     ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr);
     Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                            ContextInfo.isSingleExpressionBody());
+                            ContextInfo.isImplicitSingleExpressionReturn());
     Lookup.setIdealExpectedType(CodeCompleteTokenExpr->getType());
     Lookup.getUnresolvedMemberCompletions(ContextInfo.getPossibleTypes());
     DoPostfixExprBeginning();
@@ -6504,7 +6507,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
     if (isa<AccessorDecl>(ParsedDecl)) {
       ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr);
       Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                              ContextInfo.isSingleExpressionBody());
+                              ContextInfo.isImplicitSingleExpressionReturn());
       DoPostfixExprBeginning();
     }
     break;
@@ -6578,7 +6581,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
 
     if (shouldPerformGlobalCompletion) {
       Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                              ContextInfo.isSingleExpressionBody());
+                              ContextInfo.isImplicitSingleExpressionReturn());
       DoPostfixExprBeginning();
     }
     break;
@@ -6697,7 +6700,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
     SmallVector<Type, 2> possibleReturnTypes;
     collectPossibleReturnTypesFromContext(CurDeclContext, possibleReturnTypes);
     Lookup.setExpectedTypes(possibleReturnTypes,
-                            /*isSingleExpressionBody*/ false);
+                            /*isImplicitSingleExpressionReturn*/ false);
     Lookup.getValueCompletionsInDeclContext(Loc);
     break;
   }
@@ -6708,7 +6711,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
       if (FD->isCoroutine()) {
         // TODO: handle multi-value yields.
         Lookup.setExpectedTypes(FD->getStorage()->getValueInterfaceType(),
-                                /*isSingleExpressionBody*/ false);
+                                /*isImplicitSingleExpressionReturn*/ false);
       }
     }
     Lookup.getValueCompletionsInDeclContext(Loc);
@@ -6718,7 +6721,7 @@ void CodeCompletionCallbacksImpl::doneParsing() {
   case CompletionKind::AfterPoundExpr: {
     ExprContextInfo ContextInfo(CurDeclContext, CodeCompleteTokenExpr);
     Lookup.setExpectedTypes(ContextInfo.getPossibleTypes(),
-                            ContextInfo.isSingleExpressionBody());
+                            ContextInfo.isImplicitSingleExpressionReturn());
 
     Lookup.addPoundAvailable(ParentStmtKind);
     Lookup.addPoundLiteralCompletions(/*needPound=*/false);

lib/IDE/CodeCompletionResultBuilder.h

@@ -48,12 +48,12 @@ struct ExpectedTypeContext {
   ///
   /// Since the input may be incomplete, we take into account that the types are
   /// only a hint.
-  bool isSingleExpressionBody = false;
+  bool isImplicitSingleExpressionReturn = false;
   bool preferNonVoid = false;
 
   bool empty() const { return possibleTypes.empty(); }
   bool requiresNonVoid() const {
-    if (isSingleExpressionBody)
+    if (isImplicitSingleExpressionReturn)
       return false;
     if (preferNonVoid)
       return true;
@@ -65,9 +65,9 @@ struct ExpectedTypeContext {
   }
 
   ExpectedTypeContext() = default;
-  ExpectedTypeContext(ArrayRef<Type> types, bool isSingleExpressionBody)
+  ExpectedTypeContext(ArrayRef<Type> types, bool isImplicitSingleExprReturn)
       : possibleTypes(types.begin(), types.end()),
-        isSingleExpressionBody(isSingleExpressionBody) {}
+        isImplicitSingleExpressionReturn(isImplicitSingleExprReturn) {}
 };
 
 class CodeCompletionResultBuilder {

lib/IDE/ExprContextAnalysis.cpp

@@ -750,7 +750,7 @@ class ExprContextAnalyzer {
   SmallVectorImpl<PossibleParamInfo> &PossibleParams;
   SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees;
   Expr *&AnalyzedExpr;
-  bool &singleExpressionBody;
+  bool &implicitSingleExpressionReturn;
 
   void recordPossibleType(Type ty) {
     if (!ty || ty->is<ErrorType>())
@@ -970,8 +970,8 @@ class ExprContextAnalyzer {
     }
     case ExprKind::Closure: {
       auto *CE = cast<ClosureExpr>(Parent);
-      assert(isSingleExpressionBodyForCodeCompletion(CE->getBody()));
-      singleExpressionBody = true;
+      assert(hasImplicitSingleExpressionReturn(CE->getBody()));
+      implicitSingleExpressionReturn = true;
       SmallVector<Type, 2> candidates;
       collectPossibleReturnTypesFromContext(CE, candidates);
       for (auto ty : candidates)
@@ -1052,8 +1052,8 @@ class ExprContextAnalyzer {
     }
     default:
       if (auto *FD = dyn_cast<FuncDecl>(D)) {
-        assert(isSingleExpressionBodyForCodeCompletion(FD->getBody()));
-        singleExpressionBody = true;
+        assert(hasImplicitSingleExpressionReturn(FD->getBody()));
+        implicitSingleExpressionReturn = true;
         SmallVector<Type, 2> candidates;
         collectPossibleReturnTypesFromContext(DC, candidates);
         for (auto ty : candidates)
@@ -1105,14 +1105,18 @@ class ExprContextAnalyzer {
   }
 
   /// Whether the given \c BraceStmt, which must be the body of a function or
-  /// closure, should be treated as a single-expression return for the purposes
-  /// of code-completion.
+  /// closure, contains a single expression that would be implicitly returned if
+  /// the single-expression-body transform had been performed.
   ///
   /// We cannot use hasSingleExpressionBody, because we explicitly do not use
-  /// the single-expression-body when there is code-completion in the expression
-  /// in order to avoid a base expression affecting the type. However, now that
-  /// we've typechecked, we will take the context type into account.
-  static bool isSingleExpressionBodyForCodeCompletion(BraceStmt *body) {
+  /// the single-expression-body transform when there is a code-completion in
+  /// the expression in order to avoid a base expression affecting the type, and
+  /// need to distinguish whether the single expression body was explicitly
+  /// returned (in which case the expression's type *must* match the expected
+  /// return type) or not (in which case it *may* match, as the user could intend
+  /// it only as the first statement of many that they haven't finished writing
+  /// yet.
+  static bool hasImplicitSingleExpressionReturn(BraceStmt *body) {
     if (body->getNumElements() == 2) {
       if (auto *D = body->getFirstElement().dyn_cast<Decl *>()) {
         // Step into nested active clause.
@@ -1138,12 +1142,12 @@ class ExprContextAnalyzer {
       DeclContext *DC, Expr *ParsedExpr, SmallVectorImpl<Type> &PossibleTypes,
       SmallVectorImpl<PossibleParamInfo> &PossibleArgs,
       SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees,
-      Expr *&AnalyzedExpr, bool &singleExpressionBody)
+      Expr *&AnalyzedExpr, bool &implicitSingleExpressionReturn)
       : DC(DC), ParsedExpr(ParsedExpr), SM(DC->getASTContext().SourceMgr),
         Context(DC->getASTContext()), PossibleTypes(PossibleTypes),
         PossibleParams(PossibleArgs), PossibleCallees(PossibleCallees),
         AnalyzedExpr(AnalyzedExpr),
-        singleExpressionBody(singleExpressionBody) {}
+        implicitSingleExpressionReturn(implicitSingleExpressionReturn) {}
 
   void Analyze() {
     // We cannot analyze without target.
@@ -1184,7 +1188,7 @@ class ExprContextAnalyzer {
                   !isa<BinaryExpr>(ParentE));
         }
         case ExprKind::Closure:
-          return isSingleExpressionBodyForCodeCompletion(
+          return hasImplicitSingleExpressionReturn(
               cast<ClosureExpr>(E)->getBody());
         default:
           return false;
@@ -1208,7 +1212,7 @@ class ExprContextAnalyzer {
         default:
           if (auto *FD = dyn_cast<FuncDecl>(D))
             if (auto *body = FD->getBody())
-              return isSingleExpressionBodyForCodeCompletion(body);
+              return hasImplicitSingleExpressionReturn(body);
           return false;
         }
       } else if (auto P = Node.getAsPattern()) {
@@ -1252,6 +1256,6 @@ class ExprContextAnalyzer {
 ExprContextInfo::ExprContextInfo(DeclContext *DC, Expr *TargetExpr) {
   ExprContextAnalyzer Analyzer(DC, TargetExpr, PossibleTypes, PossibleParams,
                                PossibleCallees, AnalyzedExpr,
-                               singleExpressionBody);
+                               implicitSingleExpressionReturn);
   Analyzer.Analyze();
 }

lib/IDE/ExprContextAnalysis.h

@@ -82,20 +82,22 @@ class ExprContextInfo {
   SmallVector<PossibleParamInfo, 2> PossibleParams;
   SmallVector<FunctionTypeAndDecl, 2> PossibleCallees;
   Expr *AnalyzedExpr = nullptr;
-  bool singleExpressionBody = false;
+  bool implicitSingleExpressionReturn = false;
 
 public:
   ExprContextInfo(DeclContext *DC, Expr *TargetExpr);
 
   // Returns a list of possible context types.
   ArrayRef<Type> getPossibleTypes() const { return PossibleTypes; }
 
-  /// Whether the type context comes from a single-expression body, e.g.
-  /// `foo({ here })`.
+  /// Whether the type context comes from a single-expression body without
+  /// and explicit return e.g. `foo({ <here> })` and not foo({ return <here>}).
   ///
-  /// If the input may be incomplete, such as in code-completion, take into
+  /// Since the input may be incomplete, such as in code-completion, take into
   /// account that the types returned by `getPossibleTypes()` are only a hint.
-  bool isSingleExpressionBody() const { return singleExpressionBody; }
+  bool isImplicitSingleExpressionReturn() const {
+    return implicitSingleExpressionReturn;
+  }
 
   // Returns a list of possible argument label names.
   // Valid only if \c getKind() is \c CallArgument.

lib/Sema/TypeCheckCodeCompletion.cpp

@@ -1160,12 +1160,21 @@ static Type getTypeForCompletion(const constraints::Solution &S, Expr *E) {
   return S.getResolvedType(E);
 };
 
-static bool isSingleExpressionBodyCompletion(ConstraintSystem &CS,
+/// Whether the given completion expression is the only expression in its
+/// containing closure or function body and its value is implicitly returned.
+///
+/// If these conditions are met, code completion needs to avoid penalizing
+/// completion results that don't match the expected return type when computing
+/// type relations, as since no return statement was explicitly written by the
+/// user, it's possible they intend the single expression not as the return
+/// value but merely the first entry in a multi-statement body they just haven't
+/// finished writing yet.
+static bool isImplicitSingleExpressionReturn(ConstraintSystem &CS,
                                              Expr *CompletionExpr) {
   Expr *ParentExpr = CS.getParentExpr(CompletionExpr);
-  if (!ParentExpr) {
+  if (!ParentExpr)
     return CS.getContextualTypePurpose(CompletionExpr) == CTP_ReturnSingleExpr;
-  }
+
   if (auto *ParentCE = dyn_cast<ClosureExpr>(ParentExpr)) {
     if (ParentCE->hasSingleExpressionBody() &&
         ParentCE->getSingleExpressionBody() == CompletionExpr) {
@@ -1205,14 +1214,14 @@ sawSolution(const constraints::Solution &S) {
   auto Ret = BaseToSolutionIdx.insert({Key, Results.size()});
   if (Ret.second) {
     bool ISDMT = S.isStaticallyDerivedMetatype(ParsedExpr);
-    bool SingleExprBody = isSingleExpressionBodyCompletion(CS, CompletionExpr);
+    bool ImplicitReturn = isImplicitSingleExpressionReturn(CS, CompletionExpr);
     bool DisallowVoid = ExpectedTy
                             ? !ExpectedTy->isVoid()
                             : !ParentExpr && CS.getContextualTypePurpose(
                                                  CompletionExpr) != CTP_Unused;
 
     Results.push_back(
-        {BaseTy, ReferencedDecl, {}, DisallowVoid, ISDMT, SingleExprBody});
+        {BaseTy, ReferencedDecl, {}, DisallowVoid, ISDMT, ImplicitReturn});
     if (ExpectedTy)
       Results.back().ExpectedTypes.push_back(ExpectedTy);
   } else if (ExpectedTy) {
@@ -1242,6 +1251,6 @@ sawSolution(const constraints::Solution &S) {
     return;
   }
 
-  bool SingleExprBody = isSingleExpressionBodyCompletion(CS, CompletionExpr);
+  bool SingleExprBody = isImplicitSingleExpressionReturn(CS, CompletionExpr);
   Results.push_back({ExpectedTy, SingleExprBody});
 }