Merge pull request swiftlang#33798 from Jumhyn/function-builder-static-buildblock

[Sema] Require function builders to have at least one buildBlock method

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -5265,6 +5265,16 @@ WARNING(function_builder_missing_limited_availability, none,
         "function builder %0 does not implement 'buildLimitedAvailability'; "
         "this code may crash on earlier versions of the OS",
         (Type))
+ERROR(function_builder_static_buildblock, none,
+      "function builder must provide at least one static 'buildBlock' "
+      "method", ())
+NOTE(function_builder_non_static_buildblock, none,
+     "did you mean to make instance method 'buildBlock' static?", ())
+NOTE(function_builder_buildblock_enum_case, none,
+     "enum case 'buildBlock' cannot be used to satisfy the function builder "
+     "requirement", ())
+NOTE(function_builder_buildblock_not_static_method, none,
+     "potential match 'buildBlock' is not a static method", ())
 
 //------------------------------------------------------------------------------
 // MARK: Tuple Shuffle Diagnostics

lib/Sema/BuilderTransform.cpp

@@ -145,32 +145,8 @@ class BuilderClosureVisitor
       return known->second;
     }
 
-    bool found = false;
-    SmallVector<ValueDecl *, 4> foundDecls;
-    dc->lookupQualified(
-        builderType, DeclNameRef(fnName),
-        NL_QualifiedDefault | NL_ProtocolMembers, foundDecls);
-    for (auto decl : foundDecls) {
-      if (auto func = dyn_cast<FuncDecl>(decl)) {
-        // Function must be static.
-        if (!func->isStatic())
-          continue;
-
-        // Function must have the right argument labels, if provided.
-        if (!argLabels.empty()) {
-          auto funcLabels = func->getName().getArgumentNames();
-          if (argLabels.size() > funcLabels.size() ||
-              funcLabels.slice(0, argLabels.size()) != argLabels)
-            continue;
-        }
-
-        // Okay, it's a good-enough match.
-        found = true;
-        break;
-      }
-    }
-
-    return supportedOps[fnName] = found;
+    return supportedOps[fnName] = TypeChecker::typeSupportsBuilderOp(
+               builderType, dc, fnName, argLabels);
   }
 
   /// Build an implicit variable in this context.
@@ -1874,3 +1850,37 @@ std::vector<ReturnStmt *> TypeChecker::findReturnStatements(AnyFunctionRef fn) {
   (void)precheck.run();
   return precheck.getReturnStmts();
 }
+
+bool TypeChecker::typeSupportsBuilderOp(
+    Type builderType, DeclContext *dc, Identifier fnName,
+    ArrayRef<Identifier> argLabels, SmallVectorImpl<ValueDecl *> *allResults) {
+  bool foundMatch = false;
+  SmallVector<ValueDecl *, 4> foundDecls;
+  dc->lookupQualified(
+      builderType, DeclNameRef(fnName),
+      NL_QualifiedDefault | NL_ProtocolMembers, foundDecls);
+  for (auto decl : foundDecls) {
+    if (auto func = dyn_cast<FuncDecl>(decl)) {
+      // Function must be static.
+      if (!func->isStatic())
+        continue;
+
+      // Function must have the right argument labels, if provided.
+      if (!argLabels.empty()) {
+        auto funcLabels = func->getName().getArgumentNames();
+        if (argLabels.size() > funcLabels.size() ||
+            funcLabels.slice(0, argLabels.size()) != argLabels)
+          continue;
+      }
+
+      foundMatch = true;
+      break;
+    }
+  }
+
+  if (allResults)
+    allResults->append(foundDecls.begin(), foundDecls.end());
+
+  return foundMatch;
+}
+

lib/Sema/TypeCheckAttr.cpp

