Serialize package and SPI doc comments in swiftsourceinfo

Previously we were using the same set of conditions
for serializing as for swiftdoc, so excluded them.
However it's reasonable to have them in the
swiftsourceinfo.

Author: hamishknight

lib/Serialization/SerializeDoc.cpp

@@ -331,29 +331,42 @@ static bool hasDoubleUnderscore(Decl *D) {
   return false;
 }
 
-static bool shouldIncludeDecl(Decl *D, bool ExcludeDoubleUnderscore) {
+static bool shouldIncludeDecl(Decl *D, bool ForSourceInfo) {
   if (auto *VD = dyn_cast<ValueDecl>(D)) {
+    // When emitting for .swiftsourceinfo, we can include package decls.
+    // Otherwise, the decl must be public.
+    auto MinAccess = ForSourceInfo ? swift::AccessLevel::Package
+                                   : swift::AccessLevel::Public;
+
     // Skip the decl if it's not visible to clients. The use of
     // getEffectiveAccess is unusual here; we want to take the testability
     // state into account and emit documentation if and only if they are
     // visible to clients (which means public ordinarily, but
     // public+internal when testing enabled).
-    if (VD->getEffectiveAccess() < swift::AccessLevel::Public)
+    if (VD->getEffectiveAccess() < MinAccess)
       return false;
   }
 
-  // Skip SPI decls, unless we're generating a symbol graph with SPI information.
-  if (D->isSPI() && !D->getASTContext().SymbolGraphOpts.IncludeSPISymbols)
-    return false;
+  // Apply some extra filters, unless we're emitting .swiftsourceinfo, where
+  // we can be more relaxed with what we emit.
+  if (!ForSourceInfo) {
+    // Skip SPI decls, unless we're generating a symbol graph with SPI
+    // information.
+    if (D->isSPI() && !D->getASTContext().SymbolGraphOpts.IncludeSPISymbols)
+      return false;
+
+    // .swiftdoc doesn't include comments for double underscored symbols, but
+    // for .swiftsourceinfo, having the source location for these symbols isn't
+    // a concern because these symbols are in .swiftinterface anyway.
+    if (hasDoubleUnderscore(D))
+      return false;
+  }
 
   if (auto *ED = dyn_cast<ExtensionDecl>(D)) {
     auto *extended = ED->getExtendedNominal();
     if (!extended)
       return false;
-    return shouldIncludeDecl(extended, ExcludeDoubleUnderscore);
-  }
-  if (ExcludeDoubleUnderscore && hasDoubleUnderscore(D)) {
-    return false;
+    return shouldIncludeDecl(extended, ForSourceInfo);
   }
   return true;
 }
@@ -419,7 +432,7 @@ static void writeDeclCommentTable(
     }
 
     PreWalkAction walkToDeclPre(Decl *D) override {
-      if (!shouldIncludeDecl(D, /*ExcludeDoubleUnderscore*/true))
+      if (!shouldIncludeDecl(D, /*ForSourceInfo*/false))
         return Action::SkipChildren();
       if (!shouldSerializeDoc(D))
         return Action::Continue();
@@ -731,10 +744,7 @@ struct BasicDeclLocsTableWriter : public ASTWalker {
   }
 
   PreWalkAction walkToDeclPre(Decl *D) override {
-    // .swiftdoc doesn't include comments for double underscored symbols, but
-    // for .swiftsourceinfo, having the source location for these symbols isn't
-    // a concern because these symbols are in .swiftinterface anyway.
-    if (!shouldIncludeDecl(D, /*ExcludeDoubleUnderscore*/false))
+    if (!shouldIncludeDecl(D, /*ForSourceInfo*/true))
       return Action::SkipChildren();
     if (!shouldSerializeSourceLoc(D))
       return Action::Continue();

test/Serialization/Inputs/def_comments.swift

@@ -27,5 +27,11 @@ public protocol second_decl_protocol_1 {
 
 public var (decl_var_2, decl_var_3): (Int, Float) = (1, 1.0)
 
+/// Comment on package function
+package func second_package_function() {}
+
+/// Comment on SPI function
+@_spi(DocSPI) public func second_spi_function() {}
+
 #sourceLocation(file:"NonExistingSource.swift", line:100000)
 public func functionAfterPoundSourceLoc() {}

test/Serialization/comments.swift

@@ -1,7 +1,7 @@
 // Test the case when we have a single file in a module.
 //
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/comments.swiftmodule -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc -emit-module-source-info-path %t/comments.swiftsourceinfo %s
+// RUN: %target-swift-frontend -module-name comments -package-name comments -emit-module -emit-module-path %t/comments.swiftmodule -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc -emit-module-source-info-path %t/comments.swiftsourceinfo %s
 // RUN: llvm-bcanalyzer %t/comments.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER
 // RUN: llvm-bcanalyzer %t/comments.swiftdoc | %FileCheck %s -check-prefix=BCANALYZER
 // RUN: llvm-bcanalyzer %t/comments.swiftsourceinfo | %FileCheck %s -check-prefix=BCANALYZER
@@ -10,9 +10,9 @@
 // Test the case when we have a multiple files in a module.
 //
 // RUN: %empty-directory(%t)
-// RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/first.swiftmodule -emit-module-doc -emit-module-doc-path %t/first.swiftdoc -primary-file %s %S/Inputs/def_comments.swift -emit-module-source-info-path %t/first.swiftsourceinfo
-// RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/second.swiftmodule -emit-module-doc -emit-module-doc-path %t/second.swiftdoc %s -primary-file %S/Inputs/def_comments.swift -emit-module-source-info-path %t/second.swiftsourceinfo
-// RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/comments.swiftmodule -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc %t/first.swiftmodule %t/second.swiftmodule -emit-module-source-info-path %t/comments.swiftsourceinfo
+// RUN: %target-swift-frontend -module-name comments -package-name comments -emit-module -emit-module-path %t/first.swiftmodule -emit-module-doc -emit-module-doc-path %t/first.swiftdoc -primary-file %s %S/Inputs/def_comments.swift -emit-module-source-info-path %t/first.swiftsourceinfo
+// RUN: %target-swift-frontend -module-name comments -package-name comments -emit-module -emit-module-path %t/second.swiftmodule -emit-module-doc -emit-module-doc-path %t/second.swiftdoc %s -primary-file %S/Inputs/def_comments.swift -emit-module-source-info-path %t/second.swiftsourceinfo
+// RUN: %target-swift-frontend -module-name comments -package-name comments -emit-module -emit-module-path %t/comments.swiftmodule -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc %t/first.swiftmodule %t/second.swiftmodule -emit-module-source-info-path %t/comments.swiftsourceinfo
 // RUN: llvm-bcanalyzer %t/comments.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER
 // RUN: llvm-bcanalyzer %t/comments.swiftdoc | %FileCheck %s -check-prefix=BCANALYZER
 // RUN: llvm-bcanalyzer %t/comments.swiftsourceinfo | %FileCheck %s -check-prefix=BCANALYZER
@@ -60,16 +60,26 @@ public protocol P1 { }
 /// Comment for no member extension
 extension first_decl_class_1 : P1 {}
 
+/// Comment on package function
+package func first_package_function() {}
+
+/// Comment on SPI function
+@_spi(DocSPI) public func first_spi_function() {}
+
 // FIRST: comments.swift:26:14: Class/first_decl_generic_class_1 RawComment=[/// first_decl_generic_class_1 Aaa.\n]
 // FIRST: comments.swift:28:3: Destructor/first_decl_generic_class_1.deinit RawComment=[/// deinit of first_decl_generic_class_1 Aaa.\n]
 // FIRST: comments.swift:33:14: Class/first_decl_class_1 RawComment=[/// first_decl_class_1 Aaa.\n]
 // FIRST: comments.swift:36:15: Func/first_decl_class_1.decl_func_1 RawComment=[/// decl_func_1 Aaa.\n]
 // FIRST: comments.swift:41:15: Func/first_decl_class_1.decl_func_2 RawComment=[/**\n   * decl_func_3 Aaa.\n   */]
 // FIRST: comments.swift:45:15: Func/first_decl_class_1.decl_func_3 RawComment=[/// decl_func_3 Aaa.\n/** Bbb. */]
+// FIRST: comments.swift:64:14: Func/first_package_function RawComment=[/// Comment on package function\n]
+// FIRST: comments.swift:67:27: Func/first_spi_function RawComment=[/// Comment on SPI function\n]
 
 // SECOND: comments.swift:49:1: Extension/ RawComment=[/// Comment for bar1\n] BriefComment=[Comment for bar1]
 // SECOND: comments.swift:54:1: Extension/ RawComment=[/// Comment for bar2\n] BriefComment=[Comment for bar2]
 // SECOND: comments.swift:61:1: Extension/ RawComment=[/// Comment for no member extension\n] BriefComment=[Comment for no member extension]
+// SECOND: comments.swift:64:14: Func/first_package_function RawComment=[/// Comment on package function\n]
+// SECOND: comments.swift:67:27: Func/first_spi_function RawComment=[/// Comment on SPI function\n]
 // SECOND: Inputs/def_comments.swift:2:14: Class/second_decl_class_1 RawComment=[/// second_decl_class_1 Aaa.\n]
 // SECOND: Inputs/def_comments.swift:5:15: Struct/second_decl_struct_1
 // SECOND: Inputs/def_comments.swift:7:9: Accessor/second_decl_struct_1.<getter for second_decl_struct_1.instanceVar>
@@ -88,13 +98,23 @@ extension first_decl_class_1 : P1 {}
 // SECOND: Inputs/def_comments.swift:28:13: Var/decl_var_2 RawComment=none BriefComment=none DocCommentAsXML=none
 // SECOND: Inputs/def_comments.swift:28:25: Var/decl_var_3 RawComment=none BriefComment=none DocCommentAsXML=none
 // SECOND: Inputs/def_comments.swift:28:25: Var/decl_var_3 RawComment=none BriefComment=none DocCommentAsXML=none
+// SECOND: Inputs/def_comments.swift:31:14: Func/second_package_function RawComment=[/// Comment on package function\n]
+// SECOND: Inputs/def_comments.swift:34:27: Func/second_spi_function RawComment=[/// Comment on SPI function\n]
 // SECOND: NonExistingSource.swift:100000:13: Func/functionAfterPoundSourceLoc
 
+// Package and SPI functions won't show up in the (public) swiftinterface
+// INTERFACE: comments.swift:26:14: Class/first_decl_generic_class_1 RawComment=[/// first_decl_generic_class_1 Aaa.\n]
+// INTERFACE: comments.swift:28:3: Destructor/first_decl_generic_class_1.deinit RawComment=[/// deinit of first_decl_generic_class_1 Aaa.\n]
+// INTERFACE: comments.swift:33:14: Class/first_decl_class_1 RawComment=[/// first_decl_class_1 Aaa.\n]
+// INTERFACE: comments.swift:36:15: Func/first_decl_class_1.decl_func_1 RawComment=[/// decl_func_1 Aaa.\n]
+// INTERFACE: comments.swift:41:15: Func/first_decl_class_1.decl_func_2 RawComment=[/**\n   * decl_func_3 Aaa.\n   */]
+// INTERFACE: comments.swift:45:15: Func/first_decl_class_1.decl_func_3 RawComment=[/// decl_func_3 Aaa.\n/** Bbb. */]
+
 // Test the case when we have to import via a .swiftinterface file.
 //
 // RUN: %empty-directory(%t)
 // RUN: %empty-directory(%t/Hidden)
-// RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/Hidden/comments.swiftmodule -emit-module-interface-path %t/comments.swiftinterface -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc -emit-module-source-info-path %t/comments.swiftsourceinfo %s -enable-library-evolution -swift-version 5
+// RUN: %target-swift-frontend -module-name comments -package-name comments -emit-module -emit-module-path %t/Hidden/comments.swiftmodule -emit-module-interface-path %t/comments.swiftinterface -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc -emit-module-source-info-path %t/comments.swiftsourceinfo %s -enable-library-evolution -swift-version 5
 // RUN: llvm-bcanalyzer %t/comments.swiftdoc | %FileCheck %s -check-prefix=BCANALYZER
 // RUN: llvm-bcanalyzer %t/comments.swiftsourceinfo | %FileCheck %s -check-prefix=BCANALYZER
-// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t -swift-version 5 | %FileCheck %s -check-prefix=FIRST
+// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t -swift-version 5 | %FileCheck %s -check-prefix=INTERFACE

test/SourceKit/CodeComplete/complete_docbrief_package.swift

@@ -0,0 +1,45 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/Modules)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend \
+// RUN:     -emit-module \
+// RUN:     -module-name DocBriefTest \
+// RUN:     -package-name DocPackage \
+// RUN:     -emit-module-path %t/Modules/DocBriefTest.swiftmodule \
+// RUN:     -emit-module-source-info-path %t/Modules/DocBriefTest.swiftsourceinfo \
+// RUN:     %t/Module.swift
+
+//--- Module.swift
+package protocol P {
+  /// This is a doc comment of P.foo
+  ///
+  /// Do whatever.
+  func foo()
+}
+
+package struct S: P {
+  public init() {}
+  public func foo() {}
+}
+
+//--- User.swift
+package import DocBriefTest
+
+func test() {
+  S().foo()
+}
+
+// RUN: %sourcekitd-test -req=complete -pos=4:7 %t/User.swift -- %t/User.swift -I %t/Modules -target %target-triple -module-name DocBriefUser -package-name DocPackage -enable-experimental-feature AccessLevelOnImport | %FileCheck %s -check-prefix=CHECK
+
+// CHECK: {
+// CHECK:   key.results: [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       key.kind: source.lang.swift.decl.function.method.instance,
+// CHECK-NEXT:       key.name: "foo()",
+// CHECK-NEXT:       key.sourcetext: "foo()",
+// CHECK-NEXT:       key.description: "foo()",
+// CHECK-NEXT:       key.typename: "Void",
+// CHECK-NEXT:       key.doc.brief: "This is a doc comment of P.foo",
+// CHECK:          }
+

test/SourceKit/CodeComplete/complete_docbrief_spi.swift

@@ -0,0 +1,45 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/Modules)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend \
+// RUN:     -emit-module \
+// RUN:     -module-name DocBriefTest \
+// RUN:     -emit-module-path %t/Modules/DocBriefTest.swiftmodule \
+// RUN:     -emit-module-source-info-path %t/Modules/DocBriefTest.swiftsourceinfo \
+// RUN:     %t/Module.swift
+
+//--- Module.swift
+@_spi(SomeSPI)
+public protocol P {
+  /// This is a doc comment of P.foo
+  ///
+  /// Do whatever.
+  func foo()
+}
+
+@_spi(SomeSPI)
+public struct S: P {
+  public init() {}
+  public func foo() {}
+}
+
+//--- User.swift
+@_spi(SomeSPI) import DocBriefTest
+
+func test() {
+  S().foo()
+}
+
+// RUN: %sourcekitd-test -req=complete -pos=4:7 %t/User.swift -- %t/User.swift -I %t/Modules -target %target-triple -module-name DocBriefUser | %FileCheck %s -check-prefix=CHECK
+
+// CHECK: {
+// CHECK:   key.results: [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       key.kind: source.lang.swift.decl.function.method.instance,
+// CHECK-NEXT:       key.name: "foo()",
+// CHECK-NEXT:       key.sourcetext: "foo()",
+// CHECK-NEXT:       key.description: "foo()",
+// CHECK-NEXT:       key.typename: "Void",
+// CHECK-NEXT:       key.doc.brief: "This is a doc comment of P.foo",
+// CHECK:          }