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

[3/x] Improve compile-time checks for Swift mocks #273

Merged

Conversation

andrewchang-bird
Copy link
Contributor

@andrewchang-bird andrewchang-bird commented Jan 19, 2022

Stack

๐Ÿ“š #275 [5/x] Clean up Synchronized wrapper
๐Ÿ“š #274 [4/x] Fix nested optional property and Obj-C types
๐Ÿ“š #273 โ† [3/x] Improve compile-time checks for Swift mocks
๐Ÿ“š #272 [2/x] Remove Obj-C based nil check
๐Ÿ“š #271 [1/x] Add Algolia DocSearch to the documentation

Overview

The Objective-C overloads introduced in 0.18 makes it difficult for the Swift compiler to choose the correct branch when type checking given expressions. This is exacerbated with the stubbing operator which can provide incorrect constraints on the return type, preventing implicit bridging of optionals and Foundation types.

protocol Foo {
  var nonOptional: String { get }
  var optional: String? { get }
  var bridged: NSString { get }
}
var stored: String! = "foo"
given(myMock.nonOptional) ~> stored  // Compiler fails to coerce `String?` to `String`
given(myMock.optional) ~> "foo"  // Compiler fails to coerce `String` to `String?`
given(myMock.bridged) ~> "foo"  // Compiler fails to coerce `String` to `NSString`

Additionally, closure-based stubs on Swift methods arenโ€™t protected against changes to the function signature and silently fall back to use Objective-C wildcard stubbing.

Although the issues likely wonโ€™t surface frequently, they are very big foot guns. A few non-mutually exclusive options considered:

  1. De-unify the static and dynamic stubbing APIs with given and givenDynamic. This is a source-incompatible change and makes the API less ergonomic. In addition, the dynamic stubbing path is not just used for Objective-C mocks, but also for the Swift property syntax given(myMock.property).
  2. Catch malformed Swift stubs with a more specific overload of given. Due to how the compiler picks overloads, the new function can only be annotated with a deprecated or obsoleted warning, but not unavailable error.
  3. Declare a new stubbing API givenSwift to surface compiler errors or disambiguate the type checking when using the stubbing operator.

The best options are to pair (2) and (3), with the intention that givenSwift is only used for debugging warnings emitted by (2) or in cases where itโ€™s more ergonomic to not provide explicit type annotations (e.g. for complex closure stubs).

Test Plan

  • No stubbing warnings in MockingbirdTests
  • CI tests pass

@andrewchang-bird andrewchang-bird force-pushed the dev/andrewchang-bird/remove-objc-nilcheck branch from d1cd576 to 5dea192 Compare January 19, 2022 18:54
Base automatically changed from dev/andrewchang-bird/remove-objc-nilcheck to master January 19, 2022 19:43
@andrewchang-bird andrewchang-bird force-pushed the dev/andrewchang-bird/add-static-typechecking branch from 57eeb30 to f308e36 Compare January 19, 2022 19:44
@andrewchang-bird andrewchang-bird merged commit b703906 into master Jan 19, 2022
@andrewchang-bird andrewchang-bird deleted the dev/andrewchang-bird/add-static-typechecking branch January 19, 2022 20:39
andrewchang-bird added a commit that referenced this pull request Jan 19, 2022
## Overview

The Objective-C overloads introduced in 0.18 makes it difficult for the
Swift compiler to choose the correct branch when type checking `given`
expressions. This is exacerbated with the stubbing operator which can
provide incorrect constraints on the return type, preventing implicit
bridging of optionals and Foundation types.

```swift
protocol Foo {
  var nonOptional: String { get }
  var optional: String? { get }
  var bridged: NSString { get }
}
var stored: String! = "foo"
// Compiler fails to coerce `String?` to `String`
given(myMock.nonOptional) ~> stored
// Compiler fails to coerce `String` to `String?`
given(myMock.optional) ~> "foo"
// Compiler fails to coerce `String` to `NSString`
given(myMock.bridged) ~> "foo"
```

Additionally, closure-based stubs on Swift methods arenโ€™t protected
against changes to the function signature and silently fall back to use
Objective-C wildcard stubbing.

Although the issues likely wonโ€™t surface frequently, they are very big
foot guns. A few non-mutually exclusive options considered:

1. De-unify the static and dynamic stubbing APIs with `given` and
`givenDynamic`. This is a source-incompatible change and makes the API
less ergonomic. In addition, the dynamic stubbing path is not just used
for Objective-C mocks, but also for the Swift property syntax
`given(myMock.property)`.
2. Catch malformed Swift stubs with a more specific overload of `given`.
Due to how the compiler picks overloads, the new function can only be
annotated with a `deprecated` or `obsoleted` warning, but not
`unavailable` error.
3. Declare a new stubbing API `givenSwift` to surface compiler errors or
disambiguate the type checking when using the stubbing operator.

The best options are to pair (2) and (3), with the intention that
`givenSwift` is only used for debugging warnings emitted by (2) or in
cases where itโ€™s more ergonomic to not provide explicit type annotations
(e.g. for complex closure stubs).

## Test Plan

- No stubbing warnings in `MockingbirdTests`
- CI tests pass
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