Merge pull request #63726 from apple/QuietMisdreavus/skip-proto-impls

[SymbolGraphGen] add new flag to skip "protocol implementation" symbols

rdar://59899968

Author: QuietMisdreavus

Diff for: include/swift/Option/Options.td

@@ -1455,6 +1455,11 @@ def include_spi_symbols : Flag<["-"], "include-spi-symbols">,
          NoInteractiveOption, SupplementaryOutput, HelpHidden]>,
   HelpText<"Add symbols with SPI information to the symbol graph">;
 
+def skip_protocol_implementations : Flag<["-"], "skip-protocol-implementations">,
+  Flags<[SwiftSymbolGraphExtractOption, FrontendOption,
+         NoInteractiveOption, SupplementaryOutput, HelpHidden]>,
+  HelpText<"Skip emitting symbols that are implementations of protocol requirements or inherited from protocl extensions">;
+
 // swift-api-digester-only options
 def dump_sdk: Flag<["-", "--"], "dump-sdk">,
   Flags<[NoDriverOption, SwiftAPIDigesterOption]>,

Diff for: include/swift/SymbolGraphGen/SymbolGraphOptions.h

@@ -43,6 +43,10 @@ struct SymbolGraphOptions {
   /// Whether to skip docs for symbols with compound, "SYNTHESIZED" USRs.
   bool SkipInheritedDocs = false;
 
+  /// Whether to skip emitting symbols that are implementations of protocol requirements or
+  /// inherited from protocol extensions.
+  bool SkipProtocolImplementations = false;
+
   /// Whether to emit symbols with SPI information.
   bool IncludeSPISymbols = false;
 

Diff for: lib/DriverTool/swift_symbolgraph_extract_main.cpp

@@ -162,19 +162,18 @@ int swift_symbolgraph_extract_main(ArrayRef<const char *> Args,
     }
   }
 
