No Grouping Extension Rule #1789
Conversation
|
I can see a good configuration for this might be |
I think this can be a future improvement, since (in my experience) it's not common to have extensions on nested types that you own. |
|
|
||
| extension GroupingExtensionBanRule { | ||
|
|
||
| struct Element { |
There was a problem hiding this comment.
what do you think about making this a private top level declaration? This declaration shouldn't be visible for other classes anyway.
| } | ||
|
|
||
| private func findAllElements(in dictionary: [String: SourceKitRepresentable], | ||
| ofTypes types: [SwiftDeclarationKind], |
There was a problem hiding this comment.
maybe change ofTypes to of?
There was a problem hiding this comment.
Also, what do you think about changing types to Set<SwiftDeclarationKind>?
| namespace: [String] = []) -> [Element] { | ||
|
|
||
| return dictionary.substructure | ||
| .flatMap { subDict -> [Element] in |
There was a problem hiding this comment.
do you need to break line here (before .flatMap)
Fair enough. My personal opinion is that Swiftlint needs a common suffix for discouraged (banned) practices, in order to easily navigate the rules and add consistency to naming. Starting rules with a common word like "No" and "Discourage" might group those rules and hide the actual name "GroupingExtension". I will proceed with your suggestion now, but thought I shed light on the issue above for possible future enhancements. |
|
Weekend is finally here! All requested changes are done. Btw, what do you think about the |
Codecov Report
@@ Coverage Diff @@
## master #1789 +/- ##
==========================================
+ Coverage 88.17% 88.18% +<.01%
==========================================
Files 223 224 +1
Lines 11027 11068 +41
==========================================
+ Hits 9723 9760 +37
- Misses 1304 1308 +4
Continue to review full report at Codecov.
|
|
@Mazyod could you just rebase to fix the conflict? Thanks! |
|
TIL how to resolve conflicts right from within github 😅 .. I pulled and tested, now getting file length lint error. I think I should resolve that? |
|
Adding |
Yeah, we can do that by now 🤷♂️ |
|
Also, can you try rebasing to get rid of 4c7f262? 😬 |
|
Ah, I thought you guys just squash and merge. @marcelofabri is that an option? |
|
I can do that 👍 |
| @@ -133,6 +135,10 @@ class RulesTests: XCTestCase { | |||
| verifyRule(FunctionParameterCountRule.description) | |||
| } | |||
|
|
|||
| func testNoGroupingExtension() { | |||
There was a problem hiding this comment.
just one more thing: we try to keep this list sorted. Can you please move it (and run make sourcery again)? 😅
There was a problem hiding this comment.
Of course! Sorry about that.
Hello again!
This attempts to implement the opt-in rule for warning against grouping extensions (couldn't think of a better name).
Elementwhich we can use.enum,struct, &classtypes against the extension names.Fixes #1767