Implement part of the value(forKey) method #1711
Conversation
|
@swift-ci please test |
| let dict: NSDictionary = ["foo": "bar"] | ||
| let result = dict.value(forKey: "foo") | ||
| XCTAssert(result as? String == "bar") | ||
| } |
There was a problem hiding this comment.
Not a big issue buts it's generally better to use XCTAssertEqual() as it shows the incorrect value if the test fails.
There was a problem hiding this comment.
Will update, thanks!
|
@swift-ci please test |
|
@swift-ci please test and merge |
| @@ -33,7 +33,12 @@ open class NSDictionary : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, | |||
| } | |||
|
|
|||
| open func value(forKey key: String) -> Any? { | |||
| NSUnsupported() | |||
| if key.hasPrefix("@") { | |||
There was a problem hiding this comment.
It is part of our API, but I'm kinda conflicted bringing this in; as it is, we do not have the ability to implement the full contract of KVC in Swift, and I don't know if the presence of a method is dangerous enough to lead people to use it for its full contract.
I would probably at the very least have some sort of diagnostic (via deprecation maybe?) on usage of this method. But that's for another patch.
|
@swift-ci please test and merge |
|
Yeah we found an SDK that wouldn't run on Linux because it was using this method in the non-KVC path. So I figured it was worth fixing that as it's so easy to do. |
|
It is, but we need to be mindful of the consequences. I’m trying to figure out if there’s a way I can mark the superclass’s method as unavailable while keeping this one as available. If the SDK is open source or the provider is amenable, it may also be useful to fix the issue upstream. |
|
@DougGregor — this is a much smaller patch that still triggers the blocking issue, if working with mine is problematic. |
|
Yes we're working with upstream to move them to |
|
@swift-ci please test |
|
@swift-ci please test and merge |
In NSDictionary the
value(forKey)method is currently unsupported.This PR aims to implement part of this method based on the following documentation. That is the non-KVC path which calls out to the already implemented
object(forKey)method.I've added a unit test for the new method which tests that
value(forKey)does return the value we expect. I've also added a test for nested dictionaries.The KVC path, where the key has a
@prefix, has been left unsupported.