Don't let a field shadow a super-class field

[mleonhard]

Background

I am developing a mobile app with Flutter. Flutter programs include many classes which inherit from Widget. Widget contains a key field. User code rarely uses this field directly. Instead, we pass the value to the super constructor. The Flutter libraries consume the value.

It is standard practice to write a custom widget that wraps a core widget. Sometimes the custom widget constructor takes a key argument to pass to the core widget's constructor. The custom widget must save the key value and later pass it to the core widget's constructor. If the custom widget stores the key in a field called key, then it inadvertently assigns the value to its own super-class Widget.key field. The app ends up with two widgets having the same key. This causes problems.

I made this error and wasted some hours discovering it. I was disappointed to learn that Dart allows a class field to shadow a super class field. Android Studio does not add a warning to the field.

Simplified example:

class Widget {
  Widget({this.key});
  int? key;

  int? getKey() => this.key;
}

class Core extends Widget {
  Core({int? key}) : super(key: key);
}

class Custom extends Widget {
  Custom({this.key});
  int? key;

  Widget build() => Core(key: this.key);
}

void main() {
  final Widget custom = Custom(key: 5);
  assert(custom.getKey() == null); // Fails because getKey() returns 5.
}

Proposal

Let's help users avoid accidentally assigning a value to a super-class field. To do that, we stop compilation with an error when a class field has the same name as a super-class field. Alternatively, we could configure popular Dart development tools to show a warning on the offending field.

[mleonhard]

EDIT: Clarified example.

[Levi-Lesches]

I may be wrong, but it seems here that your problem is creating a new widget (Core) with the same key as the parent widget (Custom). I'm basing this on

The app ends up with two widgets having the same key. This causes problems.

I don't see how shadowing the inherited field causes the issue, and you can shadow and still be perfectly fine, so long as you don't continue using the same key in children widgets.

In any case, you should already get warnings. DartPad won't show it, but your code does show some lints, assuming you didn't disable them:

class Widget {
  Widget({this.key});
  int? key;

  int? getKey() => key;
}

class Core extends Widget {
  Core({int? key}) : super(key: key);
}

class Custom extends Widget {
  Custom({this.key});

  // INFO: Don't override fields
  // INFO: Annotate overriden members
  int? key;  

  Widget build() => Core(key: key);
}

void main() {
  final Widget custom = Custom(key: 5);
  assert(custom.getKey() == null); // Fails because getKey() returns 5.
}

[eernstg]

The lint overridden_fields might be helpful here. [Edit: Apparently, you'd have that already? Anyway, it might still be helpful.]

Note, however, that it may be perfectly appropriate to override the getter of a field (say, in order to log evaluations of the field, or perhaps reset the value of the field if it has "read-only-once" semantics) or the setter (say, in order to validate new values assigned to it), but in this case it is understood that the overriding declarations of a field x will invoke super.x to read or write the overridden field. If super.x is not used then the overridden field is just a waste of space, and that's very unlikely to be a good design.

The existing lints won't help you make this distinction, I think, but overridden_fields plus a few // ignore:overridden_fields may work quite well.

[mleonhard]

I followed the instructions for setting up Android Studio for Flutter development: https://flutter.dev/docs/development/tools/android-studio. I'm using the latest default install of Android Studio (2020.3.1 Patch 2), with latest IntelliJ Dart plugin (203.8292), and latest IntelliJ Flutter plugin (60.1.2), on macOS 11.5.1. I see no errors, warnings, or lints.

screenshot


It looks like I need to enable lints per-project: https://dart.googlesource.com/lints/ . There are separate lints for Flutter https://dart.dev/tools/linter-rules that also require extra per-project steps to set up. It requires adding boilerplate to pubspec.yaml and creating a new analysis_options.yaml file with boilerplate. Boilerplate clutters up the code. Busy-work wastes our time.

I followed the instructions to enable both lints and flutter_lints. The example still shows no error.

screenshot of strange error in analysis_options.yaml


screenshot of example code showing no lint warnings


It looks like the instructions for setting up flutter_lint are broken. After commenting out flutter_lint, we get a lint warning on the shadowed field:

screenshot of example code showing lint warning


For the majority of users, when lints are not enabled by default, they essentially do not exist.

There are several potential fixes:

1. Dart Team to make field shadowing a compiler error.
2. Flutter Team to make flutter create add the necessary boilerplate to enable the linter.
3. Flutter Team to update the Android Studio setup instructions to explain how to add the lint-enabling boilerplate.
4. Flutter Team to fix the flutter_lint setup instructions.
5. JetBrains to make the IntelliJ Dart plugin to use these lint rules by default.

I don't know what the best solution is. Maybe Flutter's PM Tim Sneath @timsneath would know?

[Levi-Lesches]

Flutter Team to make flutter create add the necessary boilerplate to enable the linter.

As per this doc:

The package:flutter_lints defines the latest set of recommended lints that encourage good coding practices for Flutter apps, packages, and plugins. Projects created with flutter create using Flutter version 2.3.0-12.0.pre or newer are already enabled to use the latest set of recommended lints. Projects created prior to that version can upgrade to it with the instructions in this guide.