Rule(s) Request `unnecessary_open` and `missing_final`
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 19.7k
- Forks
- 2.3k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 11
Description
New Issue Checklist
- Updated SwiftLint to the latest version
- I searched for existing GitHub issues
New rule request
The idea of unnecessary_open is that it would warn about uses of open where it is not required, because the class, var, or function is never overridden outside its own module.
missing_final would warn whenever a class, var, or method was never overridden either inside or outside its own module, and can be safely declared as final.
Presumably these would need to be analyzer rules. Because the logic required is quite similar, it might be easier to combine them into one rule.
- Why should this rule be added?
If open and final are strictly enforced, then it becomes possible to tell from the declaration whether and where a class is ever subclassed, or a var of fun overridden, as opposed to whether they could be subclassed or overridden in theory.
final - never overridden
internal or public - overridden, but only within the same module
open - overridden outside their defining module.
Because unnecessary open's and missing finals are not currently detected, in our own codebase, we were able to tighten these up very considerably when we did this analysis by hand.
- Provide several examples of what would and wouldn't trigger violations.
Triggering Examples
For unnecessary_open:
↓open class Foo {
// Foo is never subclassed outside it's defining module
}
open class Foo {
// bar is never overridden outside it's defining module
↓open var bar: Int = 0
}
For missing_final:
↓public class Foo {
// Foo is never subclassed at all
}
public final class Foo {
↓public var bar: Int = 0
// bar is never overridden
}
Non-Triggering Examples:
For unnecessary_open:
open class Foo {
// Foo is subclassed outside its defining module
}
open class Foo {
// bar is overridden outside its defining module
open var bar: Int = 0
}
For missing_final:
public class Foo {
// Foo is subclassed within its defining module
}
public final class Foo {
public var bar: Int = 0
// bar is overridden within its defining module
}
- Should the rule be configurable, if so what parameters should be configurable?
If combined into one rule, it would probably be good to allow them to be selectively enabled.
For libraries, where it is kind of expected that you will advertise your API, but not necessarily call it yourself, this could be a pain, but you can always disable it.
- Should the rule be opt-in or enabled by default? Why?
As an analyzer rule, by definition this would be opt-in.
Other notes:
Not entirely sure how you would unit test a rule like this that requires different compilation modules.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing SwiftLint's analyzer-rule architecture and the issue's triggering and non-triggering examples for unnecessary_open and missing_final. Determine how analyzer rules could inspect overrides across module boundaries, then investigate how such cases could be unit tested given the issue's concern about separate compilation modules; done means both rules correctly distinguish the listed cases and can be selectively enabled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100