Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes around optionals and partial case key paths #147

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

stephencelis
Copy link
Member

@stephencelis stephencelis commented Feb 1, 2024

We recently added code that can flatten optionals in a couple of ways, but because these methods weren't disfavored, they take precedence and can produce case key paths incapable of expressing what you want, like the ability to embed an optional in an case that contains one:

@CasePathable
enum Foo {
  case bar(Int?)
}

let kp = \Foo.Cases.bar  // CaseKeyPath<Foo, Int>, not <Foo, Int?>
kp(nil)  // 'nil' is not compatible with expected argument type 'Int'

This PR disfavors these overloads to allow you to flatten optionals contextually, but by default will leave them alone.

It also fixes a bug in PartialCaseKeyPath.callAsFunction that would prevent non-optionals from being promoted to optionals in cases that expect them:

let kp: PartialCaseKeyPath<Foo> = \.bar

// Before:
kp(42)  // nil

// After:
kp(42)  // Foo.bar(.some(42))

Finally, this PR fixes a bug in which PartialCaseKeyPath.callAsFunction was technically available on CaseKeyPath, which meant you could call it with any value:

let kp = \Foo.Cases.bar

// Before:
kp("Hello")  // nil

// After:
kp("Hello")  // Cannot convert value of type 'String' to expected argument type 'Int?'

We recently added code that can flatten optionals in a couple of ways,
but because these methods weren't disfavored, they take precedent and
can produce case key paths incapable of expressing what you want, like
the ability to embed a optional in an case that contains an optional:

```swift
@CasePathable
enum Foo {
  case bar(Int?)
}

let kp = \Foo.Cases.bar  // CaseKeyPath<Foo, Int>, not <Foo, Int?>
kp(nil)  // 'nil' is not compatible with expected argument type 'Int'
```

This PR disfavors these overloads to allow you to flatten optionals
contextually, but by default will leave them alone.

It also fixes a bug in `PartialCaseKeyPath.callAsFunction` that would
prevent non-optionals from being promoted to optionals in cases that
expect them:

```swift
let kp: PartialCaseKeyPath<Foo> = \.bar
kp(42)  // nil
```

Finally, this PR fixes a bug in which
`PartialCaseKeyPath.callAsFunction` was technically available on
`CaseKeyPath`, which meant you could call it with any value.
@@ -272,7 +272,7 @@ extension CaseKeyPath {
/// A partially type-erased key path, from a concrete root enum to any resulting value type.
public typealias PartialCaseKeyPath<Root> = PartialKeyPath<Case<Root>>

extension PartialCaseKeyPath {
extension _AppendKeyPath {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same trick the standard library uses to avoid surfacing partial key path methods on non-partial key paths.

func open<AnyAssociatedValue>(_ value: AnyAssociatedValue) -> Enum? {
(Case<Enum>()[keyPath: self] as? Case<AnyAssociatedValue>)?.embed(value) as? Enum
?? (Case<Enum>()[keyPath: self] as? Case<AnyAssociatedValue?>)?.embed(value) as? Enum
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change allows non-optionals to be embedded in cases that take optionals.

@stephencelis stephencelis merged commit e072139 into main Feb 1, 2024
4 checks passed
@stephencelis stephencelis deleted the optional-chaining-bugfixes branch February 1, 2024 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants