proposal: always_dispose_change_notifier

[FMorschel]

always_dispose_change_notifier

Description

Closing ChangeNotifiers prevents memory leaks and unexpected behavior.

Details

Closing ChangeNotifiers prevents memory leaks and unexpected behavior.

Kind

Groups: errors (just as close_sinks and cancel_subscriptions)

Good Examples

class B {
  ChangeNotifier _changeNotifier;
  void init(filename) {
    _changeNotifier = TextEditingController(); // OK
  }

  void dispose(filename) {
    _changeNotifier.dispose();
  }
}

void someFunctionOK() {
  ChangeNotifier _changeNotifier; // OK
  _changeNotifier.dispose();
}

Bad Examples

class A {
  ChangeNotifier _changeNotifier;
  void init(filename) {
    _changeNotifier = FocusNode(); // LINT
  }
}

void someFunction() {
  ChangeNotifier _changeNotifier; // LINT
}

Discussion

Since we already have close_sinks and cancel_subscriptions (and even always-remove-listener on dcm) this should be added for Flutter users that instantiate FocusNode or TextEditingController (both are ChangeNotifier) for example. Since the good practice is disposing it inside the current scope that should be just fine.
Mentioned here: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#be-explicit-about-dispose-and-the-object-lifecycle

Discussion checklist

• List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• If there's any prior art (e.g., in other linters), please add references here.
• If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn’t any corresponding advice, should there be?)
• If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[pq]

/fyi @polina-c @goderbauer

[polina-c]

Thanks. Instrumented ChangeNotifier for leak tracker prototype.

flutter/devtools#3951

[FMorschel]

Hi again. I was actually thinking, if there could be an abstract class from Flutter with only the dispose method, and all classes that do need disposing today could inherit it.

So this lint could only track variables that extend this type and Flutter users could use this as well on their classes with then the help of the lint to dispose. Maybe the same with close for Streams and related.

Edit

That could even help when instantiating classes with dispose inside new classes to remind the users to add a way to dispose (implement the same class) if that variable is created by the class, not when it's received with a parameter.

[polina-c]

We had discussion about it. There are issues around it: http://flutter.dev/go/introduce-disposable
Current strategy is to create macro @disposable: flutter/flutter#137435