[Regression] preferKeyPath option introduces compile time issue
Hello @nicklockwood ,
We have noticed the following transformation as result of the preferKeyPath option being activated:
where the compiler error presented is: Key path value type 'Game' cannot be converted to contextual type 'Game?'
Other replacements worked fine, but a couple of them gave us this error and for now we are disabling the rule. Is this a known issue?
Kind Regards,
Goffredo
@Panajev yes, this is a known issue (I just added it to the README). The simple workaround in these cases is to replace compactMap with map, since there is no need to use compactMap in cases where the return value isn't Optional.
Shall I close this or is it something that you think can be fixed at some point and this is best kept open?
@Panajev I'll leave it open so others can find it. I'll try to gauge how common the problem is, and I may consider adding an option to disable the feature just for compactMap().
I don't see any simple way for me to resolve it, but I'm hoping it's something Apple will eventually fix in Swift itself.
+1
My use case is when the return type is optional, e.g.:
// Before
colors.compactMap { $0.cgColor }
// After
colors.compactMap(\.cgColor) // Error: Key path value type 'CGColor' cannot be converted to contextual type 'CGColor?'
// Before
urls.compactMap { $0.lastPathComponent }
// After
urls.compactMap(\.lastPathComponent) // Error: Key path value type 'String' cannot be converted to contextual type 'String?'
Perhaps the option to be on the safe side and disable the feature just for compactMap isn't too bad 🤷♂️ The workaround for us is to either sprinkle // swiftformat:disable:next preferKeyPath throughout the codebase, or disable the rule completely
I also hope Apple fixes this in Swift itself though 😣 The tests above are on Swift 5.8 (just for the record)
@rogerluan in the case above your best workaround would be to replace compactMap with map, since the use of compactMap there is redundant (the returned value isn't optional).
You're totally right @nicklockwood , I apologize, as I thought the properties were optional 😮 but they're not, in both cases. So changing from compactMap to map worked (and, I must add that IMO the previous use of compactMap was wrong). Thanks for pointing it out! 🎉
FYI something like the below can also cause problems...
public protocol Model {
var text: String { get }
}
enum Example {
func modelPublisher() -> AnyPublisher<Model, Error> {
...
}
func textPublisher() -> AnyPublisher<String?, Error> {
modelPublisher()
.map(\.text) // ERROR: Key path value type 'String' cannot be converted to contextual type 'String?'
.eraseToAnyPublisher()
}
}
This can be fixed by adjusting the signature of textPublisher to return AnyPublisher<String, Error> but sometimes it might not be possible to do that for example if the method signature is defined in a protocol and other concrete instances have a valid reason to emit a nil.