Swiftlint: Rule request: complete NSNotificationCenter detachment

[marcelofabri]

From http://fauxpasapp.com/rules/#rule-CompleteNotificationCenterDetachment:

---

Warns when an object removes itself as an observer to all notifications (by invoking -[NSNotificationCenter removeObserver:] with self).

This can be a problem if a superclass or a subclass wants to keep observing some notifications. The recommended way to handle this is to remove self as an observer only for the specific notifications that you have previously begun observing.

This rule does not warn about such removals occuring in -dealloc.

[mRs-]

I don't understand this warning exactly. You write

This can be a problem if a superclass or a subclass wants to keep observing some notifications.

IMHO if the dealloc of the super or the subclass is called it's pretty normal to remove the observers here, because the Object (even the sub or the super class) is gone from memory. It's enforcing the Observer to be in the notification center pointing on dead code.

[marcelofabri]

Imagine you have a View Controller. In viewDidDisappear:, you call NotificationCenter.default.removeObserver(self).

This will unregister the view controller from ALL notifications, including the ones that the superclass registered. So if UIViewController actually needs to listen for any notification, it won't get it.

That's way this is an issue. The only place it's safe to do that is in dealloc, because you know that the object won't receive any notifications after that anyway.

[mRs-]

Yeah but here it's the same principle. If my subclass is calling viewDidDisappear it will be called in my superclass as well. So this warning is completely useless.

[marcelofabri]

It's not, because viewDidDisappear from the superclass will not register for notifications again.

Check the documentation: https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc

[mRs-]

Okay now I get it. But the rule is wrong anyway. It's implemented for objective-c code and the right place to do this would be deinit and not dealloc

[mRs-]

I will create a Pull Request to fix this issue.

[marcelofabri]

It's not implemented wrong, just the description is using Objective-C in this issue. Check the examples.

[mRs-]

Sorry it was my bad the code looked like this…

deinit {
    unregisterForNotification()
}

private func unregisterForNotification() {
  NotificationCenter.default.removeObserver(self) // creates a warning of course
}

Thanks for helping out @marcelofabri

[marcelofabri]

No problem!

Unfortunately we can't (yet?) recognize this case, so this rule is pretty dumb in general (that's why it's opt-in).

[kocyigityunus]

this behavior is absolutely correct. do not remove all observers. just remove the observers you are responsible for with

open func removeObserver(_ observer: Any, name aName: NSNotification.Name?, object anObject: Any?)

method.