Fix Directive argument name and value ranges on non-first line (#79)

* Remove unnecessary optional check

* Fix and add test case for single line block parse

* Add test for various combinations of directive argument location

* Initialize LineSegment with the text of only one line

Also, update test to verify emoji ranges

* Update documentation for `LineSegment.untrimmedText` to reflect its use

Also deprecate `LineSegment.lineStartIndex`
Also remove `lineStartIndex` parameter from internal `LineSegment` init

* Remove custom `==(_:_:)` that's same as synthesized implementation

* Update LICENSE year and code format

---------

Co-authored-by: David Rönnqvist <[email protected]>

Author: Kyle-Ye

Sources/Markdown/Base/DirectiveArgument.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2021-2022 Apple Inc. and the Swift project authors
+ Copyright (c) 2021-2023 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
@@ -43,11 +43,14 @@ public struct DirectiveArgumentText: Equatable {
 
     /// A segment of a line of argument text.
     public struct LineSegment: Equatable {
-        /// The segment's untrimmed text from which arguments can be parsed.
+        /// The original untrimmed text of the line, from which arguments can be parsed.
         public var untrimmedText: String
 
-        /// The index in ``untrimmedText`` where the line started.
-        public var lineStartIndex: String.Index
+        @available(*, deprecated, renamed: "untrimmedText.startIndex")
+        public var lineStartIndex: String.Index {
+            get { untrimmedText.startIndex }
+            set { }
+        }
 
         /// The index from which parsing should start.
         public var parseIndex: String.Index
@@ -64,28 +67,15 @@ public struct DirectiveArgumentText: Equatable {
         /// Create an argument line segment.
         /// - Parameters:
         ///   - untrimmedText: the segment's untrimmed text from which arguments can be parsed.
-        ///   - lineStartIndex: the index in ``text`` where the line started.
-        ///   - parseIndex: index from which parsing should start.
+        ///   - parseIndex: The index from which parsing should start.
         ///   - range: The range from which a segment was extracted from a line
         ///     of source, or `nil` if the argument text was provided by other means.
-        init(untrimmedText: String, lineStartIndex: String.Index, parseIndex: String.Index? = nil, range: SourceRange? = nil) {
+        init(untrimmedText: String, parseIndex: String.Index? = nil, range: SourceRange? = nil) {
             self.untrimmedText = untrimmedText
-            self.lineStartIndex = lineStartIndex
             self.parseIndex = parseIndex ?? untrimmedText.startIndex
             self.range = range
         }
 
-        /// Returns a Boolean value indicating whether two line segments are equal.
-        /// - Parameter lhs: a line segment to compare
-        /// - Parameter rhs: another line segment to compare
-        /// - Returns: `true` if the two segments are equal.
-        public static func ==(lhs: LineSegment, rhs: LineSegment) -> Bool {
-            return lhs.untrimmedText == rhs.untrimmedText &&
-                lhs.lineStartIndex == rhs.lineStartIndex &&
-                lhs.parseIndex == rhs.parseIndex &&
-                lhs.range == rhs.range
-        }
-
         /// Parse a quoted literal.
         ///
         /// ```
@@ -201,7 +191,8 @@ public struct DirectiveArgumentText: Equatable {
             var line = TrimmedLine(untrimmedText[...],
                                    source: range?.lowerBound.source,
                                    lineNumber: range?.lowerBound.line,
-                                   parseIndex: parseIndex)
+                                   parseIndex: parseIndex
+            )
             line.lexWhitespace()
             while !line.isEmptyOrAllWhitespace {
                 let name: TrimmedLine.Lex?
@@ -283,7 +274,7 @@ public struct DirectiveArgumentText: Equatable {
     /// from a string.
     public init<S: StringProtocol>(_ string: S) {
         let text = String(string)
-        self.segments = [LineSegment(untrimmedText: text, lineStartIndex: text.startIndex, range: nil)]
+        self.segments = [LineSegment(untrimmedText: text, range: nil)]
     }
 
     /// Create a body of argument text from a sequence of ``LineSegment`` elements.

Sources/Markdown/Block Nodes/Block Container Blocks/BlockDirective.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-2023 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
@@ -95,7 +95,6 @@ public extension BlockDirective {
                                                    omittingEmptySubsequences: false).map { lineText -> DirectiveArgumentText.LineSegment in
                                                     let untrimmedText = String(lineText)
                                                     return DirectiveArgumentText.LineSegment(untrimmedText: untrimmedText,
-                                                                                             lineStartIndex: untrimmedText.startIndex,
                                                                                              range: nil)
                                                    } ?? []
         try! self.init(.blockDirective(name: name,

Sources/Markdown/Parser/BlockDirectiveParser.swift

@@ -282,9 +282,6 @@ struct TrimmedLine {
     /// The original untrimmed text of the line.
     let untrimmedText: Substring
 
-    /// The starting parse index.
-    let startParseIndex: Substring.Index
-
     /// The current index a parser is looking at on a line.
     var parseIndex: Substring.Index
 
@@ -303,13 +300,16 @@ struct TrimmedLine {
         }
     }
 
-    /// - parameter untrimmedText: ``untrimmedText``
+    /// - Parameters:
+    ///   - untrimmedText: The original untrimmed text of the line.
+    ///   - source: The source file or resource from which the line came, or `nil` if no such file or resource can be identified.
+    ///   - lineNumber: The line number of this line in the source if known, starting with `0`.
+    ///   - parseIndex: The current index a parser is looking at on a line, or `nil` if a parser is looking at the start of the untrimmed text.
     init(_ untrimmedText: Substring, source: URL?, lineNumber: Int?, parseIndex: Substring.Index? = nil) {
         self.untrimmedText = untrimmedText
         self.source = source
         self.parseIndex = parseIndex ?? untrimmedText.startIndex
         self.lineNumber = lineNumber
-        self.startParseIndex = self.parseIndex
     }
 
     /// Return the UTF-8 source location of the parse index if the line
@@ -318,10 +318,7 @@ struct TrimmedLine {
         guard let lineNumber = lineNumber else {
             return nil
         }
-        let startIndex = (self.lineNumber ?? 1) == 1
-            ? untrimmedText.startIndex
-            : startParseIndex
-        let alreadyParsedPrefix = untrimmedText[startIndex..<parseIndex]
+        let alreadyParsedPrefix = untrimmedText[..<parseIndex]
         return .init(line: lineNumber, column: alreadyParsedPrefix.utf8.count + 1, source: source)
     }
 
@@ -718,14 +715,30 @@ private enum ParseContainer: CustomStringConvertible {
             let children = children.flatMap {
                 $0.convertToRawMarkup(ranges: &ranges, parent: self, options: options)
             }
-            return [.blockDirective(name: String(pendingBlockDirective.name),
-                                    nameLocation: pendingBlockDirective.atLocation,
-                                    argumentText: DirectiveArgumentText(segments: pendingBlockDirective.argumentsText.map {
-                                        let untrimmedText = String($0.text.base[$0.text.base.startIndex..<$0.text.endIndex])
-                                        return DirectiveArgumentText.LineSegment(untrimmedText: untrimmedText, lineStartIndex: untrimmedText.startIndex, parseIndex: $0.text.startIndex, range: $0.range)
-                                    }),
-                                    parsedRange: pendingBlockDirective.atLocation..<pendingBlockDirective.endLocation,
-                                    children)]
+            return [
+                .blockDirective(
+                    name: String(pendingBlockDirective.name),
+                    nameLocation: pendingBlockDirective.atLocation,
+                    argumentText: DirectiveArgumentText(segments: pendingBlockDirective.argumentsText.map {
+                        let base = $0.text.base
+                        let lineStartIndex: String.Index
+                        if let argumentRange = $0.range {
+                            // If the argument has a known source range, offset the column (number of UTF8 bytes) to find the start of the line.
+                            lineStartIndex = base.utf8.index($0.text.startIndex, offsetBy: 1 - argumentRange.lowerBound.column)
+                        } else if let newLineIndex = base[..<$0.text.startIndex].lastIndex(where: \.isNewline) {
+                            // Iterate backwards from the argument start index to find the the start of the line.
+                            lineStartIndex = base.utf8.index(after: newLineIndex)
+                        } else {
+                            lineStartIndex = base.startIndex
+                        }
+                        let parseIndex = base.utf8.index($0.text.startIndex, offsetBy: -base.utf8.distance(from: base.startIndex, to: lineStartIndex))
+                        let untrimmedLine = String(base[lineStartIndex ..< $0.text.endIndex])
+                        return DirectiveArgumentText.LineSegment(untrimmedText: untrimmedLine, parseIndex: parseIndex, range: $0.range)
+                    }),
+                    parsedRange: pendingBlockDirective.atLocation ..< pendingBlockDirective.endLocation,
+                    children
+                ),
+            ]
         case let .doxygenCommand(pendingDoxygenCommand, lines):
             let range = pendingDoxygenCommand.atLocation..<pendingDoxygenCommand.endLocation
             ranges.add(range)

Tests/MarkdownTests/Parsing/BlockDirectiveParserTests.swift

@@ -1043,6 +1043,178 @@ class BlockDirectiveArgumentParserTests: XCTestCase {
         """
         XCTAssertEqual(expected, documentation.debugDescription())
     }
+    
+    func testParsingDirectiveArgumentsWithWhitespaceBeforeDirective() throws {
+        struct ExpectedArgumentInfo {
+            var line: Int
+            let name: String
+            var nameRange: Range<Int>
+            let value: String
+            var valueRange: Range<Int>
+        }
+        
+        func assertDirectiveArguments(
+            _ expectedArguments: ExpectedArgumentInfo...,
+            parsing content: String,
+            file: StaticString = #file,
+            line: UInt = #line
+        ) throws {
+            func substring(with range: SourceRange) -> String {
+                let line = content.split(omittingEmptySubsequences: false, whereSeparator: \.isNewline)[range.lowerBound.line - 1]
+                let startIndex = line.utf8.index(line.utf8.startIndex, offsetBy: range.lowerBound.column - 1)
+                let endIndex = line.utf8.index(line.utf8.startIndex, offsetBy: range.upperBound.column - 1)
+                return String(line[startIndex ..< endIndex])
+            }
+            
+            let source = URL(fileURLWithPath: "/test-file-location")
+            let document = Document(parsing: content, source: source, options: .parseBlockDirectives)
+            let directive = try XCTUnwrap(document.children.compactMap({ $0 as? BlockDirective }).first, file: file, line: line)
+            let arguments = directive.argumentText.parseNameValueArguments()
+            XCTAssertEqual(arguments.count, expectedArguments.count, file: file, line: line)
+            for expectedArgument in expectedArguments {
+                let argument = try XCTUnwrap(arguments[expectedArgument.name], file: file, line: line)
+                
+                XCTAssertEqual(expectedArgument.name, argument.name, file: file, line: line)
+                XCTAssertEqual(
+                    argument.nameRange,
+                    SourceLocation(line: expectedArgument.line, column: expectedArgument.nameRange.lowerBound, source: source) ..< SourceLocation(line: expectedArgument.line, column: expectedArgument.nameRange.upperBound, source: source),
+                    file: file, 
+                    line: line
+                )
+                XCTAssertEqual(expectedArgument.name, argument.nameRange.map(substring(with:)), file: file, line: line)
+                
+                XCTAssertEqual(expectedArgument.value, argument.value, file: file, line: line)
+                XCTAssertEqual(
+                    argument.valueRange,
+                    SourceLocation(line: expectedArgument.line, column: expectedArgument.valueRange.lowerBound, source: source) ..< SourceLocation(line: expectedArgument.line, column: expectedArgument.valueRange.upperBound, source: source),
+                    file: file, 
+                    line: line
+                )
+                XCTAssertEqual(expectedArgument.value, argument.valueRange.map(substring(with:)), file: file, line: line)
+            }
+        }
+        
+        // One argument
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 1, name: "firstArgument", nameRange: 16 ..< 29, value: "firstValue", valueRange: 31 ..< 41),
+            parsing: "@DirectiveName(firstArgument: firstValue)"
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 16 ..< 29, value: "firstValue", valueRange: 31 ..< 41),
+            parsing: """
+            
+            @DirectiveName(firstArgument: firstValue)
+            """
+        )
+        
+        // Argument on single line
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 1, name: "firstArgument", nameRange: 17 ..< 30, value: "firstValue", valueRange: 31 ..< 41),
+            ExpectedArgumentInfo(line: 1, name: "secondArgument", nameRange: 44 ..< 58, value: "secondValue", valueRange: 62 ..< 73),
+            parsing: "@DirectiveName( firstArgument:firstValue ,\tsecondArgument: \t secondValue)"
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 17 ..< 30, value: "firstValue", valueRange: 31 ..< 41),
+            ExpectedArgumentInfo(line: 2, name: "secondArgument", nameRange: 44 ..< 58, value: "secondValue", valueRange: 62 ..< 73),
+            parsing: """
+
+            @DirectiveName( firstArgument:firstValue ,\tsecondArgument: \t secondValue)
+            """
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 19 ..< 32, value: "firstValue", valueRange: 33 ..< 43),
+            ExpectedArgumentInfo(line: 2, name: "secondArgument", nameRange: 46 ..< 60, value: "secondValue", valueRange: 64 ..< 75),
+            parsing: """
+
+              @DirectiveName( firstArgument:firstValue ,\tsecondArgument: \t secondValue)
+            """
+        )
+        
+        // Second argument on new line
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 1, name: "firstArgument", nameRange: 17 ..< 30, value: "firstValue", valueRange: 31 ..< 41),
+            ExpectedArgumentInfo(line: 2, name: "secondArgument", nameRange: 16 ..< 30, value: "secondValue", valueRange: 34 ..< 45),
+            parsing: """
+            @DirectiveName( firstArgument:firstValue ,
+                           secondArgument: \t secondValue)
+            """
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 17 ..< 30, value: "firstValue", valueRange: 31 ..< 41),
+            ExpectedArgumentInfo(line: 3, name: "secondArgument", nameRange: 16 ..< 30, value: "secondValue", valueRange: 34 ..< 45),
+            parsing: """
+
+            @DirectiveName( firstArgument:firstValue ,
+                           secondArgument: \t secondValue)
+            """
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 19 ..< 32, value: "firstValue", valueRange: 33 ..< 43),
+            ExpectedArgumentInfo(line: 3, name: "secondArgument", nameRange: 18 ..< 32, value: "secondValue", valueRange: 36 ..< 47),
+            parsing: """
+
+              @DirectiveName( firstArgument:firstValue ,
+                             secondArgument: \t secondValue)
+            """
+        )
+        
+        // Arguments on separate lines
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 2, name: "firstArgument", nameRange: 3 ..< 16, value: "firstValue", valueRange: 17 ..< 27),
+            ExpectedArgumentInfo(line: 3, name: "secondArgument", nameRange: 2 ..< 16, value: "secondValue", valueRange: 20 ..< 31),
+            parsing: """
+            @DirectiveName(
+              firstArgument:firstValue ,
+            \tsecondArgument: \t secondValue
+            )
+            """
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 3, name: "firstArgument", nameRange: 3 ..< 16, value: "firstValue", valueRange: 17 ..< 27),
+            ExpectedArgumentInfo(line: 4, name: "secondArgument", nameRange: 2 ..< 16, value: "secondValue", valueRange: 20 ..< 31),
+            parsing: """
+            
+            @DirectiveName(
+              firstArgument:firstValue ,
+            \tsecondArgument: \t secondValue
+            )
+            """
+        )
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 3, name: "firstArgument", nameRange: 5 ..< 18, value: "firstValue", valueRange: 19 ..< 29),
+            ExpectedArgumentInfo(line: 4, name: "secondArgument", nameRange: 4 ..< 18, value: "secondValue", valueRange: 22 ..< 33),
+            parsing: """
+            
+              @DirectiveName(
+                firstArgument:firstValue ,
+              \tsecondArgument: \t secondValue
+              )
+            """
+        )
+        
+        // Content and directives with emoji
+        
+        try assertDirectiveArguments(
+            ExpectedArgumentInfo(line: 3, name: "firstArgument", nameRange: 20 ..< 33, value: "first💻Value", valueRange: 35 ..< 49),
+            ExpectedArgumentInfo(line: 3, name: "secondArgument", nameRange: 51 ..< 65, value: "secondValue", valueRange: 67 ..< 78),
+            parsing: """
+            Paragraph before with emoji: 💻
+            
+            @Directive💻Name(firstArgument: first💻Value, secondArgument: secondValue)
+            """
+        )
+    }
 
     // FIXME: swift-testing macro for specifying the relationship between a bug and a test
     // Uncomment the following code when we integrate swift-testing