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

Add nullability & generic type annotations to all classes #90

Merged
merged 1 commit into from
Feb 7, 2016

Conversation

JohnSundell
Copy link
Contributor

This PR adds modern Objective-C annotations to all SPTDataLoader classes, both private and public.

Changes include:

  • Adding NS_ASSUME_NONNULL_BEGIN/END compiler instructions to all files.
  • Where objects are allowed to be nil, add the nullable annotation.
  • Missing nil-checks and corresponding _Nonnull casts added.
  • Some missing assignments to required variables added in tests.
  • Where changes required to fully conform to the nullability annotations were considered too big for this commit, or where whether passing nil would cause a crash was tested, #pragma warning silencers were added.
  • Generic annotations were added to all collections, both public & private.

Note: Whether a parameter should be nullable or not was determined by inspecting the surrounding code and the tests.

This change heavily improves SPTDataLoader’s usability in Swift, since arguments that before were typed as [AnyObject] now get full typing. Nullable properties and arguments are now also correctly treated as optionals in Swift.

This change also has the added benefit of adding more strong typing and safety to the implementation.

This commit adds modern Objective-C annotations to all SPTDataLoader
classes, both private and public.

Changes include:
- Adding NS_ASSUME_NONNULL_BEGIN/END compiler instructions to all files.
- Where objects are allowed to be `nil`, add the `nullable` annotation.
- Missing `nil`-checks and corresponding `_Nonnull` casts added.
- Some missing assignments to required variables added in tests.
- Where changes required to fully conform to the nullability annotations
were considered too big for this commit, or where whether passing nil
would cause a crash was tested, #pragma warning silencers were added.
- Generic annotations were added to all collections, both public & private.

This change heavily improves SPTDataLoader’s usability in Swift, since
arguments that before were typed as `[AnyObject]` now get full typing.
Nullable properties and arguments are now also correctly treated as
optionals in Swift.

This change also has the added benefit of adding more strong typing and
safety to the implementation.
@JohnSundell
Copy link
Contributor Author

@8W9aG @jgavris @rastersize

[self.factory requestResponseHandler:nil performRequest:request];
#pragma clang diagnostic pop
Copy link
Contributor

Choose a reason for hiding this comment

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

I like _Pragma("clang diagnostic push"), it maintains the indentation level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it too. In this case though I kind of want the warning silencers to be an eyesore, because we should ideally fix the issue and remove them. I just added them now since it was the smaller change, and I wanted to increase my confidence in the commit my not altering the tests too much. Feel free to change this to whatever code style you prefer though, for me it doesn't matter.

@jgavris
Copy link
Contributor

jgavris commented Feb 7, 2016

👍

@rastersize
Copy link
Contributor

Big like 👍

rastersize added a commit that referenced this pull request Feb 7, 2016
Add nullability & generic type annotations to all classes
@rastersize rastersize merged commit bb78b7e into spotify:master Feb 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants