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

Make tests compatible with Swift 5 #605

Merged
merged 1 commit into from Mar 20, 2020
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Mar 14, 2020

Newer versions of Swift have improved nil coalescing (SE-0230). Unfortunately, this breaks our code.

Consider the following:

let encrypter = TSMessage(inSignVerifyModeWithPrivateKey: privateKeyData,
                          peerPublicKey: publicKeyData)

let signedMessage = try? encrypter?.wrap(message.data(using: .utf8))
let verifiedMessage = try? encrypter?.unwrapData(signedMessage!)

let resultString = String(data: verifiedMessage!!, encoding: .utf8)

Here encrypter is TSMessage? and then we try? to convert the error into an optional when acting on an optional encrypter. In Swift 4 verifiedMessage has Data?? type but Swift 5 flattens the optional chain and returns Data? instead. We cannot unwrap!! that twice.

Update the tests to handle nil values more explicitly and avoid optional chaining with try?. This results in code that compiles correctly with both Swift 4 & 5.

P.S. Existing CocoaPods-based test project uses Swift 4 so the issue is not visible there. However, try updating the version and you'll see it.

Checklist

  • Change is covered by automated tests (Swift 4 only, Swift 5 is not tested)
  • The coding guidelines are followed
  • Public API has proper documentation

Newer versions of Swift have improved nil coalescing [1]. Unfortunately,
this breaks our code.

[1]: https://github.com/apple/swift-evolution/blob/master/proposals/0230-flatten-optional-try.md

Consider the following:

    let encrypter = TSMessage(inSignVerifyModeWithPrivateKey: privateKeyData,
                              peerPublicKey: publicKeyData)

    let signedMessage = try? encrypter?.wrap(message.data(using: .utf8))
    let verifiedMessage = try? encrypter?.unwrapData(signedMessage!)

    let resultString = String(data: verifiedMessage!!, encoding: .utf8)

Here "encrypter" is "TSMessage?" and then we "try?" to convert the error
into an optional when acting on an optional "encrypter". In Swift 4
"verifiedMessage" has "Data??" type but Swift 5 flattens the optional
chain and returns "Data?" instead. We cannot "unwrap!!" that twice.

Update the tests to handle "nil" values more explicitly and avoid
optional chaining with "try?". This results in code that compiles
correctly with both Swift 4 & 5.

P.S. Existing CocoaPods-based test project uses Swift 4 so the issue is
not visible there. However, try updating the version and you'll see it.
@ilammy ilammy added W-SwiftThemis 🔶 Wrapper: SwiftThemis, Swift API tests Themis test suite labels Mar 14, 2020
@vixentael
Copy link
Contributor

I tested on my machine and can confirm: it's working.

Thank you @ilammy. Let's merge!

@ilammy ilammy merged commit 0af35e3 into cossacklabs:master Mar 20, 2020
@ilammy ilammy deleted the swift-5-tests branch March 20, 2020 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Themis test suite W-SwiftThemis 🔶 Wrapper: SwiftThemis, Swift API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants