[CodeCompletion] Deduplicate the two isMemberCompletion functions in ParseExpr.cpp and ParseDecl.cpp

Also:
- propagate the Solution -> Result rename to Solution parameter of deliverDotExprResults
- fixup header comment in CodeCompletionTypeChecking.h

Author: Nathan Hawes

include/swift/Parse/Parser.h

@@ -696,6 +696,15 @@ class Parser {
       Context.LangOpts.ParseForSyntaxTreeOnly;
   }
 
+  /// If a function or closure body consists of a single expression, determine
+  /// whether we should turn wrap it in a return statement or not.
+  ///
+  /// We don't do this transformation for non-solver-based code completion
+  /// positions, as the source may be incomplete and the type mismatch in the
+  /// return statement will just confuse the type checker.
+  bool shouldSuppressSingleExpressionBodyTransform(
+      ParserStatus Status, MutableArrayRef<ASTNode> BodyElems);
+
 public:
   InFlightDiagnostic diagnose(SourceLoc Loc, Diagnostic Diag) {
     if (Diags.isDiagnosticPointsToFirstBadToken(Diag.getID()) &&

include/swift/Sema/CodeCompletionTypeChecking.h

@@ -1,3 +1,4 @@
+//===--- CodeCompletionTypeChecking.h  --------------------------*- C++ -*-===//
 //
 // Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception

lib/IDE/CodeCompletion.cpp

@@ -5911,7 +5911,7 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext,
 }
 
 void deliverDotExprResults(
-    ArrayRef<DotExprTypeCheckCompletionCallback::Result> Solutions,
+    ArrayRef<DotExprTypeCheckCompletionCallback::Result> Results,
     Expr *BaseExpr, DeclContext *DC, SourceLoc DotLoc, bool IsInSelector,
     ide::CodeCompletionContext &CompletionCtx,
     CodeCompletionConsumer &Consumer) {
@@ -5935,15 +5935,15 @@ void deliverDotExprResults(
     Lookup.setPreferFunctionReferencesToCalls();
   }
 
-  for (auto &Solution: Solutions) {
-    Lookup.setIsStaticMetatype(Solution.BaseIsStaticMetaType);
-    Lookup.getPostfixKeywordCompletions(Solution.BaseTy, BaseExpr);
-    Lookup.setExpectedTypes(Solution.ExpectedTypes,
-                            Solution.IsSingleExpressionBody,
-                            Solution.ExpectsNonVoid);
-    if (isDynamicLookup(Solution.BaseTy))
+  for (auto &Result: Results) {
+    Lookup.setIsStaticMetatype(Result.BaseIsStaticMetaType);
+    Lookup.getPostfixKeywordCompletions(Result.BaseTy, BaseExpr);
+    Lookup.setExpectedTypes(Result.ExpectedTypes,
+                            Result.IsSingleExpressionBody,
+                            Result.ExpectsNonVoid);
+    if (isDynamicLookup(Result.BaseTy))
       Lookup.setIsDynamicLookup();
-    Lookup.getValueExprCompletions(Solution.BaseTy, Solution.BaseDecl);
+    Lookup.getValueExprCompletions(Result.BaseTy, Result.BaseDecl);
   }
 
   SourceFile *SF = DC->getParentSourceFile();

lib/Parse/ParseDecl.cpp

@@ -6443,22 +6443,6 @@ ParserResult<FuncDecl> Parser::parseDeclFunc(SourceLoc StaticLoc,
   return DCC.fixupParserResult(FD);
 }
 
-static bool isMemberCompletion(ASTNode Node) {
-  struct HasMemberCompletion: public ASTWalker {
-    bool Value = false;
-    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
-      if (auto *CCE = dyn_cast<CodeCompletionExpr>(E)) {
-        Value = CCE->getBase();
-        return {false, nullptr};
-      }
-      return {true, E};
-    }
-  };
-  HasMemberCompletion Check;
-  Node.walk(Check);
-  return Check.Value;
-}
-
 /// Parse a function body for \p AFD, setting the body to \p AFD before
 /// returning it.
 BraceStmt *Parser::parseAbstractFunctionBodyImpl(AbstractFunctionDecl *AFD) {
@@ -6506,11 +6490,10 @@ BraceStmt *Parser::parseAbstractFunctionBodyImpl(AbstractFunctionDecl *AFD) {
   // If the body consists of a single expression, turn it into a return
   // statement.
   //
-  // But don't do this transformation during code completion, as the source
-  // may be incomplete and the type mismatch in return statement will just
-  // confuse the type checker.
-  if (BS->getNumElements() != 1 ||
-      (Body.hasCodeCompletion() && !isMemberCompletion(BS->getFirstElement())))
+  // But don't do this transformation when performing certain kinds of code
+  // completion, as the source may be incomplete and the type mismatch in return
+  // statement will just confuse the type checker.
+  if (shouldSuppressSingleExpressionBodyTransform(Body, BS->getElements()))
     return BS;
 
   auto Element = BS->getFirstElement();

lib/Parse/ParseExpr.cpp

@@ -2753,22 +2753,6 @@ parseClosureSignatureIfPresent(SourceRange &bracketRange,
   return invalid;
 }
 
-static bool isMemberCompletion(ASTNode Node) {
-  struct HasMemberCompletion: public ASTWalker {
-    bool Value = false;
-    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
-      if (auto *CCE = dyn_cast<CodeCompletionExpr>(E)) {
-        Value = CCE->getBase();
-        return {false, nullptr};
-      }
-      return {true, E};
-    }
-  };
-  HasMemberCompletion Check;
-  Node.walk(Check);
-  return Check.Value;
-}
-
 ParserResult<Expr> Parser::parseExprClosure() {
   assert(Tok.is(tok::l_brace) && "Not at a left brace?");
   SyntaxParsingContext ClosureContext(SyntaxContext, SyntaxKind::ClosureExpr);
@@ -2875,12 +2859,12 @@ ParserResult<Expr> Parser::parseExprClosure() {
   // If the body consists of a single expression, turn it into a return
   // statement.
   //
-  // But don't do this transformation during code completion, as the source
-  // may be incomplete and the type mismatch in return statement will just
-  // confuse the type checker.
+  // But don't do this transformation when performing certain kinds of code
+  // completion, as the source may be incomplete and the type mismatch in return
+  // statement will just confuse the type checker.
   bool hasSingleExpressionBody = false;
-  if (!missingRBrace && bodyElements.size() == 1 &&
-      (!Status.hasCodeCompletion() || isMemberCompletion(bodyElements[0]))) {
+  if (!missingRBrace &&
+      !shouldSuppressSingleExpressionBodyTransform(Status, bodyElements)) {
     // If the closure's only body element is a single return statement,
     // use that instead of creating a new wrapping return expression.
     Expr *returnExpr = nullptr;

lib/Parse/Parser.cpp

@@ -1185,6 +1185,34 @@ Parser::getStringLiteralIfNotInterpolated(SourceLoc Loc,
                                                Segments.front().Length));
 }
 
+bool Parser::
+shouldSuppressSingleExpressionBodyTransform(ParserStatus Status,
+                                            MutableArrayRef<ASTNode> BodyElems) {
+  if (BodyElems.size() != 1)
+    return true;
+
+  if (!Status.hasCodeCompletion())
+    return false;
+
+  struct HasMemberCompletion: public ASTWalker {
+    bool Value = false;
+    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
+      if (auto *CCE = dyn_cast<CodeCompletionExpr>(E)) {
+        // If it has a base expression this is member completion, which is
+        // performed using the new solver-based mechanism, so it's ok to go
+        // ahead with the transform (and necessary to pick up the correct
+        // expected type).
+        Value = CCE->getBase();
+        return {false, nullptr};
+      }
+      return {true, E};
+    }
+  };
+  HasMemberCompletion Check;
+  BodyElems.front().walk(Check);
+  return !Check.Value;
+}
+
 struct ParserUnit::Implementation {
   std::shared_ptr<SyntaxParseActions> SPActions;
   LangOptions LangOpts;