Merge pull request #36863 from apple/QuietMisdreavus/docs-inheritance

[SymbolGraph] Add information about "inherited docs" for synthesized symbols

Author: QuietMisdreavus

include/swift/Frontend/FrontendOptions.h

@@ -394,6 +394,10 @@ class FrontendOptions {
   ///
   /// \sa SymbolGraphASTWalker
   std::string SymbolGraphOutputDir;
+  
+  /// Whether to emit doc comment information in symbol graphs for symbols
+  /// which are inherited through classes or default implementations.
+  bool SkipInheritedDocs = false;
 
 private:
   static bool canActionEmitDependencies(ActionType);

include/swift/Option/Options.td

@@ -1214,4 +1214,10 @@ def minimum_access_level : Separate<["-"], "minimum-access-level">,
   HelpText<"Include symbols with this access level or more">,
   MetaVarName<"<level>">;
 
+def skip_inherited_docs : Flag<["-"], "skip-inherited-docs">,
+  Flags<[SwiftSymbolGraphExtractOption, FrontendOption,
+         NoInteractiveOption, SupplementaryOutput, HelpHidden]>,
+  HelpText<"Skip emitting doc comments for members inherited through classes or "
+           "default implementations">;
+
 include "FrontendOptions.td"

include/swift/Serialization/SerializationOptions.h

@@ -31,6 +31,7 @@ namespace swift {
     const char *DocOutputPath = nullptr;
     const char *SourceInfoOutputPath = nullptr;
     std::string SymbolGraphOutputDir;
+    bool SkipSymbolGraphInheritedDocs = true;
 
     StringRef GroupInfoPath;
     StringRef ImportedHeader;

include/swift/SymbolGraphGen/SymbolGraphOptions.h

@@ -39,6 +39,9 @@ struct SymbolGraphOptions {
   /// Whether to print informational messages when rendering
   /// a symbol graph.
   bool PrintMessages;
+  
+  /// Whether to skip docs for symbols with compound, "SYNTHESIZED" USRs.
+  bool SkipInheritedDocs;
 };
 
 } // end namespace symbolgraphgen

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -248,6 +248,8 @@ bool ArgsToFrontendOptionsConverter::convert(
   if (const Arg *A = Args.getLastArg(OPT_emit_symbol_graph_dir)) {
     Opts.SymbolGraphOutputDir = A->getValue();
   }
+  
+  Opts.SkipInheritedDocs = Args.hasArg(OPT_skip_inherited_docs);
 
   return false;
 }

lib/Frontend/Frontend.cpp

@@ -168,6 +168,7 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
     llvm::sys::fs::make_absolute(OutputDir);
     serializationOpts.SymbolGraphOutputDir = OutputDir.str().str();
   }
+  serializationOpts.SkipSymbolGraphInheritedDocs = opts.SkipInheritedDocs;
 
   if (!getIRGenOptions().ForceLoadSymbolName.empty())
     serializationOpts.AutolinkForceLoad = true;

lib/Serialization/Serialization.cpp

@@ -5635,6 +5635,7 @@ void swift::serialize(ModuleOrSourceFile DC,
         AccessLevel::Public,
         /*EmitSynthesizedMembers*/true,
         /*PrintMessages*/false,
+        /*EmitInheritedDocs*/options.SkipSymbolGraphInheritedDocs,
       };
       symbolgraphgen::emitSymbolGraphForModule(M, SGOpts);
     }

lib/SymbolGraphGen/Edge.cpp

@@ -57,5 +57,27 @@ void Edge::serialize(llvm::json::OStream &OS) const {
         });
       }
     }
