Unexpected behaviour for optional conversion

[JaapWijnen]

Hi I'm getting some unexpected behaviour for the following parser printer logic where I'm trying to make a conversion between an optional dictionary and an empty dictionary (dictionary with values maps to dictionary with values, nil maps to empty dictionary, and the reverse) For completeness the full code of my parser printer is below. The problematic conversion is as follows:

.convert( 
    apply: { $0 ?? [:] }, // here $0 is an optional dictionary, maps to empty dictionary when nil
    unapply: { $0.isEmpty ? nil : $0 } // here $0 is a dictionary, maps to nil when empty
)

.convert creates an AnyConversion using the following initializer

swift-parsing/Sources/Parsing/Conversions/AnyConversion.swift

Lines 135 to 149 in c5adbb6

	public init(
	apply: @escaping (Input) -> Output?,
	unapply: @escaping (Output) -> Input?
	) {
	self._apply = {
	guard let value = apply($0)
	else { throw ConvertingError() }
	return value
	}
	self._unapply = {
	guard let value = unapply($0)
	else { throw ConvertingError() }
	return value
	}
	}

Here unapply will fail for an empty dictionary since it will return nil (by design) but then throws a ConvertingError() instead of returning nil.

The nil value is needed to be able to also walk to Optionally path when printing in my full example below

let quotedStringParser = ParsePrint {
    "\"".utf8
    PrefixUpTo("\"".utf8).map(.string)
    "\"".utf8
}

let attributeParser = ParsePrint {
    PrefixUpTo("=".utf8).map(.string)
    "=".utf8
    quotedStringParser
}

let attributesParser = Many {
    attributeParser
} separator: {
    Whitespace(1..., .horizontal)
}.map(
    .convert(
        apply: { attributes in
            attributes.reduce(into: [:]) { $0[$1.0] = $1.1 }
        }, unapply: { dictionary in
            dictionary.map { $0 }
        }
    )
)

let tagNameParser = ParsePrint {
    From<Conversions.UTF8ViewToSubstring, Prefix<Substring>>(.substring) {
        Prefix { $0.isLetter }
    }.map(.string)
}

let tagHeadParser = ParsePrint {
    tagNameParser
    Optionally {
        Whitespace(1..., .horizontal)
        attributesParser
    }.map(
        .convert( // Here's the problematic conversion
            apply: { $0 ?? [:] },
            unapply: { $0.isEmpty ? nil : $0 }
        )
    )
    Whitespace(.horizontal)
}

[stephencelis]

Hi @JaapWijnen! I think this is more of a quirk of optional promotion in the language, and less of a bug in Parsing. You're using a convenience version of .convert that is explicitly designed to use optionality to fail, which means you're dealing with a double-optional, which can behave in confusing ways.

There are a few ways to get things working again.

1. The quickest would be to make things explicit, probably easiest with Optional.some and .none instead of nil:

   -unapply: { $0.isEmpty ? nil : $0 }
   +unapply: { $0.isEmpty ? .some(.none) : $0 }

2. But really we would recommend avoiding .convert, as it uses erasure and is slow. Instead, you could define a custom Conversion by conforming to the protocol.

Assuming this isn't a bug, can I close it out or convert it to a discussion?

[JaapWijnen]

Ah! I missed that. Thanks for pointing it out. That makes a lot of sense. Yup I'll close it!