-
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
Adds opt-in rule discouraged_optional_collection #2001
Conversation
d39fa8b
to
369c07f
Compare
Codecov Report
@@ Coverage Diff @@
## master #2001 +/- ##
==========================================
+ Coverage 89.81% 89.85% +0.03%
==========================================
Files 264 266 +2
Lines 15275 15362 +87
Branches 997 1012 +15
==========================================
+ Hits 13719 13803 +84
- Misses 1537 1539 +2
- Partials 19 20 +1
Continue to review full report at Codecov.
|
500428d
to
fbffdb3
Compare
CHANGELOG.md
Outdated
|
||
#### Enhancements | ||
|
||
* Adds `discouraged_optional_collection` opt-in rule to encourages the return |
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.
s/encourages the return/encourage the use/
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.
Thanks, @jpsim!
I'll have to refactor/rewrite this rule. There are some cases that are really hard to cover using regular expression. For instance:
fun foo(param: [String: ↓[String: String]?])
vs.
fun foo(param: ↓[String: [String: String]]?)
But thanks, I'll update the changelog entry.
64aace1
to
19c01bb
Compare
afd92a5
to
a9c2a44
Compare
Hi @marcelofabri / @jpsim. Did you have a chance to look into this one? I added comments to the code to illustrate it better; for "developers of the future" that might need to interact with the code. Thanks! |
I created a gist with the String extension (w/ examples) from this rule. It can be copied and pasted on a Playground. |
f8ae195
to
21be5ba
Compare
and adds more examples.
21be5ba
to
90d6399
Compare
Hi @marcelofabri, I am sorry to bother you with this but can you take a look on this PR? I've been using it on my project at work for weeks now and it seems solid. I'd love to see it merged into SwiftLint's master. Thanks! |
Thanks @ornithocoder 💯 |
Implements #1885.
I tried to use regular expressions to match collections, but soon realized I would need recursion support -
?R
- to find and match nested collections, something Foundation doesn't offer.So I decided to re-implement the rule as an
ASTRule
, looking for balanced optional collections in functions and variables declarations.Example:
[String: [Int]?]
Example:
[String: [Int]?]?
Example:
var x = Set<Int>?
I've added similar examples to the private
String
extension used by the rule to illustrate the code.