-
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
File Types Order & Type Contents Order #2296
Conversation
I just added configuration files to both rules, this means the only missing thing is fixing the locations in the tests. Since I have no idea why this happens, help or at least a hint is highly appreciated! |
I think it actually points to |
@marcelofabri Okay, you were right about that example which I fixed now. But that was not what was irritating me, here's a few examples which now still are a problem and I can't understand why:
It only happens when there's the |
11d424e
to
f5b1272
Compare
cb0f213
to
705f23a
Compare
I just fixed all issues and rebased this to the latest master. Ready to merge from my point of view! 🎉 |
a2a872f
to
745ee3c
Compare
Okay, I just found the issue which made the CI fail when running this with Swift 4: It's due to I hope the CI will be happy now and this can be merged. Anything else holding this off? |
Codecov Report
@@ Coverage Diff @@
## master #2296 +/- ##
=========================================
Coverage ? 90.93%
=========================================
Files ? 326
Lines ? 16851
Branches ? 0
=========================================
Hits ? 15323
Misses ? 1528
Partials ? 0
Continue to review full report at Codecov.
|
a17aa1b
to
b6b8a02
Compare
Just rebased to solve merge conflicts. |
b6b8a02
to
8df61bf
Compare
8df61bf
to
9e415c6
Compare
Just rebased this branch onto current master. |
9e415c6
to
b8ac708
Compare
b8ac708
to
c8bec9e
Compare
Source/SwiftLintFramework/Rules/Style/TypeContentsOrderRule.swift
Outdated
Show resolved
Hide resolved
b75b8b6
to
2b60fc2
Compare
I just rebased this to resolve merge conflicts. @jpsim Would you mind taking another look? I fixed your small comment a week ago, do you have any other reservations or can we merge now (once the CI has passed)? |
2b60fc2
to
085c5b8
Compare
085c5b8
to
d0bf8d8
Compare
Okay, just as I announced a month ago (and I waited way longer than 10 days), since there was no feedback whatsoever since then and due to the reasons explained here, I'm merging this without additional feedback from other contributors once CI passes. |
e25e0cf
to
d35e2a7
Compare
d35e2a7
to
b145d66
Compare
I'd like to have the following structure:
Can I reach it using your rule? Thanks in advance |
You can't differentiate between the types as of now (e.g. enums vs. protocols, both would belong to file_types_order:
order:
- supporting_type
- main_type
- extension Note that this is actually the default, so you could also just turn on the rule and it should be like this. Regarding the type contents, what you want is currently not possible as you can't differentiate between type_contents_order:
order:
- case
- [type_alias, associated_type, subtype]
- ib_outlet
- [type_property, ib_inspectable, instance_property]
- initializer
- type_method
- view_life_cycle_method
- [ib_action, other_method]
- subscript Feel free to create issues which request additional rules named something like
|
Wow, what a great source of Custom Rules, thanks @Dschee |
Thanks for all your work on this PR and really stepping up as a contributor and maintainer of this project overall @Dschee! I especially appreciate how thorough you were in adding tests and considering edges cases here.
I understand how frustrating it must be to not get feedback on your work for very long stretches of time. You did the right thing merging this. I'm sorry for not being very involved in SwiftLint on a regular basis any more 😕. SwiftLint doesn't have a single gatekeeper (I'm sorry if I ever gave that impression). There are over a dozen people with write access to this git repository and any one of those contributors should feel empowered to merge PRs, cut releases and help review other PRs/issues. I hope you'll keep contributing to this project as you've really made it so much better by your contributions so far. |
Thank you for those kind words @jpsim and also for approving my self-merge. I still feel bad about it since usually I‘m the guy stressing how important a review is, but I felt different in this case. Thank you for your understanding! I can understand that maintaining open source software - especially when there are so many users with different needs - can become hard at times. That‘s why I tried to help out when I had some time left over. Let‘s keep improving SwiftLint all together! |
Thanks for this awesome addition @Dschee! I'm getting one false positive. Files that only contain extensions are being flagged with:
|
@samrayner Could you please open an issue with that false positive so we can track it and fix it? Also, please provide a code example where this happens so we can properly reproduce it. |
@Dschee #2749 Sorry for the delay! |
great rule. How do we get this to autocorrect? |
Just dropping a line to show my appreciation over this rule! Absolutely great addition, can't wait to use it! 🚀 |
This fixes #2294 by introducing two new rules:
file_types_order
: Specifies how the types within a file should be ordered.type_contents_order
: Specifies the order of subtypes, properties, methods & more within a type.