Only emit error about missing import when we're missing the import

This eliminates some diagnostics that incorrectly refer to a missin
`@_spi` import as an outright missing import, which is incorrect. The
one test case changed here really is a case of a missing import that
happens to be SPI.

Author: DougGregor

lib/Sema/CSDiagnostics.cpp

@@ -6164,49 +6164,60 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
   auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
   auto accessLevel = Member->getFormalAccessScope().accessLevelForDiagnostics();
   bool suppressDeclHereNote = false;
-  if (accessLevel == AccessLevel::Public) {
+  if (accessLevel == AccessLevel::Public &&
+      !Member->findImport(getDC())) {
     auto definingModule = Member->getDeclContext()->getParentModule();
     emitDiagnosticAt(loc, diag::candidate_from_missing_import,
                      Member->getDescriptiveKind(), Member->getName(),
                      definingModule->getName());
 
-    if (auto enclosingSF = getDC()->getParentSourceFile()) {
-      SourceLoc bestLoc;
-      SourceManager &srcMgr = Member->getASTContext().SourceMgr;
-      for (auto item : enclosingSF->getTopLevelItems()) {
-        // If we found an import declaration, we want to insert after it.
-        if (auto importDecl =
-                dyn_cast_or_null<ImportDecl>(item.dyn_cast<Decl *>())) {
-          SourceLoc loc = importDecl->getEndLoc();
-          if (loc.isValid()) {
-            bestLoc = Lexer::getLocForEndOfLine(srcMgr, loc);
-          }
-
-          // Keep looking for more import declarations.
-          continue;
-        }
-
-        // If we got a location based on import declarations, we're done.
-        if (bestLoc.isValid())
-          break;
-        
-        // For any other item, we want to insert before it.
-        SourceLoc loc = item.getStartLoc();
+    auto enclosingSF = getDC()->getParentSourceFile();
+    SourceLoc bestLoc;
+    SourceManager &srcMgr = Member->getASTContext().SourceMgr;
+    for (auto item : enclosingSF->getTopLevelItems()) {
+      // If we found an import declaration, we want to insert after it.
+      if (auto importDecl =
+              dyn_cast_or_null<ImportDecl>(item.dyn_cast<Decl *>())) {
+        SourceLoc loc = importDecl->getEndLoc();
         if (loc.isValid()) {
-          bestLoc = Lexer::getLocForStartOfLine(srcMgr, loc);
-          break;
+          bestLoc = Lexer::getLocForEndOfLine(srcMgr, loc);
         }
+
+        // Keep looking for more import declarations.
+        continue;
+      }
+
+      // If we got a location based on import declarations, we're done.
+      if (bestLoc.isValid())
+        break;
+
+      // For any other item, we want to insert before it.
+      SourceLoc loc = item.getStartLoc();
+      if (loc.isValid()) {
+        bestLoc = Lexer::getLocForStartOfLine(srcMgr, loc);
+        break;
       }
+    }
+
+    if (bestLoc.isValid()) {
+      llvm::SmallString<64> importText;
 
-      if (bestLoc.isValid()) {
-        llvm::SmallString<64> importText;
-        importText += "import ";
-        importText += definingModule->getName().str();
-        importText += "\n";
-        emitDiagnosticAt(bestLoc, diag::candidate_add_import,
-                         definingModule->getName())
-          .fixItInsert(bestLoc, importText);
+      // @_spi imports.
+      if (Member->isSPI()) {
+        auto spiGroups = Member->getSPIGroups();
+        if (!spiGroups.empty()) {
+          importText += "@_spi(";
+          importText += spiGroups[0].str();
+          importText += ") ";
+        }
       }
+
+      importText += "import ";
+      importText += definingModule->getName().str();
+      importText += "\n";
+      emitDiagnosticAt(bestLoc, diag::candidate_add_import,
+                       definingModule->getName())
+        .fixItInsert(bestLoc, importText);
     }
 
     suppressDeclHereNote = true;
@@ -6220,7 +6231,8 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
         .highlight(nameLoc.getSourceRange());
   }
 
-  emitDiagnosticAt(Member, diag::decl_declared_here, Member);
+  if (!suppressDeclHereNote)
+    emitDiagnosticAt(Member, diag::decl_declared_here, Member);
   return true;
 }
 

test/SPI/client_reuse_spi.swift

@@ -6,7 +6,7 @@
 // RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unknown -DLIB_C %s -I %t
 
 #if LIB_A
-
+// expected-note@-1{{add import of module 'A'}}{{1-1=@_spi(A) import A\n}}
 @_spi(A) public struct SecretStruct {
   @_spi(A) public func bar() {}
 }
@@ -22,7 +22,7 @@
 @_spi(B) import B
 
 var a = foo() // OK
-a.bar() // expected-error{{'bar' is inaccessible due to '@_spi' protection level}}
+a.bar() // expected-error{{instance method 'bar()' is not available due to missing import of defining module 'A'}}
 
 var b = SecretStruct() // expected-error{{cannot find 'SecretStruct' in scope}}