@DeprecationSummary not having effect for some symbols (#982)

anferbui · web-flow · commit e54de2ea6c59 · 2024-07-11T17:07:59.000+01:00
When specifying the @DeprecationSummary directive in a symbol extension markdown file, the directive wasn't taking effect if the symbol had multiple lines of documentation comments.

This was happening because `DocumentationMarkup.deprecation` was only being populated in the abstract. This has now been updated to take an effect in the Discussion section as well.

* Make `@DeprecationSummary` have an effect in the Discussion section
* Add tests to verify `@DeprecationSummary` has effect in Discussion section
* Return early when `@DeprecationSummary` is detected. When we've parsed the `@DeprecationSummary` directive, no need to continue parsing, we can exit early.
* Use `XCTUnwrap` in DeprecationSummaryTests. `XCTUnwrap` is preferred over force-unwrapping or using `XCTFail` in tests.
* Remove `internal` keyword. It's not needed, `internal` is the default access level.

Resolves rdar://70056350.
diff --git a/Sources/SwiftDocC/Model/DocumentationMarkup.swift b/Sources/SwiftDocC/Model/DocumentationMarkup.swift
@@ -74,12 +74,17 @@ struct DocumentationMarkup {
     }
     
     /// Directives which are removed from the markdown content after being parsed.
-    private static let directivesRemovedFromContent = [
+    static let directivesRemovedFromContent = [
         Comment.directiveName,
         Metadata.directiveName,
         Options.directiveName,
         Redirect.directiveName,
     ]
+    
+    private static let allowedSectionsForDeprecationSummary = [
+        ParserSection.abstract,
+        ParserSection.discussion,
+    ]
 
     // MARK: - Parsed Data
     
@@ -144,17 +149,21 @@ struct DocumentationMarkup {
                 }
             }
             
+            // The deprecation summary directive is allowed to have an effect in multiple sections of the content.
+            if let directive = child as? BlockDirective,
+               directive.name == DeprecationSummary.directiveName,
+               Self.allowedSectionsForDeprecationSummary.contains(currentSection) {
+                deprecation = MarkupContainer(directive.children)
+                return
+            }
+            
             // Parse an abstract, if found
             if currentSection == .abstract {
                 if abstractSection == nil, let firstParagraph = child as? Paragraph {
                     abstractSection = AbstractSection(paragraph: firstParagraph)
                     return
                 } else if let directive = child as? BlockDirective {
-                    if directive.name == DeprecationSummary.directiveName {
-                        // Found deprecation notice in the abstract.
-                        deprecation = MarkupContainer(directive.children)
-                        return
-                    } else if Self.directivesRemovedFromContent.contains(directive.name) {
+                    if Self.directivesRemovedFromContent.contains(directive.name) {
                         // These directives don't affect content so they shouldn't break us out of
                         // the automatic abstract section.
                         return
diff --git a/Tests/SwiftDocCTests/Model/DocumentationMarkupTests.swift b/Tests/SwiftDocCTests/Model/DocumentationMarkupTests.swift
@@ -89,8 +89,28 @@ class DocumentationMarkupTests: XCTestCase {
             XCTAssertNil(model.abstractSection)
             XCTAssertEqual(expected, model.discussionSection?.content.map({ $0.detachedFromParent.debugDescription() }).joined(separator: "\n"))
         }
+        
+        // Directives which shouldn't break us out of the automatic abstract section.
+        for allowedDirective in DocumentationMarkup.directivesRemovedFromContent {
+            do {
+                let source = """
+                # Title
+                @\(allowedDirective)
+                My abstract __content__.
+                """
+                let expected = """
+                Text "My abstract "
+                Strong
+                └─ Text "content"
+                Text "."
+                """
+                let model = DocumentationMarkup(markup: Document(parsing: source, options: .parseBlockDirectives))
+                XCTAssertEqual(expected, model.abstractSection?.content.map({ $0.detachedFromParent.debugDescription() }).joined(separator: "\n"))
+                XCTAssertNil(model.discussionSection)
+            }
+        }
 
-        // Directives in between sections
+        // Directives in between sections should go into the discussion section.
         do {
             let source = """
             # Title
@@ -328,6 +348,48 @@ class DocumentationMarkupTests: XCTestCase {
               Deprecated!
             }
             """
+            let expected = """
+            Deprecated!
+            """
+            let model = DocumentationMarkup(markup: Document(parsing: source, options: .parseBlockDirectives))
+            XCTAssertEqual(expected, model.deprecation?.elements.map({ $0.format() }).joined(separator: "\n").trimmingCharacters(in: .whitespacesAndNewlines))
+        }
+        
+        // Deprecation in the topics
+        do {
+            let source = """
+            # Title
+            My abstract __content__.
+            
+            Discussion __content__.
+            ## Topics
+            
+            @DeprecationSummary {
+              Deprecated!
+            }
+            
+            ### Basics
+             - <doc:link>
+            """
+            let model = DocumentationMarkup(markup: Document(parsing: source, options: .parseBlockDirectives))
+            XCTAssertNil(model.deprecation)
+        }
+        
+        // Deprecation in the SeeAlso
+        do {
+            let source = """
+            # Title
+            My abstract __content__.
+            
+            Discussion __content__.
+            ## See Also
+            
+            @DeprecationSummary {
+              Deprecated!
+            }
+            
+             - <doc:link>
+            """
             let model = DocumentationMarkup(markup: Document(parsing: source, options: .parseBlockDirectives))
             XCTAssertNil(model.deprecation)
         }
diff --git a/Tests/SwiftDocCTests/Rendering/DeprecationSummaryTests.swift b/Tests/SwiftDocCTests/Rendering/DeprecationSummaryTests.swift
@@ -14,9 +14,9 @@ import XCTest
 
 class DeprecationSummaryTests: XCTestCase {
     func testDecodeDeprecatedSymbol() throws {
-        let deprecatedSymbolURL = Bundle.module.url(
+        let deprecatedSymbolURL = try XCTUnwrap(Bundle.module.url(
             forResource: "deprecated-symbol", withExtension: "json",
-            subdirectory: "Rendering Fixtures")!
+            subdirectory: "Rendering Fixtures"))
         
         let data = try Data(contentsOf: deprecatedSymbolURL)
         let symbol = try RenderNode.decode(fromJSON: data)
@@ -35,14 +35,10 @@ class DeprecationSummaryTests: XCTestCase {
         let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/SideKit/SideClass/init()", sourceLanguage: .swift))
         
         // Compile docs and verify contents
-        let symbol = node.semantic as! Symbol
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
         var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
         
-        guard let renderNode = translator.visit(symbol) as? RenderNode else {
-            XCTFail("Could not compile the node")
-            return
-        }
-        
+        let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
         XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [.text("This initializer has been deprecated.")])
     }
 
@@ -70,14 +66,10 @@ class DeprecationSummaryTests: XCTestCase {
         let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/SideKit/SideClass", sourceLanguage: .swift))
         
         // Compile docs and verify contents
-        let symbol = node.semantic as! Symbol
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
         var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
         
-        guard let renderNode = translator.visit(symbol) as? RenderNode else {
-            XCTFail("Could not compile the node")
-            return
-        }
-        
+        let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
         XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [.text("This class has been deprecated.")])
         
         // Verify that the in-abstract directive didn't make the context overflow into the discussion
@@ -98,15 +90,15 @@ class DeprecationSummaryTests: XCTestCase {
         )
 
         // Compile docs and verify contents
-        let symbol = node.semantic as! Symbol
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
         var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
 
         guard let renderNode = translator.visit(symbol) as? RenderNode else {
             XCTFail("Could not compile the node")
             return
         }
 
-        let expected: [RenderInlineContent] = [
+        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [
             .text("Use the "),
             SwiftDocC.RenderInlineContent.reference(
                 identifier: SwiftDocC.RenderReferenceIdentifier("doc://org.swift.docc.example/documentation/CoolFramework/CoolClass/coolFunc()"),
@@ -116,9 +108,7 @@ class DeprecationSummaryTests: XCTestCase {
             ),
             SwiftDocC.RenderInlineContent.text(" "),
             SwiftDocC.RenderInlineContent.text("initializer instead."),
-        ]
-
-        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, expected)
+        ])
     }
     
     func testSymbolDeprecatedSummary() throws {
@@ -132,20 +122,15 @@ class DeprecationSummaryTests: XCTestCase {
         )
         
         // Compile docs and verify contents
-        let symbol = node.semantic as! Symbol
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
         var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
         
-        guard let renderNode = translator.visit(symbol) as? RenderNode else {
-            XCTFail("Could not compile the node")
-            return
-        }
-        
+        let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
+
         // `doUncoolThings(with:)` has a blanket deprecation notice from the class, but no curated article - verify that the deprecation notice from the class still shows up on the rendered page
-        let expected: [RenderInlineContent] = [
+        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [
             .text("This class is deprecated."),
-        ]
-        
-        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, expected)
+        ])
     }
   
   func testDeprecationOverride() throws {
@@ -159,26 +144,72 @@ class DeprecationSummaryTests: XCTestCase {
       )
       
       // Compile docs and verify contents
-      let symbol = node.semantic as! Symbol
+      let symbol = try XCTUnwrap(node.semantic as? Symbol)
       var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
       
-      guard let renderNode = translator.visit(symbol) as? RenderNode else {
-          XCTFail("Could not compile the node")
-          return
-      }
-      
+      let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
+
       // `init()` has deprecation information in both the symbol graph and the documentation extension; when there are extra headings in an extension file, we need to make sure we correctly parse out the deprecation message from the extension and display that
-      let expected: [RenderInlineContent] = [
-          .text("Use the "),
-          .reference(
-              identifier: SwiftDocC.RenderReferenceIdentifier("doc://org.swift.docc.example/documentation/CoolFramework/CoolClass/init(config:cache:)"),
-              isActive: true,
-              overridingTitle: nil,
-              overridingTitleInlineContent: nil
-          ),
-          .text(" initializer instead."),
-      ]
-      
-      XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, expected)
+      XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [
+        .text("Use the "),
+        .reference(
+            identifier: SwiftDocC.RenderReferenceIdentifier("doc://org.swift.docc.example/documentation/CoolFramework/CoolClass/init(config:cache:)"),
+            isActive: true,
+            overridingTitle: nil,
+            overridingTitleInlineContent: nil
+        ),
+        .text(" initializer instead."),
+    ])
   }