+    
+    const ValueDecl *InheritingDecl = nullptr;
+    if (const auto *ID = Source.getDeclInheritingDocs()) {
+      if (Target.getSymbolDecl() == ID || Source.getSynthesizedBaseTypeDecl())
+        InheritingDecl = ID;
+    }
+    
+    // If our source symbol is a inheriting decl, write in information about
+    // where it's inheriting docs from.
+    if (InheritingDecl) {
+      Symbol inheritedSym(Graph, InheritingDecl, nullptr);
+      SmallString<256> USR, Display;
+      llvm::raw_svector_ostream DisplayOS(Display);
+      
+      inheritedSym.getUSR(USR);
+      inheritedSym.printPath(DisplayOS);
+      
+      OS.attributeObject("sourceOrigin", [&](){
+        OS.attribute("identifier", USR.str());
+        OS.attribute("displayName", Display.str());
+      });
+    }
   });
 }

lib/SymbolGraphGen/FormatVersion.h

@@ -15,6 +15,6 @@
 
 #define SWIFT_SYMBOLGRAPH_FORMAT_MAJOR 0
 #define SWIFT_SYMBOLGRAPH_FORMAT_MINOR 5
-#define SWIFT_SYMBOLGRAPH_FORMAT_PATCH 2
+#define SWIFT_SYMBOLGRAPH_FORMAT_PATCH 3
 
 #endif // SWIFT_SYMBOLGRAPHGEN_FORMATVERSION_H

lib/SymbolGraphGen/Symbol.cpp

@@ -172,12 +172,34 @@ void Symbol::serializeRange(size_t InitialIndentation,
   });
 }
 
