swiftlang/swift-markdown

Directive argument name and value ranges are incorrect for single-line directives

ethan-kusters opened this issue · 7 comments

Summary

Directive argument name and value ranges are incorrect for single-line directives that are written anywhere besides the first line of the document.

Details

I've reproduced this issue with the following test run on the current main branch:

func testDirectiveWithOneArgOnDifferentLines() throws {
    var source = """
        @Options(scope: page)
        """
    
    for line in 1...10 {
        let document = Document(parsing: source, options: .parseBlockDirectives)
        let directive = try XCTUnwrap(document.child(at: 0) as? BlockDirective)
        let arguments = directive.argumentText.parseNameValueArguments()
        let scopeArg = try XCTUnwrap(arguments["scope"])
        
        XCTAssertEqual("scope", scopeArg.name)
        XCTAssertEqual(
            SourceLocation(line: line, column: 10, source: nil)..<SourceLocation(line: line, column: 15, source: nil),
            scopeArg.nameRange,
            "Unexpected name range when authored at line '\(line)'"
        )
        
        XCTAssertEqual("page", scopeArg.value)
        XCTAssertEqual(
            SourceLocation(line: line, column: 17, source: nil)..<SourceLocation(line: line, column: 21, source: nil),
            scopeArg.valueRange,
            "Unexpected value range when authored at line '\(line)'"
        )
        
        source = "\n" + source
    }
}

which gives the following output:

XCTAssertEqual failed: ("Optional(Range(2:10..<2:15))") is not equal to ("Optional(Range(2:1..<2:6))") - Unexpected name range when authored at line '2'
XCTAssertEqual failed: ("Optional(Range(2:17..<2:21))") is not equal to ("Optional(Range(2:8..<2:12))") - Unexpected value range when authored at line '2'
XCTAssertEqual failed: ("Optional(Range(3:10..<3:15))") is not equal to ("Optional(Range(3:1..<3:6))") - Unexpected name range when authored at line '3'
XCTAssertEqual failed: ("Optional(Range(3:17..<3:21))") is not equal to ("Optional(Range(3:8..<3:12))") - Unexpected value range when authored at line '3'
XCTAssertEqual failed: ("Optional(Range(4:10..<4:15))") is not equal to ("Optional(Range(4:1..<4:6))") - Unexpected name range when authored at line '4'
XCTAssertEqual failed: ("Optional(Range(4:17..<4:21))") is not equal to ("Optional(Range(4:8..<4:12))") - Unexpected value range when authored at line '4'
XCTAssertEqual failed: ("Optional(Range(5:10..<5:15))") is not equal to ("Optional(Range(5:1..<5:6))") - Unexpected name range when authored at line '5'
XCTAssertEqual failed: ("Optional(Range(5:17..<5:21))") is not equal to ("Optional(Range(5:8..<5:12))") - Unexpected value range when authored at line '5'
XCTAssertEqual failed: ("Optional(Range(6:10..<6:15))") is not equal to ("Optional(Range(6:1..<6:6))") - Unexpected name range when authored at line '6'
XCTAssertEqual failed: ("Optional(Range(6:17..<6:21))") is not equal to ("Optional(Range(6:8..<6:12))") - Unexpected value range when authored at line '6'
XCTAssertEqual failed: ("Optional(Range(7:10..<7:15))") is not equal to ("Optional(Range(7:1..<7:6))") - Unexpected name range when authored at line '7'
XCTAssertEqual failed: ("Optional(Range(7:17..<7:21))") is not equal to ("Optional(Range(7:8..<7:12))") - Unexpected value range when authored at line '7'
XCTAssertEqual failed: ("Optional(Range(8:10..<8:15))") is not equal to ("Optional(Range(8:1..<8:6))") - Unexpected name range when authored at line '8'
XCTAssertEqual failed: ("Optional(Range(8:17..<8:21))") is not equal to ("Optional(Range(8:8..<8:12))") - Unexpected value range when authored at line '8'
XCTAssertEqual failed: ("Optional(Range(9:10..<9:15))") is not equal to ("Optional(Range(9:1..<9:6))") - Unexpected name range when authored at line '9'
XCTAssertEqual failed: ("Optional(Range(9:17..<9:21))") is not equal to ("Optional(Range(9:8..<9:12))") - Unexpected value range when authored at line '9'
XCTAssertEqual failed: ("Optional(Range(10:10..<10:15))") is not equal to ("Optional(Range(10:1..<10:6))") - Unexpected name range when authored at line '10'
XCTAssertEqual failed: ("Optional(Range(10:17..<10:21))") is not equal to ("Optional(Range(10:8..<10:12))") - Unexpected value range when authored at line '10'