+    
+    func testDeprecationSummaryInDiscussionSection() throws {
+        let (bundle, context) = try testBundleAndContext(named: "BundleWithLonelyDeprecationDirective")
+        let node = try context.entity(
+            with: ResolvedTopicReference(
+                bundleIdentifier: bundle.identifier,
+                path: "/documentation/CoolFramework/CoolClass/coolFunc()",
+                sourceLanguage: .swift
+            )
+        )
+        
+        // Compile docs and verify contents
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
+        var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
+        
+        let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
+
+        // `coolFunc()` has deprecation information in both the symbol graph and the documentation extension; the deprecation information is part of the "Overview" section of the markup but it should still be parsed as expected.
+        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [
+            .text("Use the "),
+            .reference(
+                identifier: SwiftDocC.RenderReferenceIdentifier("doc://org.swift.docc.example/documentation/CoolFramework/CoolClass/init()"),
+                isActive: true,
+                overridingTitle: nil,
+                overridingTitleInlineContent: nil
+            ),
+            .text(" initializer instead."),
+        ])
+    }
+    
+    func testDeprecationSummaryWithMultiLineCommentSymbol() throws {
+        let (bundle, context) = try testBundleAndContext(named: "BundleWithLonelyDeprecationDirective")
+        let node = try context.entity(
+            with: ResolvedTopicReference(
+                bundleIdentifier: bundle.identifier,
+                path: "/documentation/CoolFramework/CoolClass/init(config:cache:)",
+                sourceLanguage: .swift
+            )
+        )
+        
+        // Compile docs and verify contents
+        let symbol = try XCTUnwrap(node.semantic as? Symbol)
+        var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: node.reference)
+        
+        let renderNode = try XCTUnwrap(translator.visit(symbol) as? RenderNode, "Could not compile the node")
+        
+        // `init(config:cache:)` has deprecation information in both the symbol graph and the documentation extension; the symbol graph has multiple lines of documentation comments for the function, but adding deprecation information in the documentation extension should still work.
+        XCTAssertEqual(renderNode.deprecationSummary?.firstParagraph, [
+            .text("This initializer is deprecated as of version 1.0.0."),
+        ])
+    }
 }
