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

[PLAT-5789] Fix property declaration warnings #985

Merged
merged 6 commits into from
Jan 29, 2021

Conversation

nickdowell
Copy link
Contributor

Goal

The public headers were triggering compiler warnings when imported by a compilation unit with ARC disabled.

Furthermore many of the string properties were not declared as copy potentially leading to being passed mutable values that could subsequently be changed.

Design

Following the raywenderlich.com Objective-C style guide for the property declarations;

Property attributes should be explicitly listed, and will help new programmers when reading the code. The order of properties should be storage then atomicity, which is consistent with automatically generated code when connecting UI elements from Interface Builder.

Preferred:

@property (weak, nonatomic) IBOutlet UIView *containerView;
@property (strong, nonatomic) NSString *tutorialName;

Not Preferred:

@property (nonatomic, weak) IBOutlet UIView *containerView;
@property (nonatomic) NSString *tutorialName;

Properties with mutable counterparts (e.g. NSString) should prefer copy instead of strong.
Why? Even if you declared a property as NSString somebody might pass in an instance of an NSMutableString and then change it without you noticing that.

Preferred:

@property (copy, nonatomic) NSString *tutorialName;

Not Preferred:

@property (strong, nonatomic) NSString *tutorialName;

Changeset

  • Adds an empty .m file that is compiled without ARC to expose all the warnings
  • Fixes all the warnings
  • Properties with mutable counterparts (e.g. NSString) now use copy
  • Most properties are nonatomic because they will not be written and read by different threads.

Testing

Tested warnings with Xcode 12.3

@github-actions
Copy link

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size did not change - 1,060,360 bytes

Generated by 🚫 Danger

@nickdowell nickdowell merged commit eceded7 into next Jan 29, 2021
@nickdowell nickdowell deleted the nickdowell/fix-property-declaration-warnings branch January 29, 2021 08:57
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.

2 participants