-  symbolgraphgen::SymbolGraphOptions Options{
-      OutputDir,
-      Target,
-      ParsedArgs.hasArg(OPT_pretty_print),
-      AccessLevel::Public,
-      !ParsedArgs.hasArg(OPT_skip_synthesized_members),
-      ParsedArgs.hasArg(OPT_v),
-      ParsedArgs.hasArg(OPT_skip_inherited_docs),
-      ParsedArgs.hasArg(OPT_include_spi_symbols),
-      /*IncludeClangDocs=*/false,
+  symbolgraphgen::SymbolGraphOptions Options;
+  Options.OutputDir = OutputDir;
+  Options.Target = Target;
+  Options.PrettyPrint = ParsedArgs.hasArg(OPT_pretty_print);
+  Options.EmitSynthesizedMembers = !ParsedArgs.hasArg(OPT_skip_synthesized_members);
+  Options.PrintMessages = ParsedArgs.hasArg(OPT_v);
+  Options.SkipInheritedDocs = ParsedArgs.hasArg(OPT_skip_inherited_docs);
+  Options.SkipProtocolImplementations = ParsedArgs.hasArg(OPT_skip_protocol_implementations);
+  Options.IncludeSPISymbols = ParsedArgs.hasArg(OPT_include_spi_symbols);
+  Options.EmitExtensionBlockSymbols =
       ParsedArgs.hasFlag(OPT_emit_extension_block_symbols,
-                         OPT_omit_extension_block_symbols, /*default=*/false),
-  };
+                         OPT_omit_extension_block_symbols, /*default=*/false);
 
   if (auto *A = ParsedArgs.getLastArg(OPT_minimum_access_level)) {
     Options.MinimumAccessLevel =

Diff for: lib/Frontend/CompilerInvocation.cpp

@@ -1339,6 +1339,7 @@ static void ParseSymbolGraphArgs(symbolgraphgen::SymbolGraphOptions &Opts,
   Opts.Target = LangOpts.Target;
 
   Opts.SkipInheritedDocs = Args.hasArg(OPT_skip_inherited_docs);
+  Opts.SkipProtocolImplementations = Args.hasArg(OPT_skip_protocol_implementations);
   Opts.IncludeSPISymbols = Args.hasArg(OPT_include_spi_symbols);
   Opts.EmitExtensionBlockSymbols =
       Args.hasFlag(OPT_emit_extension_block_symbols,

Diff for: lib/SymbolGraphGen/Edge.cpp

@@ -20,33 +20,6 @@
 using namespace swift;
 using namespace symbolgraphgen;
 
-namespace {
-const ValueDecl *getForeignProtocolRequirement(const ValueDecl *VD, const ModuleDecl *M) {
-  std::queue<const ValueDecl *> requirements;
-  while (true) {
-    for (auto *req : VD->getSatisfiedProtocolRequirements()) {
-      if (req->getModuleContext()->getNameStr() != M->getNameStr())
-        return req;
-      else
-        requirements.push(req);
-    }
-    if (requirements.empty())
-      return nullptr;
-    VD = requirements.front();
-    requirements.pop();
-  }
-}
-
-const ValueDecl *getProtocolRequirement(const ValueDecl *VD) {
-  auto reqs = VD->getSatisfiedProtocolRequirements();
-
-  if (!reqs.empty())
-    return reqs.front();
-  else
-    return nullptr;
-}
-} // end anonymous namespace
-
 void Edge::serialize(llvm::json::OStream &OS) const {
   OS.object([&](){
     OS.attribute("kind", Kind.Name);
@@ -87,22 +60,7 @@ void Edge::serialize(llvm::json::OStream &OS) const {
       }
     }
 
-    const ValueDecl *InheritingDecl = nullptr;
-    if (const auto *ID = Source.getDeclInheritingDocs())
-      InheritingDecl = ID;
-
-    if (!InheritingDecl && Source.getSynthesizedBaseTypeDecl())
-      InheritingDecl = Source.getSymbolDecl();
-
-    if (!InheritingDecl) {
-      if (const auto *ID = getForeignProtocolRequirement(Source.getSymbolDecl(), &Graph->M))
-        InheritingDecl = ID;
-    }
-
-    if (!InheritingDecl) {
-      if (const auto *ID = getProtocolRequirement(Source.getSymbolDecl()))
-        InheritingDecl = ID;
-    }
+    const ValueDecl *InheritingDecl = Source.getInheritedDecl();
 
     // If our source symbol is a inheriting decl, write in information about
     // where it's inheriting docs from.

Diff for: lib/SymbolGraphGen/Symbol.cpp

@@ -30,6 +30,8 @@
 #include "SymbolGraphASTWalker.h"
 #include "DeclarationFragmentPrinter.h"
 
+#include <queue>
+
 using namespace swift;
 using namespace symbolgraphgen;
 
@@ -225,6 +227,60 @@ const ValueDecl *Symbol::getDeclInheritingDocs() const {
   }
 }
 
+const ValueDecl *Symbol::getForeignProtocolRequirement() const {
+  if (const auto *VD = dyn_cast_or_null<ValueDecl>(D)) {
+    std::queue<const ValueDecl *> requirements;
+    while (true) {
+      for (auto *req : VD->getSatisfiedProtocolRequirements()) {
+        if (req->getModuleContext()->getNameStr() != Graph->M.getNameStr())
+          return req;
+        else
+          requirements.push(req);
+      }
+      if (requirements.empty())
+        return nullptr;
+      VD = requirements.front();
+      requirements.pop();
+    }
+  }
+
+  return nullptr;
+}
+
+const ValueDecl *Symbol::getProtocolRequirement() const {
+  if (const auto *VD = dyn_cast_or_null<ValueDecl>(D)) {
+    auto reqs = VD->getSatisfiedProtocolRequirements();
+
+    if (!reqs.empty())
+      return reqs.front();
+    else
+      return nullptr;
+  }
+
+  return nullptr;
+}
+
+const ValueDecl *Symbol::getInheritedDecl() const {
+  const ValueDecl *InheritingDecl = nullptr;
+  if (const auto *ID = getDeclInheritingDocs())
+    InheritingDecl = ID;
+
+  if (!InheritingDecl && getSynthesizedBaseTypeDecl())
+    InheritingDecl = getSymbolDecl();
+
+  if (!InheritingDecl) {
+    if (const auto *ID = getForeignProtocolRequirement())
+      InheritingDecl = ID;
+  }
+
+  if (!InheritingDecl) {
+    if (const auto *ID = getProtocolRequirement())
+      InheritingDecl = ID;
+  }
+
+  return InheritingDecl;
+}
+
 namespace {
 
 StringRef getFileNameForDecl(const Decl *D) {

Diff for: lib/SymbolGraphGen/Symbol.h

@@ -130,6 +130,19 @@ class Symbol {
   /// the symbol has its own doc comments to render.
   const ValueDecl *getDeclInheritingDocs() const;
 
+  /// If this symbol is an implementation of a protocol requirement for a
+  /// protocol declared outside its module, returns the upstream decl for that
+  /// requirement.
+  const ValueDecl *getForeignProtocolRequirement() const;
+
+  /// If this symbol is an implementation of a protocol requirement, returns the
+  /// upstream decl for that requirement.
+  const ValueDecl *getProtocolRequirement() const;
+
+  /// If this symbol is a synthesized symbol or an implementation of a protocol
+  /// requirement, returns the upstream decl.
+  const ValueDecl *getInheritedDecl() const;
+
   static bool supportsKind(DeclKind Kind);
 
   /// Determines the effective access level of the given extension.

Diff for: lib/SymbolGraphGen/SymbolGraph.cpp

@@ -12,6 +12,7 @@
 
 #include "clang/AST/DeclObjC.h"
 #include "swift/AST/ASTContext.h"
+#include "swift/AST/Comment.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ProtocolConformance.h"
@@ -189,6 +190,15 @@ SymbolGraph::isRequirementOrDefaultImplementation(const ValueDecl *VD) const {
 // MARK: - Symbols (Nodes)
 
 void SymbolGraph::recordNode(Symbol S) {
+  if (Walker.Options.SkipProtocolImplementations && S.getInheritedDecl()) {
+    const auto *DocCommentProvidingDecl =
+      getDocCommentProvidingDecl(S.getLocalSymbolDecl(), /*AllowSerialized=*/true);
+
+    // allow implementation symbols to remain if they have their own comment
+    if (DocCommentProvidingDecl != S.getLocalSymbolDecl())
+      return;
+  }
+
   Nodes.insert(S);
 
   // Record all of the possible relationships (edges) originating
@@ -294,7 +304,7 @@ bool SymbolGraph::synthesizedMemberIsBestCandidate(const ValueDecl *VD,
 }
 
 void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
-  if (!Walker.Options.EmitSynthesizedMembers) {
+  if (!Walker.Options.EmitSynthesizedMembers || Walker.Options.SkipProtocolImplementations) {
     return;
   }
   const auto D = S.getLocalSymbolDecl();

Diff for: test/SymbolGraph/Symbols/SkipProtocolImplementations.swift

@@ -0,0 +1,45 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %s -module-name SkipProtocolImplementations -emit-module -emit-module-path %t/SkipProtocolImplementations.swiftmodule -emit-symbol-graph -emit-symbol-graph-dir %t/ -skip-protocol-implementations
+// RUN: %{python} -m json.tool %t/SkipProtocolImplementations.symbols.json %t/SkipProtocolImplementations.formatted.symbols.json
+// RUN: %FileCheck %s --input-file %t/SkipProtocolImplementations.formatted.symbols.json
+
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %s -module-name SkipProtocolImplementations -emit-module -emit-module-path %t/SkipProtocolImplementations.swiftmodule -emit-module-doc-path %t/SkipProtocolImplementations.swiftdoc
+// RUN: %target-swift-symbolgraph-extract -module-name SkipProtocolImplementations -I %t -skip-protocol-implementations -pretty-print -output-dir %t
+// RUN: %FileCheck %s --input-file %t/SkipProtocolImplementations.symbols.json
+
+// make sure that using `-skip-protocol-implementations` removes the functions from `SomeProtocol` on `SomeStruct`
+// CHECK-NOT: s:27SkipProtocolImplementations04SomeB0PAAE9bonusFuncyyF::SYNTHESIZED::s:27SkipProtocolImplementations10SomeStructV
+// CHECK-NOT: s:27SkipProtocolImplementations10SomeStructV8someFuncyyF
+
+// CHECK-LABEL: "symbols": [
+
+// SomeStruct.otherFunc() should be present because it has its own doc comment
+// CHECK: s:27SkipProtocolImplementations10SomeStructV9otherFuncyyF
+
+// CHECK-LABEL: "relationships": [
+
+// we want to make sure that the conformance relationship itself stays
+// CHECK-DAG: conformsTo
+
+// SomeStruct.otherFunc() should be the only one with sourceOrigin information
+// CHECK-COUNT-1: sourceOrigin
+
+public protocol SomeProtocol {
+    /// Base docs
+    func someFunc()
+
+    /// Base docs
+    func otherFunc()
+}
+
+public extension SomeProtocol {
+    func bonusFunc() {}
+}
+
+public struct SomeStruct: SomeProtocol {
+    public func someFunc() {}
+
+    /// Local docs
+    public func otherFunc() {}
+}

Diff for: tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -1013,18 +1013,12 @@ fillSymbolInfo(CursorSymbolInfo &Symbol, const DeclInfo &DInfo,
   if (AddSymbolGraph) {
     SmallVector<symbolgraphgen::PathComponent, 4> PathComponents;
     SmallVector<symbolgraphgen::FragmentInfo, 8> FragmentInfos;
-    symbolgraphgen::SymbolGraphOptions Options{
-        "",
-        Invoc.getLangOptions().Target,
-        /*PrettyPrint=*/false,
-        AccessLevel::Private,
-        /*EmitSynthesizedMembers=*/false,
-        /*PrintMessages=*/false,
-        /*SkipInheritedDocs=*/false,
-        /*IncludeSPISymbols=*/true,
-        /*IncludeClangDocs=*/true,
-        /*EmitExtensionBlockSymbols=*/false,
-    };
+
+    symbolgraphgen::SymbolGraphOptions Options;
+    Options.Target = Invoc.getLangOptions().Target;
+    Options.MinimumAccessLevel = AccessLevel::Private;
+    Options.IncludeSPISymbols = true;
+    Options.IncludeClangDocs = true;
 
     symbolgraphgen::printSymbolGraphForDecl(DInfo.VD, DInfo.BaseType,
                                             DInfo.InSynthesizedExtension,