don't automatically transfer the child elements' ranges to the parent (#202)

QuietMisdreavus · web-flow · commit 7530dfe833d8 · 2024-09-24T12:37:42.000-06:00
this commit also adds methods to BasicBlockContainer and BasicInlineContainer that inherits the new element's source range from its children

rdar://136218537
diff --git a/Sources/Markdown/Base/Document.swift b/Sources/Markdown/Base/Document.swift
@@ -65,8 +65,14 @@ public extension Document {
     }
 
     /// Create a document from a sequence of block markup elements.
-    init<Children: Sequence>(_ children: Children) where Children.Element == BlockMarkup {
-        try! self.init(.document(parsedRange: nil, children.map { $0.raw.markup }))
+    init(_ children: some Sequence<BlockMarkup>) {
+        self.init(children, inheritSourceRange: false)
+    }
+
+    init(_ children: some Sequence<BlockMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = children.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.document(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: Visitation
diff --git a/Sources/Markdown/Base/RawMarkup.swift b/Sources/Markdown/Base/RawMarkup.swift
@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2021 Apple Inc. and the Swift project authors
+ Copyright (c) 2021-2024 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -196,7 +196,7 @@ final class RawMarkup: ManagedBuffer<RawMarkupHeader, RawMarkup> {
 
         let parsedRange: SourceRange?
         if preserveRange {
-            parsedRange = header.parsedRange ?? newChild.header.parsedRange
+            parsedRange = header.parsedRange
         } else {
             parsedRange = newChild.header.parsedRange
         }
@@ -366,3 +366,13 @@ fileprivate extension Sequence where Element == RawMarkup {
         return self.lazy.map { $0.subtreeCount }.reduce(0, +)
     }
 }
+
+extension BidirectionalCollection where Element == RawMarkup {
+    var parsedRange: SourceRange? {
+        if let lowerBound = first?.parsedRange?.lowerBound, let upperBound = last?.parsedRange?.upperBound {
+            return lowerBound..<upperBound
+        } else {
+            return nil
+        }
+    }
+}
diff --git a/Sources/Markdown/Block Nodes/Block Container Blocks/BlockQuote.swift b/Sources/Markdown/Block Nodes/Block Container Blocks/BlockQuote.swift
@@ -29,8 +29,14 @@ public struct BlockQuote: BlockMarkup, BasicBlockContainer {
 public extension BlockQuote {
     // MARK: BasicBlockContainer
 
-    init<Children: Sequence>(_ children: Children) where Children.Element == BlockMarkup {
-        try! self.init(.blockQuote(parsedRange: nil, children.map { $0.raw.markup }))
+    init(_ children: some Sequence<BlockMarkup>) {
+        self.init(children, inheritSourceRange: false)
+    }
+
+    init(_ children: some Sequence<BlockMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = children.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.blockQuote(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: Visitation
diff --git a/Sources/Markdown/Block Nodes/Block Container Blocks/CustomBlock.swift b/Sources/Markdown/Block Nodes/Block Container Blocks/CustomBlock.swift
@@ -29,8 +29,14 @@ public struct CustomBlock: BlockMarkup, BasicBlockContainer {
 // MARK: - Public API
 
 public extension CustomBlock {
-    init<Children: Sequence>(_ children: Children) where Children.Element == BlockMarkup {
-        try! self.init(.customBlock(parsedRange: nil, children.map { $0.raw.markup }))
+    init(_ children: some Sequence<BlockMarkup>) {
+        self.init(children, inheritSourceRange: false)
+    }
+
+    init(_ children: some Sequence<BlockMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = children.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.customBlock(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: Visitation
diff --git a/Sources/Markdown/Block Nodes/Inline Container Blocks/Paragraph.swift b/Sources/Markdown/Block Nodes/Inline Container Blocks/Paragraph.swift
@@ -29,8 +29,14 @@ public struct Paragraph: BlockMarkup, BasicInlineContainer {
 public extension Paragraph {
     // MARK: InlineContainer
 
-    init<Children: Sequence>(_ newChildren: Children) where Children.Element == InlineMarkup {
-        try! self.init(.paragraph(parsedRange: nil, newChildren.map { $0.raw.markup }))
+    init(_ newChildren: some Sequence<InlineMarkup>) {
+        self.init(newChildren, inheritSourceRange: false)
+    }
+
+    init(_ newChildren: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = newChildren.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.paragraph(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: Visitation
diff --git a/Sources/Markdown/Block Nodes/Tables/TableCell.swift b/Sources/Markdown/Block Nodes/Tables/TableCell.swift
@@ -66,12 +66,22 @@ public extension Table.Cell {
 
     // MARK: BasicInlineContainer
 
-    init<Children>(_ children: Children) where Children : Sequence, Children.Element == InlineMarkup {
+    init(_ children: some Sequence<InlineMarkup>) {
         self.init(colspan: 1, rowspan: 1, children)
     }
 
-    init<Children>(colspan: UInt, rowspan: UInt, _ children: Children) where Children : Sequence, Children.Element == InlineMarkup {
-        try! self.init(RawMarkup.tableCell(parsedRange: nil, colspan: colspan, rowspan: rowspan, children.map { $0.raw.markup }))
+    init(_ children: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        self.init(colspan: 1, rowspan: 1, children, inheritSourceRange: inheritSourceRange)
+    }
+
+    init(colspan: UInt, rowspan: UInt, _ children: some Sequence<InlineMarkup>) {
+        self.init(colspan: colspan, rowspan: rowspan, children, inheritSourceRange: false)
+    }
+
+    init(colspan: UInt, rowspan: UInt, _ children: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = children.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.tableCell(parsedRange: parsedRange, colspan: colspan, rowspan: rowspan, rawChildren))
     }
 
     // MARK: Visitation
diff --git a/Sources/Markdown/Inline Nodes/Inline Containers/Emphasis.swift b/Sources/Markdown/Inline Nodes/Inline Containers/Emphasis.swift
@@ -29,8 +29,14 @@ public struct Emphasis: RecurringInlineMarkup, BasicInlineContainer {
 public extension Emphasis {
     // MARK: BasicInlineContainer
 
-    init<Children>(_ newChildren: Children) where Children : Sequence, Children.Element == InlineMarkup {
-        try! self.init(RawMarkup.emphasis(parsedRange: nil, newChildren.map { $0.raw.markup }))
+    init(_ newChildren: some Sequence<InlineMarkup>) {
+        self.init(newChildren, inheritSourceRange: false)
+    }
+
+    init(_ newChildren: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = newChildren.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.emphasis(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: PlainTextConvertibleMarkup
diff --git a/Sources/Markdown/Inline Nodes/Inline Containers/Strikethrough.swift b/Sources/Markdown/Inline Nodes/Inline Containers/Strikethrough.swift
@@ -28,8 +28,14 @@ public struct Strikethrough: RecurringInlineMarkup, BasicInlineContainer {
 public extension Strikethrough {
     // MARK: BasicInlineContainer
 
-    init<Children>(_ newChildren: Children) where Children : Sequence, Children.Element == InlineMarkup {
-        try! self.init(.strikethrough(parsedRange: nil, newChildren.map { $0.raw.markup }))
+    init(_ newChildren: some Sequence<InlineMarkup>) {
+        self.init(newChildren, inheritSourceRange: false)
+    }
+
+    init(_ newChildren: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = newChildren.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.strikethrough(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: PlainTextConvertibleMarkup
diff --git a/Sources/Markdown/Inline Nodes/Inline Containers/Strong.swift b/Sources/Markdown/Inline Nodes/Inline Containers/Strong.swift
@@ -28,8 +28,14 @@ public struct Strong: RecurringInlineMarkup, BasicInlineContainer {
 public extension Strong {
     // MARK: BasicInlineContainer
 
-    init<Children>(_ newChildren: Children) where Children : Sequence, Children.Element == InlineMarkup {
-        try! self.init(.strong(parsedRange: nil, newChildren.map { $0.raw.markup }))
+    init(_ newChildren: some Sequence<InlineMarkup>) {
+        self.init(newChildren, inheritSourceRange: false)
+    }
+
+    init(_ newChildren: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        let rawChildren = newChildren.map { $0.raw.markup }
+        let parsedRange = inheritSourceRange ? rawChildren.parsedRange : nil
+        try! self.init(.strong(parsedRange: parsedRange, rawChildren))
     }
 
     // MARK: PlainTextConvertibleMarkup
diff --git a/Sources/Markdown/Structural Restrictions/BasicBlockContainer.swift b/Sources/Markdown/Structural Restrictions/BasicBlockContainer.swift
@@ -11,7 +11,10 @@
 /// A block element that can contain only other block elements and doesn't require any other information.
 public protocol BasicBlockContainer: BlockContainer {
     /// Create this element from a sequence of block markup elements.
-    init<Children: Sequence>(_ children: Children) where Children.Element == BlockMarkup
+    init(_ children: some Sequence<BlockMarkup>)
+
+    /// Create this element from a sequence of block markup elements, and optionally inherit the source range from those elements.
+    init(_ children: some Sequence<BlockMarkup>, inheritSourceRange: Bool)
 }
 
 // MARK: - Public API
@@ -21,4 +24,14 @@ extension BasicBlockContainer {
     public init(_ children: BlockMarkup...) {
         self.init(children)
     }
+
+    /// Create this element with a sequence of block markup elements, and optionally inherit the source range from those elements.
+    public init(_ children: BlockMarkup..., inheritSourceRange: Bool) {
+        self.init(children, inheritSourceRange: inheritSourceRange)
+    }
+
+    /// Default implementation of `init(_:inheritSourceRange:)` that discards the `inheritSourceRange` parameter.
+    public init(_ children: some Sequence<BlockMarkup>, inheritSourceRange: Bool) {
+        self.init(children)
+    }
 }
diff --git a/Sources/Markdown/Structural Restrictions/BasicInlineContainer.swift b/Sources/Markdown/Structural Restrictions/BasicInlineContainer.swift
@@ -11,12 +11,24 @@
 /// A block or inline markup element that can contain only `InlineMarkup` elements and doesn't require any other information.
 public protocol BasicInlineContainer: InlineContainer {
     /// Create this element with a sequence of inline markup elements.
-    init<Children: Sequence>(_ children: Children) where Children.Element == InlineMarkup 
+    init(_ children: some Sequence<InlineMarkup>)
+
+    /// Create this element with a sequence of inline markup elements, and optionally inherit the source range from those elements.
+    init(_ children: some Sequence<InlineMarkup>, inheritSourceRange: Bool)
 }
 
 extension BasicInlineContainer {
     /// Create this element with a sequence of inline markup elements.
     public init(_ children: InlineMarkup...) {
         self.init(children)
     }
+
+    public init(_ children: InlineMarkup..., inheritSourceRange: Bool) {
+        self.init(children, inheritSourceRange: inheritSourceRange)
+    }
+
+    /// Default implementation for `init(_:inheritSourceRange:)` that discards the `inheritSourceRange` parameter.
+    public init(_ children: some Sequence<InlineMarkup>, inheritSourceRange: Bool) {
+        self.init(children)
+    }
 }
diff --git a/Tests/MarkdownTests/Interpretive Nodes/AsideTests.swift b/Tests/MarkdownTests/Interpretive Nodes/AsideTests.swift
@@ -196,6 +196,27 @@ class AsideTests: XCTestCase {
         }
     }
 
+    /// Ensure that creating block quotes by construction doesn't trip the "loss of source information" assertion
+    /// by mistakenly gaining source information.
+    func testConstructedBlockQuoteDoesntChangeRangeSource() throws {
+        let source = "Note: This is just a paragraph."
+        let fakeFileLocation = URL(fileURLWithPath: "/path/to/some-file.md")
+        let document = Document(parsing: source, source: fakeFileLocation)
+        let paragraph = try XCTUnwrap(document.child(at: 0) as? Paragraph)
+
+        // this block quote has no source information, but its children do
+        let blockQuote = BlockQuote(paragraph)
+        let aside = try XCTUnwrap(Aside(blockQuote))
+
+        let expectedRootDump = """
+        BlockQuote
+        └─ Paragraph @/path/to/some-file.md:1:1-/path/to/some-file.md:1:32
+           └─ Text @/path/to/some-file.md:1:7-/path/to/some-file.md:1:32 "This is just a paragraph."
+        """
+
+        XCTAssertEqual(expectedRootDump, aside.content[0].root.debugDescription(options: .printSourceLocations))
+    }
+
     func assertAside(source: String, conversionStrategy: Aside.TagRequirement, expectedKind: Aside.Kind, expectedRootDump: String, file: StaticString = #file, line: UInt = #line) throws {
         let fakeFileLocation = URL(fileURLWithPath: "/path/to/some-file.md")
         let document = Document(parsing: source, source: fakeFileLocation)
diff --git a/Tests/MarkdownTests/Structural Restrictions/BasicBlockContainerTests.swift b/Tests/MarkdownTests/Structural Restrictions/BasicBlockContainerTests.swift
@@ -110,4 +110,22 @@ final class BasicBlockContainerTests: XCTestCase {
             """
         XCTAssertEqual(expectedDump, newDocument.debugDescription())
     }
+
+    func testInheritSourceRange() throws {
+        let source = "Note: This is just a paragraph."
+        let fakeFileLocation = URL(fileURLWithPath: "/path/to/some-file.md")
+        let document = Document(parsing: source, source: fakeFileLocation)
+        let paragraph = try XCTUnwrap(document.child(at: 0) as? Paragraph)
+
+        // this block quote has no source information, but its children do
+        let blockQuote = BlockQuote(paragraph, inheritSourceRange: true)
+
+        let expectedRootDump = """
+        BlockQuote @/path/to/some-file.md:1:1-/path/to/some-file.md:1:32
+        └─ Paragraph @/path/to/some-file.md:1:1-/path/to/some-file.md:1:32
+           └─ Text @/path/to/some-file.md:1:1-/path/to/some-file.md:1:32 "Note: This is just a paragraph."
+        """
+
+        XCTAssertEqual(expectedRootDump, blockQuote.debugDescription(options: .printSourceLocations))
+    }
 }