diff --git a/Tests/SwiftDocCTests/Test Bundles/BundleWithLonelyDeprecationDirective.docc/CoolClass.coolFunc.md b/Tests/SwiftDocCTests/Test Bundles/BundleWithLonelyDeprecationDirective.docc/CoolClass.coolFunc.md
@@ -0,0 +1,13 @@
+# ``CoolFramework/CoolClass/coolFunc()``
+
+This is a very cool (yet deprecated) function.
+
+## Overview
+
+We can also deprecate anywhere in the discussion section.
+
+@DeprecationSummary {
+  Use the ``CoolClass/init()`` initializer instead.
+}
+
+<!-- Copyright (c) 2024 Apple Inc and the Swift Project authors. All Rights Reserved. -->
diff --git a/Tests/SwiftDocCTests/Test Bundles/BundleWithLonelyDeprecationDirective.docc/CoolClass.initConfigCache.md b/Tests/SwiftDocCTests/Test Bundles/BundleWithLonelyDeprecationDirective.docc/CoolClass.initConfigCache.md
@@ -0,0 +1,9 @@
+# ``CoolFramework/CoolClass/init(config:cache:)``
+
+@DeprecationSummary {
+    This initializer is deprecated as of version 1.0.0.
+}
+
+Overriding the deprecation summary of a symbol that has multiple lines of documentation comments also works!
+
+<!-- Copyright (c) 2024 Apple Inc and the Swift Project authors. All Rights Reserved. -->
