Recurse through Optional when allowlisting predicate key paths

[adityasingh2400]

Summary

PredicateCodableConfiguration.allowKeyPathsForPropertiesProvided(by:recursive:) skipped recursion for any property whose declared type is Optional, so an optional sub-object's key paths were never added to the allowlist. Encoding a predicate that traversed such a property then failed with a "keypath is not in the provided allowlist" error unless the wrapped type was allowlisted by hand. This addresses #1356.

Root cause

The recursion guard cast the key path's value type directly to PredicateCodableKeyPathProviding:

if recursive, let valueType = Swift.type(of: keyPath).valueType as? any PredicateCodableKeyPathProviding.Type {
    allowKeyPathsForPropertiesProvided(by: valueType, recursive: true)
}

For a non-optional property like let address: Address, the value type is Address, the cast succeeds, and recursion proceeds. For an optional property like let address: Address?, the value type is Optional<Address>. Optional<Address> does not conform to PredicateCodableKeyPathProviding even when Address does, so the cast fails and the wrapped type's key paths are silently never allowed. That mismatch is exactly why the issue's example works after a manual allowKeyPathsForPropertiesProvided(by: Address.self) call but not with recursive: true alone.

Fix

A small helper looks through a single layer of Optional before checking for conformance. If the value type already conforms it is used directly, preserving existing behavior. If it is an Optional, the wrapped type is checked for conformance and recursed into when it conforms. The same helper is used by the matching disallow path so the two stay symmetric. Looking through the optional means an optional providing property now recurses into its wrapped type the same way a non-optional property already did.

Testing

Added recursiveProvidedPropertiesThroughOptional to PredicateCodableTests, which encodes and decodes a predicate that traverses an optional providing property under a recursive configuration and verifies the round-tripped predicate evaluates identically. The test fails before this change (encoding throws because the wrapped type's key path is not allowlisted) and passes after it.

The predicate archiving code is gated behind FOUNDATION_FRAMEWORK, so I confirmed the FoundationEssentials and FoundationEssentialsTests targets build cleanly, validated the Optional look-through logic in isolation, and ran the SwiftPM predicate test suite, which passes with no regressions.

Fixes #1356

[jmschonfeld]

This is an interesting bug that I've thought about for a bit but haven't come up with a good answer for yet. The problem is somewhat twofold:

1. It's unclear what effect this should have on the operators required to traverse the Optional. In a Predicate, you need to use the flat map operator to traverse to an optional property's value. As it stands, this would change the API to allow access to \ObjectWithOptional.child and \Object2.a etc, but it would not actually allow the flat map operator that would be required for use in a predicate like #Predicate<ObjectWithOptional> { $0.child?.a == 2 }. To me, it seems both that for some clients it may be unexpected for allowing key paths on a type to also allow an operator like flat map, while for other clients it may be unexpected that allowing all key paths on a type would not allow traversal to those key paths.
2. While Optional may be primary for your use case, it is somewhat of an arbitrary selection of many possible options here. It's also entirely possible that model properties may use dictionaries, arrays, or even other arbitrary collections of child objects. If this API recurses through Optionals, I could also see an argument that it should recurse through these two. If so, this also feeds into the point above, since traversing through a dictionary or array requires the subscript operators instead of the flat map operator.

Based on those two points, thus far the simplest answer has been that if you want to allow key paths of properties wrapped in any other type, that you must call that out yourself (and provide any operators to traverse that type yourself). Have you thought about either of these aspects and how they may impact various client expectations?

[adityasingh2400]

Thanks, both of these are fair, and they pushed me to look more closely at how the allowlists actually compose. Let me take them in turn.

On the operators point: I think the key-path allowlist and the operator allowlist are already fully decoupled, and this change keeps them that way. allowKeyPathsForPropertiesProvided only ever adds entries to the key-path allowlist. The flat map operator is a separate PredicateExpressions.OptionalFlatMap entry, and standardConfiguration already allows it (alongside NilCoalesce, ForcedUnwrap, and the subscript operators). That is exactly why the new test round-trips: the recursion adds the Object2.a key path, while the traversal operator comes from the standard config the test starts from, not from this change. So allowing a type's key paths does not implicitly enable flat map here, and the surprise you describe in the other direction does not occur either. If a client built a config without the standard operators, recursing through the optional would allow child and Object2.a but a $0.child?.a predicate would still fail to decode because the operator is not allowed. The two lists stay orthogonal, which I think is the property you want to preserve.

On Optional being arbitrary: this is the part I am genuinely unsure about, and I agree it is the real design question. My reasoning for picking Optional specifically was that it is the one wrapper where the recursion target is unambiguous and the value identity is preserved: child is either one Object2 or nil, so recursing into Object2's key paths reaches exactly the properties a child?.a predicate would name. For Array, Dictionary, or Set the wrapped relationship is one-to-many and the traversal is a subscript or a sequence operation rather than a member access, so "recurse into Element's key paths" does not map cleanly onto a key path the client would write. That asymmetry is why I scoped it to Optional rather than all collections. But I take your underlying point that this is still a selection among many, and a reasonable reader could expect either all of them or none of them.

Given that, I am happy to go whichever way you prefer. If the team's lean is that any wrapped property should be called out explicitly, then the consistent answer is to not special-case Optional and drop this, and I will close it. If looking through Optional specifically is acceptable because of the single-wrapped-type and value-identity properties above, I will keep it scoped to Optional and document that collections are intentionally not traversed. I do not have a strong stake in expanding to collections, so I would lean toward either "Optional only, documented" or "drop it," not the full collection traversal. Which direction reads as least surprising to you?