Add override_in_extension rule #1888
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1888 +/- ##
==========================================
+ Coverage 88.76% 88.78% +0.02%
==========================================
Files 238 240 +2
Lines 11723 11748 +25
==========================================
+ Hits 10406 10431 +25
Misses 1317 1317
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@marcelofabri Nice PR! Got a nit with the name and this other thing:
I saw that in the linked issue you were already wondering whether this should be opt-in or not.
With people often using extensions to organize their code (e.g. a UITableViewDataSource section for a UITableViewController - can also be seen in osscheck) we should either somehow detect this situation (i.e. file also contains class) or make this opt-in.
CHANGELOG.md
Outdated
| @@ -10,7 +10,10 @@ | |||
|
|
|||
| ##### Enhancements | |||
|
|
|||
| * None. | |||
| * Add `override_in_extension` rule that warns against overriding declarations | |||
| in a `extension`. | |||
| public init() {} | ||
|
|
||
| public static let description = RuleDescription( | ||
| identifier: "override_in_extension", |
There was a problem hiding this comment.
This sounds assertive (as in do this), instead of restrictive. "disallow_override_in_extensions" maybe?
There was a problem hiding this comment.
I'm not sure we have a super consistent naming convention. We already have force_try, force_cast, fallthrough and others that aren't restrictive.
I was thinking about this, but I was afraid of this situation: class Foo: NSObject {
override var description: String {
return "1"
}
}
extension Foo {
override var description: String {
return "2"
}
}However, the compiler already catches this:
So I'll work on this later 👍 |
If someone wants to audit SwiftLint's naming for related things (flags, rule names, configuration names, config params, etc), I'd be very happy to review. If we settle on a convention, we could also enforce this to some extent with unit tests. Semi-related example: every description string should read like a sentence and be terminated by a period, which we could confirm with tests. |
Fixes #1884
Let's see how many violations
oss-checkreports.