@@ -3008,8 +3008,32 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) {
 }
 
 void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) {
-  // TODO: check that the type at least provides a `sequence` factory?
-  // Any other validation?
+  auto *nominal = dyn_cast<NominalTypeDecl>(D);
+  SmallVector<ValueDecl *, 4> potentialMatches;
+  bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
+      nominal->getDeclaredType(), nominal, D->getASTContext().Id_buildBlock,
+      /*argLabels=*/{}, &potentialMatches);
+
+  if (!supportsBuildBlock) {
+    diagnose(nominal->getLoc(), diag::function_builder_static_buildblock);
+
+    // For any close matches, attempt to explain to the user why they aren't
+    // valid.
+    for (auto *member : potentialMatches) {
+      if (member->isStatic() && isa<FuncDecl>(member))
+        continue;
+
+      if (isa<FuncDecl>(member) &&
+          member->getDeclContext()->getSelfNominalTypeDecl() == nominal)
+        diagnose(member->getLoc(), diag::function_builder_non_static_buildblock)
+          .fixItInsert(member->getAttributeInsertionLoc(true), "static ");
+      else if (isa<EnumElementDecl>(member))
+        diagnose(member->getLoc(), diag::function_builder_buildblock_enum_case);
+      else
+        diagnose(member->getLoc(),
+                 diag::function_builder_buildblock_not_static_method);
+    }
+  }
 }
 
 void

lib/Sema/TypeChecker.h

@@ -1221,6 +1221,13 @@ bool requireArrayLiteralIntrinsics(ASTContext &ctx, SourceLoc loc);
 /// If \c expr is not part of a member chain or the base is something other than
 /// an \c UnresolvedMemberExpr, \c nullptr is returned.
 UnresolvedMemberExpr *getUnresolvedMemberChainBase(Expr *expr);
+
+/// Checks whether a function builder type has a well-formed function builder
+/// method with the given name. If provided and non-empty, the argument labels
+/// are verified against any candidates.
+bool typeSupportsBuilderOp(Type builderType, DeclContext *dc, Identifier fnName,
+                           ArrayRef<Identifier> argLabels = {},
+                           SmallVectorImpl<ValueDecl *> *allResults = nullptr);
 }; // namespace TypeChecker
 
 /// Temporary on-stack storage and unescaping for encoded diagnostic

test/IDE/print_swift_module.swift

@@ -28,7 +28,9 @@ public func returnsAlias() -> Alias<Int> {
 }
 
 @_functionBuilder
