Skip to content

Commit

Permalink
Add unneeded_notification_center_removal rule
Browse files Browse the repository at this point in the history
Fixes #2755
  • Loading branch information
marcelofabri committed Aug 10, 2020
1 parent 6a8f413 commit f0ae411
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
* Add `swiftlint docs` command to easily open online documentation.
[417-72KI](https://github.com/417-72KI)

* Add `unneeded_notification_center_removal` rule to warn against using
`NotificationCenter.removeObserver(self)` in `deinit` since it's not required
after iOS 9/macOS 10.11.
[Amzed](https://github.com/Amzd)
[#2755](https://github.com/realm/SwiftLint/issues/2755)

#### Bug Fixes

* Fix UnusedImportRule breaking transitive imports.
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 @@ -174,6 +174,7 @@ public let masterRuleList = RuleList(rules: [
TypeNameRule.self,
UnavailableFunctionRule.self,
UnneededBreakInSwitchRule.self,
UnneededNotificationCenterRemovalRule.self,
UnneededParenthesesInClosureArgumentRule.self,
UnownedVariableCaptureRule.self,
UntypedErrorInCatchRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import Foundation
import SourceKittenFramework

public struct UnneededNotificationCenterRemovalRule: ASTRule, ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "unneeded_notification_center_removal",
name: "Unneeded NotificationCenter Removal",
description: "Observers are automatically unregistered on dealloc (iOS 9 / macOS 10.11) so you should't call " +
"`removeObserver(self)` in the deinit.",
kind: .lint,
nonTriggeringExamples: [
Example("""
class Example {
deinit {
NotificationCenter.default.removeObserver(someOtherObserver)
}
}
"""),
Example("""
class Example {
func removeObservers() {
NotificationCenter.default.removeObserver(self)
}
}
"""),
Example("""
class Example {
deinit {
cleanup()
}
}
""")
],
triggeringExamples: [
Example("""
class Foo {
deinit {
NotificationCenter.default.removeObserver(↓self)
}
}
""")
,
Example("""
class Foo {
deinit {
NotificationCenter.default.removeObserver(↓self,
name: UITextView.textDidChangeNotification, object: nil)
}
}
""")
]
)

public func validate(file: SwiftLintFile,
kind: SwiftDeclarationKind,
dictionary: SourceKittenDictionary) -> [StyleViolation] {
guard kind == .class else { return [] }

let methodCollector = NamespaceCollector(dictionary: dictionary)
let methods = methodCollector.findAllElements(of: [.functionMethodInstance])
let deinitMethod = methods.first(where: { $0.name == "deinit" })

return deinitMethod?.dictionary.substructure.compactMap { subDict -> StyleViolation? in
guard subDict.expressionKind == .call else { return nil }

return violationRange(in: file, dictionary: subDict).map {
StyleViolation(ruleDescription: Self.description,
severity: configuration.severity,
location: Location(file: file, byteOffset: $0.location))
}
} ?? []
}

private func violationRange(in file: SwiftLintFile, dictionary: SourceKittenDictionary) -> ByteRange? {
guard
dictionary.name == "NotificationCenter.default.removeObserver",
let observerRange = firstArgumentBody(in: dictionary),
let observerName = file.stringView.substringWithByteRange(observerRange),
observerName == "self"
else { return nil }

return observerRange
}

/// observer parameter range
private func firstArgumentBody(in dictionary: SourceKittenDictionary) -> ByteRange? {
if dictionary.enclosedArguments.names == [nil, "name", "object"],
let bodyOffset = dictionary.enclosedArguments.first?.bodyOffset,
let bodyLength = dictionary.enclosedArguments.first?.bodyLength {
return ByteRange(location: bodyOffset, length: bodyLength)
} else if dictionary.enclosedArguments.isEmpty,
let bodyOffset = dictionary.bodyOffset,
let bodyLength = dictionary.bodyLength {
return ByteRange(location: bodyOffset, length: bodyLength)
} else {
return nil
}
}
}

private extension Array where Element == SourceKittenDictionary {
var names: [String?] {
return map { $0.name }
}
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@
D47079AB1DFDCF7A00027086 /* SwiftExpressionKind.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */; };
D47079AD1DFE2FA700027086 /* EmptyParametersRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079AC1DFE2FA700027086 /* EmptyParametersRule.swift */; };
D47079AF1DFE520000027086 /* VoidReturnRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47079AE1DFE520000027086 /* VoidReturnRule.swift */; };
D47421F424E14760009AE788 /* UnneededNotificationCenterRemovalRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47421F324E14760009AE788 /* UnneededNotificationCenterRemovalRule.swift */; };
D47A510E1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47A510D1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift */; };
D47A51101DB2DD4800A4CC21 /* AttributesRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */; };
D47EF4801F69E3100012C4CA /* ColonRule+FunctionCall.swift in Sources */ = {isa = PBXBuildFile; fileRef = D47EF47F1F69E3100012C4CA /* ColonRule+FunctionCall.swift */; };
Expand Down Expand Up @@ -867,6 +868,7 @@
D47079AA1DFDCF7A00027086 /* SwiftExpressionKind.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftExpressionKind.swift; sourceTree = "<group>"; };
D47079AC1DFE2FA700027086 /* EmptyParametersRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EmptyParametersRule.swift; sourceTree = "<group>"; };
D47079AE1DFE520000027086 /* VoidReturnRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VoidReturnRule.swift; sourceTree = "<group>"; };
D47421F324E14760009AE788 /* UnneededNotificationCenterRemovalRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnneededNotificationCenterRemovalRule.swift; sourceTree = "<group>"; };
D47A510D1DB29EEB00A4CC21 /* SwitchCaseOnNewlineRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwitchCaseOnNewlineRule.swift; sourceTree = "<group>"; };
D47A510F1DB2DD4800A4CC21 /* AttributesRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesRule.swift; sourceTree = "<group>"; };
D47EF47F1F69E3100012C4CA /* ColonRule+FunctionCall.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ColonRule+FunctionCall.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1238,6 +1240,7 @@
D450D1D021EC4A6900E60010 /* StrongIBOutletRule.swift */,
D40E041B1F46E3B30043BC4E /* SuperfluousDisableCommandRule.swift */,
E88DEA811B0990A700A66CB0 /* TodoRule.swift */,
D47421F324E14760009AE788 /* UnneededNotificationCenterRemovalRule.swift */,
7565E5F02262BA0900B0597C /* UnusedCaptureListRule.swift */,
D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */,
D4D7320C21E15ED4001C07D9 /* UnusedControlFlowLabelRule.swift */,
Expand Down Expand Up @@ -2192,6 +2195,7 @@
D4B022981E102EE8007E5297 /* ObjectLiteralRule.swift in Sources */,
2E336D1B1DF08BFB00CCFE77 /* EmojiReporter.swift in Sources */,
E8EA41171C2D1DBE004F9930 /* CheckstyleReporter.swift in Sources */,
D47421F424E14760009AE788 /* UnneededNotificationCenterRemovalRule.swift in Sources */,
006ECFC41C44E99E00EF6364 /* LegacyConstantRule.swift in Sources */,
82FE254120F604CB00295958 /* VerticalWhitespaceClosingBracesRule.swift in Sources */,
429644B61FB0A9B400D75128 /* SortedFirstLastRule.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,12 @@ extension UnneededBreakInSwitchRuleTests {
]
}

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

extension UnneededParenthesesInClosureArgumentRuleTests {
static var allTests: [(String, (UnneededParenthesesInClosureArgumentRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1896,6 +1902,7 @@ XCTMain([
testCase(TypeNameRuleTests.allTests),
testCase(UnavailableFunctionRuleTests.allTests),
testCase(UnneededBreakInSwitchRuleTests.allTests),
testCase(UnneededNotificationCenterRemovalRuleTests.allTests),
testCase(UnneededParenthesesInClosureArgumentRuleTests.allTests),
testCase(UnownedVariableCaptureRuleTests.allTests),
testCase(UntypedErrorInCatchRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,12 @@ class UnneededBreakInSwitchRuleTests: XCTestCase {
}
}

class UnneededNotificationCenterRemovalRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(UnneededNotificationCenterRemovalRule.description)
}
}

class UnneededParenthesesInClosureArgumentRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(UnneededParenthesesInClosureArgumentRule.description)
Expand Down

0 comments on commit f0ae411

Please sign in to comment.