Skip to content

Commit

Permalink
Add deinit_required rule
Browse files Browse the repository at this point in the history
Classes are required to have a deinit method.

This is a style to help debugging memory issues, when it is common to want to set a breakpoint at the point of deallocation. Most classes don't have a deinit, so the developer ends up having to quit, add a deinit and rebuild to proceed. If all classes have a deinit, debugging is much smoother.

Ref: realm#2620
  • Loading branch information
BenStaveleyTaylor authored and Tomas Camin committed Jun 17, 2019
1 parent 740e398 commit a6463d9
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 1 deletion.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@

* Make `redundant_objc_attribute` rule autocorrectable.
[Daniel Metzing](https://github.com/dirtydanee)
* Add `deinit_required` opt-in rule to ensure that all classes have a deinit
method. The purpose of this is to make memory leak debugging easier so all
classes have a place to set a breakpoint to track deallocation.
[Ben Staveley-Taylor](https://github.com/BenStaveleyTaylor)
[#2620](https://github.com/realm/SwiftLint/issues/2620)

* Add `required_deinit` opt-in rule to ensure that all classes have a deinit
method. The purpose of this is to make memory leak debugging easier so all
Expand Down
71 changes: 71 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* [Convenience Type](#convenience-type)
* [Custom Rules](#custom-rules)
* [Cyclomatic Complexity](#cyclomatic-complexity)
* [Deinit Required](#deinit-required)
* [Deployment Target](#deployment-target)
* [Discarded Notification Center Observer](#discarded-notification-center-observer)
* [Discouraged Direct Initialization](#discouraged-direct-initialization)
Expand Down Expand Up @@ -2986,6 +2987,76 @@ if true {}; if true {}; if true {}; if true {}; if true {}



## Deinit Required

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`deinit_required` | Disabled | No | style | No | 3.0.0

Classes should have an explicit deinit method.

### Examples

<details>
<summary>Non Triggering Examples</summary>

```swift
class Apple {
deinit { }
}
```

```swift
enum Banana { }
```

```swift
protocol Cherry { }
```

```swift
struct Damson { }
```

</details>
<details>
<summary>Triggering Examples</summary>

```swift
↓class Apple { }
```

```swift
↓class Banana: NSObject, Equatable { }
```

```swift
↓class Cherry {
// deinit { }
}
```

```swift
↓class Damson {
func deinitialize() { }
}
```

```swift
class Outer {
func hello() -> String { return "outer" }
deinit { }

↓class Inner {
func hello() -> String { return "inner" }
}
}
```

</details>



## Deployment Target

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public let masterRuleList = RuleList(rules: [
ConvenienceTypeRule.self,
CustomRules.self,
CyclomaticComplexityRule.self,
DeinitRequiredRule.self,
DeploymentTargetRule.self,
DiscardedNotificationCenterObserverRule.self,
DiscouragedDirectInitRule.self,
Expand Down
76 changes: 76 additions & 0 deletions Source/SwiftLintFramework/Rules/Style/DeinitRequiredRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import SourceKittenFramework

public struct DeinitRequiredRule: OptInRule, ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public static let description = RuleDescription(
identifier: "deinit_required",
name: "Deinit Required",
description: "Classes should have an explicit deinit method.",
kind: .style,
nonTriggeringExamples: [
"""
class Apple {
deinit { }
}
""",
"enum Banana { }",
"protocol Cherry { }",
"struct Damson { }"
],
triggeringExamples: [
"↓class Apple { }",
"↓class Banana: NSObject, Equatable { }",
"""
↓class Cherry {
// deinit { }
}
""",
"""
↓class Damson {
func deinitialize() { }
}
""",
"""
class Outer {
func hello() -> String { return "outer" }
deinit { }
↓class Inner {
func hello() -> String { return "inner" }
}
}
"""
]
)

public init() {}

public func validate(file: File) -> [StyleViolation] {
let classCollector = NamespaceCollector(dictionary: file.structure.dictionary)
let classes = classCollector.findAllElements(of: [.class])

let violations: [StyleViolation] = classes.compactMap { element in
guard let offset = element.dictionary.offset else {
return nil
}

let methodCollector = NamespaceCollector(dictionary: element.dictionary)
let methods = methodCollector.findAllElements(of: [.functionMethodInstance])

let containsDeinit = methods.contains {
$0.name == "deinit"
}

if containsDeinit {
return nil
}

return StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, byteOffset: offset))
}

return violations
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,13 @@
</BuildableProductRunnable>
<CommandLineArguments>
<CommandLineArgument
argument = "lint --no-cache"
argument = "lint --no-cache --config /Users/bestave/Desktop/deinit_required_test/swiftlint.yml"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "rules"
isEnabled = "NO">
</CommandLineArgument>
</CommandLineArguments>
<EnvironmentVariables>
<EnvironmentVariable
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ extension CyclomaticComplexityRuleTests {
]
}

extension DeinitRequiredRuleTests {
static var allTests: [(String, (DeinitRequiredRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension DeploymentTargetConfigurationTests {
static var allTests: [(String, (DeploymentTargetConfigurationTests) -> () throws -> Void)] = [
("testAppliesConfigurationFromDictionary", testAppliesConfigurationFromDictionary),
Expand Down Expand Up @@ -1550,6 +1556,7 @@ XCTMain([
testCase(CustomRulesTests.allTests),
testCase(CyclomaticComplexityConfigurationTests.allTests),
testCase(CyclomaticComplexityRuleTests.allTests),
testCase(DeinitRequiredRuleTests.allTests),
testCase(DeploymentTargetConfigurationTests.allTests),
testCase(DeploymentTargetRuleTests.allTests),
testCase(DisableAllTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class ConvenienceTypeRuleTests: XCTestCase {
}
}

class DeinitRequiredRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(DeinitRequiredRule.description)
}
}

class DiscardedNotificationCenterObserverRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(DiscardedNotificationCenterObserverRule.description)
Expand Down

0 comments on commit a6463d9

Please sign in to comment.