-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 implicitly_unwrapped_optional
rule
#1362
Add implicitly_unwrapped_optional
rule
#1362
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1362 +/- ##
==========================================
+ Coverage 81.91% 81.91% +<.01%
==========================================
Files 176 181 +5
Lines 8924 9076 +152
==========================================
+ Hits 7310 7435 +125
- Misses 1614 1641 +27
Continue to review full report at Codecov.
|
It seems that tests failed not because of my changes... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also revert the changes that were made in the project to make it conform to the new rule?
I don't think we want to enforce it here (at least for now), as there're only a few places that use IUO and I'm not sure the proposed approaches are really better.
CHANGELOG.md
Outdated
@@ -9,6 +9,11 @@ | |||
|
|||
##### Enhancements | |||
|
|||
* Add `implicitly_unwrapped_optional` rule | |||
that warns when using implicitly unwrapped optional, | |||
except cases when this IUO is IBOutlet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requires two trailing spaces as described in CONTRIBUTING.md: https://github.com/realm/SwiftLint/blob/master/CONTRIBUTING.md#tracking-changes
guard let typeName = dictionary.typeName else { return [] } | ||
guard typeName.hasSuffix("!") else { return [] } | ||
|
||
let isOutlet = dictionary.enclosedSwiftAttributes.contains("source.decl.attribute.iboutlet") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about making this configurable? I think by default we should skip outlets, but I know some people who think that IUO shouldn't be used even on outlets.
…ering on complex types(AnyCollection<Int!> for ex.)
@marcelofabri Thanks for your feedback! Just updated the pull request. |
@marcelofabri Any updates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add the new test cases to LinuxMain.swift
, so they can be run on Linux as well.
Also, can you mention #56 on CHANGELOG since this PR fixes it?
@marcelofabri just updated the PR. |
Thanks for implementing this! 👏 |
Thanks for the PR @SiarheiFedartsou!
👍 thanks for making this point @marcelofabri! |
Add
implicitly_unwrapped_optional
rule that warns when using implicitly unwrapped optional,except cases when this IUO is IBOutlet.
Also some small fixes to conform to new rule(NSRegularExpression! substituted with NSRegularExpression?).