Merge pull request #80476 from beccadax/implement-this

Stub fix-its for missing objcImpl requirements

Author: beccadax

include/swift/AST/ASTPrinter.h

@@ -413,7 +413,8 @@ class ExtraIndentStreamPrinter : public StreamPrinter {
 void printContext(raw_ostream &os, DeclContext *dc);
 
 bool printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
-                          Type AdopterTy, SourceLoc TypeLoc, raw_ostream &OS);
+                          Type AdopterTy, SourceLoc TypeLoc, raw_ostream &OS,
+                          bool withExplicitObjCAttr = false);
 
 /// Print a keyword or punctuator directly by its kind.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, tok keyword);

include/swift/AST/DiagnosticsSema.def

@@ -1980,10 +1980,19 @@ ERROR(objc_implementation_wrong_swift_name,none,
       "you mean %1?",
       (ObjCSelector, const ValueDecl *))
 
-ERROR(objc_implementation_missing_impl,none,
-      "extension for %select{main class interface|category %0}0 should "
-      "provide implementation for %kind1",
-      (Identifier, ValueDecl *))
+ERROR(objc_implementation_missing_impls,none,
+      "extension for %select{main class interface|category %0}0 does not "
+      "provide all required implementations",
+      (Identifier))
+NOTE(objc_implementation_missing_impls_fixit,none,
+     "add stub%s0 for missing '@implementation' requirement%s0",
+     (unsigned))
+NOTE(objc_implementation_missing_impl,none,
+     "missing %kind0",
+     (ValueDecl *))
+NOTE(objc_implementation_missing_impl_either,none,
+     "missing %kind0 or %1",
+     (ValueDecl *, ValueDecl *))
 
 ERROR(objc_implementation_class_or_instance_mismatch,none,
       "%kind0 does not match %kindonly1 declared in header",

include/swift/AST/PrintOptions.h

@@ -143,6 +143,9 @@ struct PrintOptions {
   /// Whether to print *any* accessors on properties.
   bool PrintPropertyAccessors = true;
 
+  /// Use \c let for a read-only computed property.
+  bool InferPropertyIntroducerFromAccessors = false;
+
   /// Whether to print *any* accessors on subscript.
   bool PrintSubscriptAccessors = true;
 
@@ -172,6 +175,10 @@ struct PrintOptions {
   /// Whether to print the bodies of accessors in protocol context.
   bool PrintAccessorBodiesInProtocols = false;
 
+  /// Whether to print the parameter list of accessors like \c set . (Even when
+  /// \c true , parameters marked implicit still won't be printed.)
+  bool PrintExplicitAccessorParameters = true;
+
   /// Whether to print type definitions.
   bool TypeDefinitions = false;
 

lib/AST/ASTPrinter.cpp

@@ -3932,11 +3932,18 @@ void PrintAST::visitVarDecl(VarDecl *decl) {
   if (decl->isStatic() && Options.PrintStaticKeyword)
     printStaticKeyword(decl->getCorrectStaticSpelling());
   if (decl->getKind() == DeclKind::Var || Options.PrintParameterSpecifiers) {
+    // If InferPropertyIntroducerFromAccessors is set, turn all read-only
+    // properties to `let`.
+    auto introducer = decl->getIntroducer();
+    if (Options.InferPropertyIntroducerFromAccessors &&
+          !cast<VarDecl>(decl)->isSettable(nullptr))
+      introducer = VarDecl::Introducer::Let;
+
     // Map all non-let specifiers to 'var'.  This is not correct, but
     // SourceKit relies on this for info about parameter decls.
 
     Printer.printIntroducerKeyword(
-      decl->getIntroducer() == VarDecl::Introducer::Let ? "let" : "var",
+      introducer == VarDecl::Introducer::Let ? "let" : "var",
       Options, " ");
   }
   printContextIfNeeded(decl);
@@ -4221,7 +4228,8 @@ void PrintAST::visitAccessorDecl(AccessorDecl *decl) {
         Printer << getAccessorLabel(decl->getAccessorKind());
 
         auto params = decl->getParameters();
-        if (params->size() != 0 && !params->get(0)->isImplicit()) {
+        if (params->size() != 0 && !params->get(0)->isImplicit()
+              && Options.PrintExplicitAccessorParameters) {
           auto Name = params->get(0)->getName();
           if (!Name.empty()) {
             Printer << "(";

lib/Sema/TypeCheckDeclObjC.cpp

@@ -19,6 +19,7 @@
 #include "TypeCheckProtocol.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTContext.h"
+#include "swift/AST/ASTPrinter.h"
 #include "swift/AST/AvailabilityInference.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/ExistentialLayout.h"
@@ -4038,18 +4039,46 @@ class ObjCImplementationChecker {
 
 public:
   void diagnoseUnmatchedRequirements() {
+    auto ext = dyn_cast<ExtensionDecl>(decl);
+    if (!ext)
+      return;
+
+    llvm::SmallString<128> stubs;
+    llvm::raw_svector_ostream stubStream(stubs);
+
+    unsigned numEmitted = 0;
+
     for (auto req : unmatchedRequirements) {
       // Ignore `@optional` protocol requirements.
       if (isOptionalObjCProtocolRequirement(req))
         continue;
 
-      auto ext = cast<IterableDeclContext>(req->getDeclContext()->getAsDecl())
-                        ->getImplementationContext();
+      if (numEmitted == 0) {
+        // Emit overall diagnostic for all the notes to attach to.
+        diagnose(ext, diag::objc_implementation_missing_impls,
+                 getCategoryName(req->getDeclContext()));
+      }
 
-      diagnose(ext->getDecl(), diag::objc_implementation_missing_impl,
-               getCategoryName(req->getDeclContext()), req);
+      numEmitted += 1;
+
+      // Emit different diagnostic if there's an async alternative.
+      if (auto asyncAlternative = getAsyncAlternative(req)) {
+        diagnose(ext, diag::objc_implementation_missing_impl_either,
+                 asyncAlternative, req);
+        req = asyncAlternative;
+      } else {
+        diagnose(ext, diag::objc_implementation_missing_impl, req);
+      }
+
+      // Append stub for this requirement into eventual fix-it.
+      swift::printRequirementStub(req, ext, ext->getSelfInterfaceType(),
+                                  ext->getStartLoc(), stubStream,
+                                  /*objCAttr=*/true);
+    }
 
-      // FIXME: Should give fix-it to add stub implementation
+    if (!stubs.empty()) {
+      diagnose(ext, diag::objc_implementation_missing_impls_fixit, numEmitted)
+        .fixItInsertAfter(ext->getBraces().Start, stubs);
     }
   }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -3704,8 +3704,12 @@ static Type getTupleConformanceTypeWitness(DeclContext *dc,
 
 bool swift::
 printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
-                     Type AdopterTy, SourceLoc TypeLoc, raw_ostream &OS) {
-  if (isa<ConstructorDecl>(Requirement)) {
+                     Type AdopterTy, SourceLoc TypeLoc, raw_ostream &OS,
+                     bool withExplicitObjCAttr) {
+  // We sometimes use this for @implementation extensions too.
+  bool forProtocol = isa<ProtocolDecl>(Requirement->getDeclContext());
+
+  if (isa<ConstructorDecl>(Requirement) && forProtocol) {
     if (auto CD = Adopter->getSelfClassDecl()) {
       if (!CD->isSemanticallyFinal() && isa<ExtensionDecl>(Adopter)) {
         // In this case, user should mark class as 'final' or define
@@ -3731,15 +3735,35 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
   ExtraIndentStreamPrinter Printer(OS, StubIndent);
   Printer.printNewline();
 
+  PrintOptions Options = PrintOptions::printForDiagnostics(
+      AccessLevel::Private, Ctx.TypeCheckerOpts.PrintFullConvention);
+  Options.PrintDocumentationComments = false;
+  Options.PrintAccess = false;
+  Options.SkipAttributes = true;
+  Options.FunctionDefinitions = true;
+  Options.PrintAccessorBodiesInProtocols = true;
+  Options.PrintExplicitAccessorParameters = false;
+  Options.FullyQualifiedTypesIfAmbiguous = true;
+
+  if (withExplicitObjCAttr) {
+    if (auto runtimeName = Requirement->getObjCRuntimeName()) {
+      llvm::SmallString<32> scratch;
+      Printer.printAttrName("@objc");
+      Printer << "(" << runtimeName->getString(scratch) << ")";
+      Printer.printNewline();
+      Options.ExcludeAttrList.push_back(DeclAttrKind::ObjC);
+    }
+  }
+
   AccessLevel Access =
     std::min(
       /* Access of the context */
       Adopter->getSelfNominalTypeDecl()->getFormalAccess(),
       /* Access of the protocol */
-      Requirement->getDeclContext()->getSelfProtocolDecl()->
-        getFormalAccess());
-  if (Access == AccessLevel::Public)
-    Printer << "public ";
+      Requirement->getDeclContext()->getSelfNominalTypeDecl()
+                      ->getFormalAccess());
+  if (Access > AccessLevel::Internal)
+    Printer.printKeyword(getAccessLevelSpelling(Access), Options, " ");
 
   if (auto MissingTypeWitness = dyn_cast<AssociatedTypeDecl>(Requirement)) {
     Printer << "typealias " << MissingTypeWitness->getName() << " = ";
@@ -3753,7 +3777,7 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
 
     Printer << "\n";
   } else {
-    if (isa<ConstructorDecl>(Requirement)) {
+    if (isa<ConstructorDecl>(Requirement) && forProtocol) {
       if (auto CD = Adopter->getSelfClassDecl()) {
         if (!CD->isFinal()) {
           Printer << "required ";
@@ -3763,15 +3787,6 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
       }
     }
 
-    PrintOptions Options = PrintOptions::printForDiagnostics(
-        AccessLevel::Private, Ctx.TypeCheckerOpts.PrintFullConvention);
-    Options.PrintDocumentationComments = false;
-    Options.PrintAccess = false;
-    Options.SkipAttributes = true;
-    Options.FunctionDefinitions = true;
-    Options.PrintAccessorBodiesInProtocols = true;
-    Options.FullyQualifiedTypesIfAmbiguous = true;
-
     bool AdopterIsClass = Adopter->getSelfClassDecl() != nullptr;
     // Skip 'mutating' only inside classes: mutating methods usually
     // don't have a sensible non-mutating implementation.
@@ -3797,9 +3812,12 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
     };
     Options.setBaseType(AdopterTy);
     Options.CurrentModule = Adopter->getParentModule();
-    if (isa<NominalTypeDecl>(Adopter)) {
-      // Create a variable declaration instead of a computed property in
-      // nominal types...
+
+    // Can the conforming declaration declare a stored property?
+    auto ImplementedAdopter = Adopter->getImplementedObjCContext();
+    if (isa<NominalTypeDecl>(ImplementedAdopter) &&
+          (!isa<EnumDecl>(ImplementedAdopter) || Requirement->isStatic())) {
+      // Create a variable declaration instead of a computed property...
       Options.PrintPropertyAccessors = false;
 
       // ...but a non-mutating setter requirement will force us into a
@@ -3810,6 +3828,11 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
           if (const auto Set = VD->getOpaqueAccessor(AccessorKind::Set))
             if (Set->getAttrs().hasAttribute<NonMutatingAttr>())
               Options.PrintPropertyAccessors = true;
+
+      // If we're not printing the accessors, make them affect the introducer
+      // instead.
+      Options.InferPropertyIntroducerFromAccessors =
+          !Options.PrintPropertyAccessors;
     }
     Requirement->print(Printer, Options);
     Printer << "\n";

test/ClangImporter/rdar123543707.swift

@@ -7,8 +7,10 @@ import Module
 import Module_Private.Sub4
 
 @_objcImplementation extension Module {
-  // expected-warning@-1 {{extension for main class interface should provide implementation for class method 'version()'}}
-  // expected-warning@-2 {{extension for main class interface should provide implementation for class method 'alloc()'}}
+  // expected-warning@-1 {{extension for main class interface does not provide all required implementations}}
+  // expected-note@-2 {{missing class method 'version()'}}
+  // expected-note@-3 {{missing class method 'alloc()'}}
+  // expected-note@-4 {{add stubs for missing '@implementation' requirements}} {{40-40=\n    @objc(version)\n    open class func version() -> UnsafePointer<CChar>! {\n        <#code#>\n    \}\n\n    @objc(alloc)\n    open class func alloc() -> Self! {\n        <#code#>\n    \}\n}}
 }
 
 extension Module: @retroactive ModuleProto {} // no-error

test/Sema/fixits-derived-conformances.swift

@@ -17,5 +17,5 @@ extension GenericStruct: @retroactive Equatable { }
 
 extension Enum: @retroactive CaseIterable { }
 // expected-error@-1 {{extension outside of file declaring enum 'Enum' prevents automatic synthesis of 'allCases' for protocol 'CaseIterable'}}
-// expected-note@-2 {{add stubs for conformance}}{{44-44=\n    public static var allCases: [Enum]\n}}
+// expected-note@-2 {{add stubs for conformance}}{{44-44=\n    public static let allCases: [Enum]\n}}
 

test/decl/ext/Inputs/objc_implementation.h

@@ -53,6 +53,7 @@
 @property (readonly) int readonlyPropertyFromHeader4;
 @property (readonly) int readonlyPropertyFromHeader5;
 @property (readonly) int readonlyPropertyFromHeader6;
+@property (readonly) int readonlyPropertyFromHeader7;
 
 + (void)classMethod1:(int)param;
 + (void)classMethod2:(int)param;
@@ -84,6 +85,10 @@
 @property int categoryPropertyFromHeader2;
 @property int categoryPropertyFromHeader3;
 @property int categoryPropertyFromHeader4;
+@property int categoryPropertyFromHeader5;
+
+@property (readonly) int categoryReadonlyPropertyFromHeader1;
+
 
 @end
 
@@ -118,6 +123,7 @@
 - (void)doSomethingAsynchronousWithCompletionHandler:(void (^ _Nonnull)(id _Nullable result, NSError * _Nullable error))completionHandler;
 - (void)doSomethingElseAsynchronousWithCompletionHandler:(void (^ _Nullable)(id _Nonnull result))completionHandler;
 - (void)doSomethingFunAndAsynchronousWithCompletionHandler:(void (^ _Nonnull)(id _Nullable result, NSError * _Nullable error))completionHandler;
+- (void)doSomethingMissingAndAsynchronousWithCompletionHandler:(void (^ _Nonnull)(id _Nullable result, NSError * _Nullable error))completionHandler;
 
 - (void)doSomethingOverloadedWithCompletionHandler:(void (^ _Nonnull)())completionHandler;
 - (void)doSomethingOverloaded __attribute__((__swift_attr__("@_unavailableFromAsync(message: \"Use async doSomethingOverloaded instead.\")")));