Merge pull request #64213 from xedin/rdar-104384604

[BuilderTransform] Verify that builder type has at least one accessible build{Partial}Block

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -6629,7 +6629,21 @@ ERROR(cannot_declare_computed_var_in_result_builder,none,
 ERROR(cannot_convert_result_builder_result_to_return_type,none,
       "cannot convert result builder result type %0 to return type %1",
       (Type,Type))
-
+ERROR(result_builder_buildblock_not_accessible,none,
+      "result builder must provide at least one static 'buildBlock' as "
+      "accessible as result builder type %0 "
+      "(which is %select{private|fileprivate|internal|package|public|open}1)",
+      (DeclName, AccessLevel))
+ERROR(result_builder_buildpartialblock_first_not_accessible,none,
+      "result builder must provide at least one static 'buildPartialBlock(first:)' "
+      "as accessible as result builder type %0 "
+      "(which is %select{private|fileprivate|internal|package|public|open}1)",
+      (DeclName, AccessLevel))
+ERROR(result_builder_buildpartialblock_accumulated_not_accessible,none,
+      "result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' "
+      "as accessible as result builder type %0 "
+      "(which is %select{private|fileprivate|internal|package|public|open}1)",
+      (DeclName, AccessLevel))
 
 //------------------------------------------------------------------------------
 // MARK: Tuple Shuffle Diagnostics

lib/Sema/TypeCheckAttr.cpp

@@ -3942,19 +3942,22 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) {
 void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
   auto *nominal = dyn_cast<NominalTypeDecl>(D);
   auto &ctx = D->getASTContext();
-  SmallVector<ValueDecl *, 4> potentialMatches;
+  SmallVector<ValueDecl *, 4> buildBlockMatches;
+  SmallVector<ValueDecl *, 4> buildPartialBlockFirstMatches;
+  SmallVector<ValueDecl *, 4> buildPartialBlockAccumulatedMatches;
+
   bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
       nominal->getDeclaredType(), nominal, ctx.Id_buildBlock,
-      /*argLabels=*/{}, &potentialMatches);
+      /*argLabels=*/{}, &buildBlockMatches);
+
   bool supportsBuildPartialBlock =
       TypeChecker::typeSupportsBuilderOp(
-          nominal->getDeclaredType(), nominal,
-          ctx.Id_buildPartialBlock,
-          /*argLabels=*/{ctx.Id_first}, &potentialMatches) &&
+          nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
+          /*argLabels=*/{ctx.Id_first}, &buildPartialBlockFirstMatches) &&
       TypeChecker::typeSupportsBuilderOp(
-          nominal->getDeclaredType(), nominal,
-          ctx.Id_buildPartialBlock,
-          /*argLabels=*/{ctx.Id_accumulated, ctx.Id_next}, &potentialMatches);
+          nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
+          /*argLabels=*/{ctx.Id_accumulated, ctx.Id_next},
+          &buildPartialBlockAccumulatedMatches);
 
   if (!supportsBuildBlock && !supportsBuildPartialBlock) {
     {
@@ -3968,7 +3971,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
       Type componentType;
       std::tie(buildInsertionLoc, stubIndent, componentType) =
           determineResultBuilderBuildFixItInfo(nominal);
-      if (buildInsertionLoc.isValid() && potentialMatches.empty()) {
+      if (buildInsertionLoc.isValid() && buildBlockMatches.empty()) {
         std::string fixItString;
         {
           llvm::raw_string_ostream out(fixItString);
@@ -3984,7 +3987,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
 
     // For any close matches, attempt to explain to the user why they aren't
     // valid.
-    for (auto *member : potentialMatches) {
+    for (auto *member : buildBlockMatches) {
       if (member->isStatic() && isa<FuncDecl>(member))
         continue;
 
@@ -3998,6 +4001,69 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
         diagnose(member->getLoc(),
                  diag::result_builder_buildblock_not_static_method);
     }
+
+    return;
+  }
+
+  // Let's check whether one or more overloads of buildBlock or
+  // buildPartialBlock are as accessible as the builder type itself.
+  {
+    auto isBuildMethodAsAccessibleAsType = [&](ValueDecl *member) {
+      return !isMemberLessAccessibleThanType(nominal, member);
+    };
+
+    bool hasAccessibleBuildBlock =
+        llvm::any_of(buildBlockMatches, isBuildMethodAsAccessibleAsType);
+
+    bool hasAccessibleBuildPartialBlockFirst = false;
+    bool hasAccessibleBuildPartialBlockAccumulated = false;
+
+    if (supportsBuildPartialBlock) {
+      DeclName buildPartialBlockFirst(ctx, ctx.Id_buildPartialBlock,
+                                      /*argLabels=*/{ctx.Id_first});
+      DeclName buildPartialBlockAccumulated(
+          ctx, ctx.Id_buildPartialBlock,
+          /*argLabels=*/{ctx.Id_accumulated, ctx.Id_next});
+
+      buildPartialBlockFirstMatches.clear();
+      buildPartialBlockAccumulatedMatches.clear();
+
+      auto builderType = nominal->getDeclaredType();
+      nominal->lookupQualified(builderType, DeclNameRef(buildPartialBlockFirst),
+                               NL_QualifiedDefault,
+                               buildPartialBlockFirstMatches);
+      nominal->lookupQualified(
+          builderType, DeclNameRef(buildPartialBlockAccumulated),
+          NL_QualifiedDefault, buildPartialBlockAccumulatedMatches);
+
+      hasAccessibleBuildPartialBlockFirst = llvm::any_of(
+          buildPartialBlockFirstMatches, isBuildMethodAsAccessibleAsType);
+      hasAccessibleBuildPartialBlockAccumulated = llvm::any_of(
+          buildPartialBlockAccumulatedMatches, isBuildMethodAsAccessibleAsType);
+    }
+
+    if (!hasAccessibleBuildBlock) {
+      // No or incomplete `buildPartialBlock` and all overloads of
+      // `buildBlock(_:)` are less accessible than the type.
+      if (!supportsBuildPartialBlock) {
+        diagnose(nominal->getLoc(),
+                 diag::result_builder_buildblock_not_accessible,
+                 nominal->getName(), nominal->getFormalAccess());
+      } else {
+        if (!hasAccessibleBuildPartialBlockFirst) {
+          diagnose(nominal->getLoc(),
+                   diag::result_builder_buildpartialblock_first_not_accessible,
+                   nominal->getName(), nominal->getFormalAccess());
+        }
+
+        if (!hasAccessibleBuildPartialBlockAccumulated) {
+          diagnose(
+              nominal->getLoc(),
+              diag::result_builder_buildpartialblock_accumulated_not_accessible,
+              nominal->getName(), nominal->getFormalAccess());
+        }
+      }
+    }
   }
 }
 

test/IDE/print_swift_module.swift

@@ -29,7 +29,7 @@ public func returnsAlias() -> Alias<Int> {
 
 @resultBuilder
 public struct BridgeBuilder {
-    static func buildBlock(_: Any...) {}
+    public static func buildBlock(_: Any...) {}
 }
 
 public struct City {

test/NameLookup/Inputs/custom_attrs_A.swift

@@ -9,5 +9,5 @@ public struct Wrapper<Value> {
 
 @resultBuilder
 public struct Builder {
-  static func buildBlock<T>(_ component: T) -> T { component }
+  public static func buildBlock<T>(_ component: T) -> T { component }
 }

test/NameLookup/Inputs/custom_attrs_B.swift

@@ -9,5 +9,5 @@ public struct Wrapper<Value> {
 
 @resultBuilder
 public struct Builder {
-  static func buildBlock<T>(_ component: T) -> T { component }
+  public static func buildBlock<T>(_ component: T) -> T { component }
 }

test/Serialization/Inputs/def_macros.swift

@@ -27,7 +27,7 @@ public struct TestMacroArgTypechecking {
 
 @resultBuilder
 public struct Builder {
-  static func buildBlock(_: Int...) -> Void {}
+  public static func buildBlock(_: Int...) -> Void {}
 }
 @freestanding(expression)
 public macro macroWithBuilderArgs(@Builder _: () -> Void) = #externalMacro(module: "A", type: "B")

test/attr/attr_function_builder.swift

@@ -0,0 +1,78 @@
+// RUN: %target-typecheck-verify-swift
+
+@_functionBuilder // expected-warning{{'@_functionBuilder' has been renamed to '@resultBuilder'}}{{2-18=resultBuilder}}
+struct MyBuilder {
+  static func buildBlock(_: Any...) -> Any { }
+}
+
+// rdar://104384604 - empty result builder in swiftinterface file
+@resultBuilder
+public struct TestInvalidBuildBlock1 {
+  // expected-error@-1 {{result builder must provide at least one static 'buildBlock' as accessible as result builder type 'TestInvalidBuildBlock1' (which is public)}}
+  static func buildBlock(_: Int) -> Int { 42 }
+}
+
+@resultBuilder
+public struct TestInvalidBuildBlock2 { // Ok
+  static func buildBlock(_: Int) -> Int { 42 }
+  public static func buildBlock(_: String) -> String { "" }
+}
+
+@resultBuilder
+public struct TestInvalidBuildPartialBlockFirst1 {
+  // expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(first:)' as accessible as result builder type 'TestInvalidBuildPartialBlockFirst1' (which is public)}}
+  static func buildPartialBlock(first: Int) -> Int { first }
+  public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestInvalidBuildPartialBlockFirst2 { // Ok
+  static func buildPartialBlock(first: Int) -> Int { first }
+  public static func buildPartialBlock<T>(first: T) -> T { first }
+  public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestInvalidBuildPartialBlockAccumulated1 {
+  // expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' as accessible as result builder type 'TestInvalidBuildPartialBlockAccumulated1' (which is public)}}
+  public static func buildPartialBlock(first: Int) -> Int { first }
+  private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestInvalidBuildPartialBlockAccumulated2 { // Ok
+  public static func buildPartialBlock<T>(first: T) -> T { first }
+  public static func buildPartialBlock<T>(accumulated: T, next: T) -> T { fatalError() }
+
+  private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestBuildPartialBlock1 { // Ok
+  public static func buildPartialBlock(first: Int) -> Int { first }
+  public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestBuildPartialBlock2 { // Ok
+  private static func buildBlock(_: Int) -> Int { 42 }
+
+  public static func buildPartialBlock(first: Int) -> Int { first }
+  public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestBuildPartialBlock3 { // Ok
+  public static func buildBlock(_: Int) -> Int { 42 }
+
+  fileprivate static func buildPartialBlock(first: Int) -> Int { first }
+  fileprivate static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}
+
+@resultBuilder
+public struct TestBuildPartialBlock4 { // Ok
+  public static func buildBlock(_: Int) -> Int { 42 }
+
+  public static func buildPartialBlock(first: Int) -> Int { first }
+  public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
+}