showing that the name and value ranges are incorrect when the directive is authored beyond line 1.

Notes

This only reproduces for single-line directives and so seems related to #49.

@Kyle-Ye this looks related to #49- any idea what might be going on here?

let arguments = directive.argumentText.parseNameValueArguments()
// Not Compile since "arguements" is [DirectiveArguement]. Did you mean "arguments[0]"?
let scopeArg = try XCTUnwrap(arguments["scope"])

The code snippet seems not compile. Could you update it so that I can better reproduce the bug? @ethan-kusters

@Kyle-Ye this looks related to #49- any idea what might be going on here?

Yes, I can reproduce it. I'm digging for more info and trying to solve it.

The code snippet seems not compile. Could you update it so that I can better reproduce the bug? @ethan-kusters

@Kyle-Ye - looks like that test only works in BlockDirectiveParserTests, sorry about that! There's a fileprivate extension that adds the functionality:

fileprivate extension RandomAccessCollection where Element == DirectiveArgument {
    /// Look for an argument named `name` or log an XCTest failure.
    subscript<S: StringProtocol>(_ name: S, file: StaticString = #filePath, line: UInt = #line) -> DirectiveArgument? {
        guard let found = self.first(where: {
            $0.name == name
        }) else {
            XCTFail("Expected argument named \(name) but it was not found", file: file, line: line)
            return nil
        }
        return found
    }
}

This a new bug needs to be fixed. Even I revert the change in #50, the test case you provide still fails.

A more simplified version to reproduce:

func testDirectiveWithOneArgOnDifferentLines() throws {
    let source = """

    @Options(scope: page)
    """

    let line = 2
    let document = Document(parsing: source, options: .parseBlockDirectives)
    let directive = try XCTUnwrap(document.child(at: 0) as? BlockDirective)
    let arguments = directive.argumentText.parseNameValueArguments()
    let scopeArg = try XCTUnwrap(arguments["scope"])

    XCTAssertEqual("scope", scopeArg.name)
    XCTAssertEqual(
        SourceLocation(line: line, column: 10, source: nil) ..< SourceLocation(line: line, column: 15, source: nil),
        scopeArg.nameRange,
        "Unexpected name range when authored at line '\(line)'"
    )

    XCTAssertEqual("page", scopeArg.value)
    XCTAssertEqual(
        SourceLocation(line: line, column: 17, source: nil) ..< SourceLocation(line: line, column: 21, source: nil),
        scopeArg.valueRange,
        "Unexpected value range when authored at line '\(line)'"
    )
}

The block directive parser seems to parse properly. And you can verify it with the following code. (By testing on directive.argumentText.segments[0].range)

let source = """

@Options(scope: page)
"""

let line = 2
let document = Document(parsing: source, options: .parseBlockDirectives)
let directive = try XCTUnwrap(document.child(at: 0) as? BlockDirective)
XCTAssertEqual(
    directive.argumentText.segments[0].range,
    SourceLocation(line: line, column: 10, source: nil) ..< SourceLocation(line: line, column: 21, source: nil)
)

The root cause of the bug may be caused by the implementation of DirectiveArgumentText.parseNameValueArguments

I have added a PR to fix it. Could you help review it? @ethan-kusters