-void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
+const ValueDecl *Symbol::getDeclInheritingDocs() const {
+  // get the decl that would provide docs for this symbol
   const auto *DocCommentProvidingDecl =
-      dyn_cast_or_null<ValueDecl>(
-          getDocCommentProvidingDecl(VD, /*AllowSerialized=*/true));
-  if (!DocCommentProvidingDecl) {
-    DocCommentProvidingDecl = VD;
+    dyn_cast_or_null<ValueDecl>(
+      getDocCommentProvidingDecl(VD, /*AllowSerialized=*/true));
+  
+  // if the decl is the same as the one for this symbol, we're not
+  // inheriting docs, so return null. however, if this symbol is
+  // a synthesized symbol, `VD` is actually the source symbol, and
+  // we should point to that one regardless.
+  if (DocCommentProvidingDecl == VD && !SynthesizedBaseTypeDecl) {
+    return nullptr;
+  } else {
+    // otherwise, return whatever `getDocCommentProvidingDecl` returned.
+    // it will be null if there are no decls that provide docs for this
+    // symbol.
+    return DocCommentProvidingDecl;
+  }
+}
+
+void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
+  const auto *DocCommentProvidingDecl = VD;
+  if (!Graph->Walker.Options.SkipInheritedDocs) {
+    DocCommentProvidingDecl = dyn_cast_or_null<ValueDecl>(
+      getDocCommentProvidingDecl(VD, /*AllowSerialized=*/true));
+    if (!DocCommentProvidingDecl) {
+      DocCommentProvidingDecl = VD;
+    }
   }
   auto RC = DocCommentProvidingDecl->getRawComment(/*SerializedOK=*/true);
   if (RC.isEmpty()) {

lib/SymbolGraphGen/Symbol.h

@@ -111,6 +111,11 @@ class Symbol {
   void printPath(llvm::raw_ostream &OS) const;
 
   void getUSR(SmallVectorImpl<char> &USR) const;
+  
+  /// If this symbol is inheriting docs from a parent class, protocol, or default
+  /// implementation, returns that decl. Returns null if there are no docs or if
+  /// the symbol has its own doc comments to render.
+  const ValueDecl *getDeclInheritingDocs() const;
 
   static bool supportsKind(DeclKind Kind);
 };

test/SymbolGraph/Relationships/Synthesized/InheritedDocs.swift

@@ -0,0 +1,63 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %s -module-name InheritedDocs -emit-module -emit-module-path %t/InheritedDocs.swiftmodule -emit-module-source-info-path %t/InheritedDocs.swiftsourceinfo -emit-module-doc-path %t/InheritedDocs.swiftdoc
+
+// RUN: %target-swift-symbolgraph-extract -module-name InheritedDocs -I %t -pretty-print -output-dir %t
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes CHECK,DOCS
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes IMPL
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes BONUS
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes BONUS-DOCS
+
+// RUN: %target-swift-symbolgraph-extract -module-name InheritedDocs -I %t -pretty-print -output-dir %t -skip-inherited-docs
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes CHECK,SKIP
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes IMPL
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes BONUS
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes BONUS-SKIP
+
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -module-name InheritedDocs -emit-module -emit-module-path %t/InheritedDocs.swiftmodule -emit-symbol-graph -emit-symbol-graph-dir %t/ -skip-inherited-docs
+// RUN: %FileCheck %s --input-file %t/InheritedDocs.symbols.json --check-prefixes SKIP
+
+// DOCS-COUNT-3: Some Function
+// BONUS-DOCS-COUNT-2: Bonus docs!
+// SKIP-COUNT-1: Some Function
+// BONUS-SKIP-COUNT-1: Bonus docs!
+
+// synthesized symbols should have a sourceOrigin field that points to where its docs come from
+
+// CHECK:            "source": "s:13InheritedDocs1PPAAE8someFuncyyF::SYNTHESIZED::s:13InheritedDocs1SV"
+// CHECK-NEXT:       "target": "s:13InheritedDocs1SV"
+// CHECK-NEXT:       "sourceOrigin"
+// CHECK-NEXT:         "identifier": "s:13InheritedDocs1PP8someFuncyyF"
+// CHECK-NEXT:         "displayName": "P.someFunc()"
+
+// non-synthesized symbols that nonetheless inherit docs (like this extension) should have the same
+
+// IMPL:             "source": "s:13InheritedDocs1PPAAE8someFuncyyF"
+// IMPL-NEXT:        "target": "s:13InheritedDocs1PP8someFuncyyF"
+// IMPL-NEXT:        "sourceOrigin"
+// IMPL-NEXT:          "identifier": "s:13InheritedDocs1PP8someFuncyyF"
+// IMPL-NEXT:          "displayName": "P.someFunc()"
+
+// synthesized symbols that point directly to their docs should also have a sourceOrigin field
+
+// BONUS:            "source": "s:13InheritedDocs1PPAAE9bonusFuncyyF::SYNTHESIZED::s:13InheritedDocs1SV"
+// BONUS-NEXT:       "target": "s:13InheritedDocs1SV"
+// BONUS-NEXT:       "sourceOrigin"
+// BONUS-NEXT:         "identifier": "s:13InheritedDocs1PPAAE9bonusFuncyyF"
+// BONUS-NEXT:         "displayName": "P.bonusFunc()"
+
+/// Protocol P
+public protocol P {
+    /// Some Function
+    func someFunc()
+}
+
+public extension P {
+    func someFunc() {}
+
+    /// Bonus docs!
+    func bonusFunc() {}
+}
+
+public struct S: P {
+}

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -870,6 +870,7 @@ fillSymbolInfo(CursorSymbolInfo &Symbol, const DeclInfo &DInfo,
         AccessLevel::Private,
         /*EmitSynthesizedMembers*/ false,
         /*PrintMessages*/ false,
+        /*SkipInheritedDocs*/ false,
     };
 
     symbolgraphgen::printSymbolGraphForDecl(DInfo.VD, DInfo.BaseType,

tools/driver/swift_symbolgraph_extract_main.cpp

@@ -167,6 +167,7 @@ int swift_symbolgraph_extract_main(ArrayRef<const char *> Args,
       AccessLevel::Public,
       !ParsedArgs.hasArg(OPT_skip_synthesized_members),
       ParsedArgs.hasArg(OPT_v),
+      ParsedArgs.hasArg(OPT_skip_inherited_docs),
   };
 
   if (auto *A = ParsedArgs.getLastArg(OPT_minimum_access_level)) {