-struct BridgeBuilder {}
+struct BridgeBuilder {
+    static func buildBlock(_: Any...) {}
+}
 
 public struct City {
   public init(@BridgeBuilder builder: () -> ()) {}

test/decl/var/function_builders.swift

@@ -7,10 +7,10 @@ var globalBuilder: Int
 func globalBuilderFunction() -> Int { return 0 }
 
 @_functionBuilder
-struct Maker {}
+struct Maker {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
 
 @_functionBuilder
-class Inventor {}
+class Inventor {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
 
 @Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter, function, or computed property}}
 typealias typename = Inventor
@@ -66,11 +66,11 @@ func makerParamAutoclosure(@Maker // expected-error {{function builder attribute
                            fn: @autoclosure () -> ()) {}
 
 @_functionBuilder
-struct GenericMaker<T> {} // expected-note {{generic type 'GenericMaker' declared here}}
+struct GenericMaker<T> {} // expected-note {{generic type 'GenericMaker' declared here}} expected-error {{function builder must provide at least one static 'buildBlock' method}}
 
 struct GenericContainer<T> {  // expected-note {{generic type 'GenericContainer' declared here}}
   @_functionBuilder
-  struct Maker {}
+  struct Maker {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
 }
 
 func makeParamUnbound(@GenericMaker // expected-error {{reference to generic type 'GenericMaker' requires arguments}}
@@ -89,7 +89,7 @@ func makeParamNestedBound(@GenericContainer<Int>.Maker
 protocol P { }
 
 @_functionBuilder
-struct ConstrainedGenericMaker<T: P> {}
+struct ConstrainedGenericMaker<T: P> {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
 
 
 struct WithinGeneric<U> {
@@ -99,3 +99,121 @@ struct WithinGeneric<U> {
   func makeParamBoundInContextBad(@ConstrainedGenericMaker<U>
     fn: () -> ()) {}
 }
+
+@_functionBuilder
+struct ValidBuilder1 {
+  static func buildBlock(_ exprs: Any...) -> Int { return exprs.count }
+}
+
+protocol BuilderFuncHelper {}
+
+extension BuilderFuncHelper {
+  static func buildBlock(_ exprs: Any...) -> Int { return exprs.count }
+}
+
+@_functionBuilder
+struct ValidBuilder2: BuilderFuncHelper {}
+
+class BuilderFuncBase {
+  static func buildBlock(_ exprs: Any...) -> Int { return exprs.count }
+}
+
+@_functionBuilder
+class ValidBuilder3: BuilderFuncBase {}
+
+@_functionBuilder
+struct ValidBuilder4 {}
+extension ValidBuilder4 {
+    static func buildBlock(_ exprs: Any...) -> Int { return exprs.count }
+}
+
+@_functionBuilder
+struct ValidBuilder5 {
+    static func buildBlock() -> Int { 0 }
+}
+
+@_functionBuilder
+struct InvalidBuilder1 {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+
+@_functionBuilder
+struct InvalidBuilder2 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+  func buildBlock(_ exprs: Any...) -> Int { return exprs.count } // expected-note {{did you mean to make instance method 'buildBlock' static?}} {{3-3=static }}
+}
+
+@_functionBuilder
+struct InvalidBuilder3 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+  var buildBlock: (Any...) -> Int = { return $0.count } // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+@_functionBuilder
+struct InvalidBuilder4 {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+extension InvalidBuilder4 {
+  func buildBlock(_ exprs: Any...) -> Int { return exprs.count } // expected-note {{did you mean to make instance method 'buildBlock' static?}} {{3-3=static }}
+}
+
+protocol InvalidBuilderHelper {}
+extension InvalidBuilderHelper {
+  func buildBlock(_ exprs: Any...) -> Int { return exprs.count } // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+@_functionBuilder
+struct InvalidBuilder5: InvalidBuilderHelper {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+
+@_functionBuilder
+struct InvalidBuilder6 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+    static var buildBlock: Int = 0 // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+struct Callable {
+    func callAsFunction(_ exprs: Any...) -> Int { return exprs.count }
+}
+
+@_functionBuilder
+struct InvalidBuilder7 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+    static var buildBlock = Callable() // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+class BuilderVarBase {
+  static var buildBlock: (Any...) -> Int = { return $0.count } // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+@_functionBuilder
+class InvalidBuilder8: BuilderVarBase {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+
+protocol BuilderVarHelper {}
+
+extension BuilderVarHelper {
+  static var buildBlock: (Any...) -> Int { { return $0.count } } // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+@_functionBuilder
+struct InvalidBuilder9: BuilderVarHelper {} // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+
+@_functionBuilder
+struct InvalidBuilder10 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+  static var buildBlock: (Any...) -> Int = { return $0.count } // expected-note {{potential match 'buildBlock' is not a static method}}
+}
+
+@_functionBuilder
+enum InvalidBuilder11 { // expected-error {{function builder must provide at least one static 'buildBlock' method}}
+    case buildBlock(Any) // expected-note {{enum case 'buildBlock' cannot be used to satisfy the function builder requirement}}
+}
+
+struct S {
+  @ValidBuilder1 var v1: Int { 1 }
+  @ValidBuilder2 var v2: Int { 1 }
+  @ValidBuilder3 var v3: Int { 1 }
+  @ValidBuilder4 var v4: Int { 1 }
+  @ValidBuilder5 func v5() -> Int {}
+  @InvalidBuilder1 var i1: Int { 1 } // expected-error {{type 'InvalidBuilder1' has no member 'buildBlock'}}
+  @InvalidBuilder2 var i2: Int { 1 } // expected-error {{instance member 'buildBlock' cannot be used on type 'InvalidBuilder2'; did you mean to use a value of this type instead?}}
+  @InvalidBuilder3 var i3: Int { 1 } // expected-error {{instance member 'buildBlock' cannot be used on type 'InvalidBuilder3'; did you mean to use a value of this type instead?}}
+  @InvalidBuilder4 var i4: Int { 1 } // expected-error {{instance member 'buildBlock' cannot be used on type 'InvalidBuilder4'; did you mean to use a value of this type instead?}}
+  @InvalidBuilder5 var i5: Int { 1 } // expected-error {{instance member 'buildBlock' cannot be used on type 'InvalidBuilder5'; did you mean to use a value of this type instead?}}
+  @InvalidBuilder6 var i6: Int { 1 } // expected-error {{cannot call value of non-function type 'Int'}}
+  @InvalidBuilder7 var i7: Int { 1 }
+  @InvalidBuilder8 var i8: Int { 1 }
+  @InvalidBuilder9 var i9: Int { 1 }
+  @InvalidBuilder10 var i10: Int { 1 }
+  @InvalidBuilder11 var i11: InvalidBuilder11 { 1 }
+}