supermarin / ObjectiveSugar

ObjectiveC additions for humans. Ruby style.
MIT License
2.17k stars 190 forks source link

Add (backward compatible) generic parametrization and nullability #113

Open tomquist opened 8 years ago

tomquist commented 8 years ago

This pull request adds generics to all Array/Set/Dictionary methods and nullability modifiers in a backward compatible way.

orta commented 8 years ago

This is awesome, I want this, @supermarin can we get this merged and a release made please? In #101 you said you were waiting on this for a release :dancers:

supermarin commented 8 years ago

Thank you @tomquist for the great work and @orta for pinging. My GH notifications are so noisy that I unfortunately missed the opening of this PR.

I checked out and verified this code, and the same compiler bugs are still there since time I was trying to get this achieved.

Namely, if you look at this test it should warn or fail the compilation. I did amend the test code locally and can confirm that this is not the case.

Regardless of the fact above, I'm in favor of merging this as the compiler bug (nullability validation in Objc blocks) is not ObjectiveSugar's fault and might be resolved on it's own. Until then, this is pretty useless unless I'm doing something wrong.

Before merging, @tomquist would you mind updating the test code / project settings to affect your changes and use the nullability stuff?

On the side note, I'd even be open of removing the compatibility stuff and just rolling the Generics/Nullability for 2.0.

tomquist commented 8 years ago

@supermarin Could you please further explain what nullability validation bug you mean. I enabled nullability warnings in the project and updated the test code to use the nullability stuff. However, I get warnings as expected. The test you mentioned doesn't produce a warning because I declared the return value of the map method of NSArray to be nullable. This is something we probably should discuss.

In contrast to NSArray I declared the return value for map of NSSet and NSDictionary to be nonnull because they just emit nil values and NSArray converts them to NSNull

supermarin commented 8 years ago

I think map should be nonnull in both cases, leaving it to the user to return a NSNull if they want (which I doubt anyone wants). If you change map to return a nonnull block result and still access a dictionary element (like user[@"name"]), there's no warnings whereas one would be expected.

tomquist commented 8 years ago

Maybe I didn't fully understand the issue but I get a warning for the case you explained, no issue here:

Btw. I'm happy to remove the compatibility stuff. Commit will follow

orta commented 8 years ago

:+1: ace