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 sjavora committed Mar 9, 2019
1 parent 9bae50f commit 9b982b1
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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)

* `nimble_operator` now warns about `beTrue()` and `beFalse()`
[Igor-Palaguta](https://github.com/Igor-Palaguta)
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 @@ -2901,6 +2902,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
}
}
6 changes: 5 additions & 1 deletion SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
67EB4DFA1E4CC111004E9ACD /* CyclomaticComplexityConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */; };
67EB4DFC1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB4DFB1E4CD7F5004E9ACD /* CyclomaticComplexityRuleTests.swift */; };
69F88BF71BDA38A6005E7CAE /* OpeningBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */; };
6BE79EB12204EC0700B5A2FE /* DeinitRequiredRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BE79EB02204EC0700B5A2FE /* DeinitRequiredRule.swift */; };
6C1D763221A4E69600DEF783 /* Request+DisableSourceKit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6C1D763121A4E69600DEF783 /* Request+DisableSourceKit.swift */; };
6C7045441C6ADA450003F15A /* SourceKitCrashTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6C7045431C6ADA450003F15A /* SourceKitCrashTests.swift */; };
6CB514E91C760C6900FA02C4 /* Structure+SwiftLint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */; };
Expand Down Expand Up @@ -575,6 +576,7 @@
692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OpeningBraceRule.swift; sourceTree = "<group>"; };
692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatementPositionRule.swift; sourceTree = "<group>"; };
695BE9CE1BDFD92B0071E985 /* CommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CommaRule.swift; sourceTree = "<group>"; };
6BE79EB02204EC0700B5A2FE /* DeinitRequiredRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeinitRequiredRule.swift; sourceTree = "<group>"; };
6C1D763121A4E69600DEF783 /* Request+DisableSourceKit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Request+DisableSourceKit.swift"; sourceTree = "<group>"; };
6C27B5FC2079D33F00353E17 /* Mac-XCTest.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = "Mac-XCTest.xcconfig"; sourceTree = "<group>"; };
6C7045431C6ADA450003F15A /* SourceKitCrashTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SourceKitCrashTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1099,6 +1101,7 @@
93E0C3CD1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift */,
65454F451B14D73800319A6C /* ControlStatementRule.swift */,
3B1DF0111C5148140011BCED /* CustomRules.swift */,
6BE79EB02204EC0700B5A2FE /* DeinitRequiredRule.swift */,
D4470D581EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift */,
D47079AC1DFE2FA700027086 /* EmptyParametersRule.swift */,
D47079A61DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift */,
Expand Down Expand Up @@ -1828,7 +1831,7 @@
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "if which swiftlint >/dev/null; then\n swiftlint\nelse\n echo \"SwiftLint does not exist, download from https://github.com/realm/SwiftLint\"\nfi";
shellScript = "if which swiftlint >/dev/null; then\n swiftlint\nelse\n echo \"SwiftLint does not exist, download from https://github.com/realm/SwiftLint\"\nfi\n";
};
/* End PBXShellScriptBuildPhase section */

Expand Down Expand Up @@ -1988,6 +1991,7 @@
E889D8C51F1D11A200058332 /* Configuration+LintableFiles.swift in Sources */,
094385011D5D2894009168CF /* WeakDelegateRule.swift in Sources */,
82F614F22106014500D23904 /* MultilineParametersBracketsRule.swift in Sources */,
6BE79EB12204EC0700B5A2FE /* DeinitRequiredRule.swift in Sources */,
3B1DF0121C5148140011BCED /* CustomRules.swift in Sources */,
2E5761AA1C573B83003271AF /* FunctionParameterCountRule.swift in Sources */,
E86396C91BADB2B9002C9E88 /* JSONReporter.swift in Sources */,
Expand Down
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 @@ -232,6 +232,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 @@ -1478,6 +1484,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 @@ -84,6 +84,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 9b982b1

Please sign in to comment.