proposal: `different_default_value_in_override`

`different_default_value_in_override`

Description

Lint each optional parameter that overrides a corresponding optional parameter whose default value is different.

Details

Technically, this lint flags each instance method declaration D1 where an optional parameter declaration p1 with a default value d1 corresponds to an optional parameter declaration p2 with default value d2 in a member declaration D2 that overrides D1, and d1 is not identical to d2.

No lint message is emitted in the case where at most one of the parameter declarations has a default value (for example, there need not be a default value for an optional parameter in an abstract method declaration). An implicitly induced default value of null does count as a default value, so there will be a lint message when a default value of null is overridden by a different default value, or a non-null default value is overridden by null (whether or not those nulls are implicit).

Kind

Guard against errors.

Bad Examples

class A {
  void m([int i = 0]) {}
}

class B extends A {
  @override
  void m([int i = 123]) {} // LINT
}

abstract class C {
  void m({int i = 0, String? s, bool k = true});
}

class D extends C {
  @override
  void m({
    int i = 123, // LINT
    String? s = '', // LINT
    bool? k = false, // LINT. The type of `k` doesn't matter for the lint.
  }) {}
}

Good Examples

class A {
  void m([int i = 0]) {}
}

class B extends A {
  @override
  void m([int i = 0]) {}
}

abstract class C {
  void m({int i = 0, String? s, int k});
}

class D extends C {
  @override
  void m({
    int i = 0,
    String? s = null, // No lint, implicit/explicit null is the same.
    int k = 2, // No lint, `C.m.k` does not have a default value.
  }) {}
}

Discussion

Historically, using a different default value for an optional parameter in an overriding declaration gave rise to a warning in Dart for many years. The rationale was that it is misleading if the overridden member declares one default value (which may be statically known), but the overriding member's default value (which is actually used at run time) is different.

This warning was removed when null safety was introduced, with Dart 2.12. Several warnings have been removed from the language over time, aiming at a situation where the language only specifies errors, and warnings (as well as lints, hints, etc.) are handled uniformly by tools according to the perceived usefulness for that warning with that tool, independently of the language specification.

This is a proposal that we restore this warning as a lint, based on the error-prone nature of code where it is violated.

class A {
  void foo([int i = 1, int j = 2]) {...}
}

class B extends A {
  @override
  void foo([int i = 10, int j = 2]) {...} // LINT the first parameter declaration.
}

void main() {
  A a = B();
  a.foo(1, 175); // Passing the default value for argument 1 ... NOT.
}

If it is actually intended that two overriding declarations with a corresponding optional parameter should have different default values in the implementation then it is probably much better to avoid "overriding the default value" by a different value, and instead claim a sentinel value (typically null) for that parameter:

class C {
  void foo([String? s]) { // The default value is null.
    s ??= 'defaultValueForC';
    ...
  }
}

class D extends C {
  @override
  void foo([String? s]) { // The default value is still null.
    s ??= 'defaultValueForD';
  }
}

The point is that it is now possible to pass null as an argument to C.foo and rely on this to have the same semantics as not passing an argument at all. At the same time, the implementation specific "default value" can be computed dynamically. All in all, this is a quite flexible and useful approach. Crucially, it does not rely on actually having different default values for the given parameter, because it uses a separate sentinel value to mark that the actual argument is absent.

Of course, this will only work in the case where foo can actually claim ownership of null for the parameter type (that is, that parameter does not need to have the value null for some other reason which is unrelated to the optionality of the parameter). It should also be noted that this approach will make call sites non-nullsafe in the sense that it is now possible to pass null in a situation where this is a bug in the actual argument expression, rather than being an intentional request for the default semantics.

If these issues are difficult to handle then it may be possible to use some other sentinel value which is replaced in the method body by the implementation-specific "default value".

The point is that we don't really need to override a default value by a different default value, and this proposal argues that it's better to avoid it.

Discussion checklist

[x] List any existing rules this proposal modifies, complements, overlaps or conflicts with. There are none that I'm aware of.
[ ] List any relevant issues (reported here, the [SDK Tracker], or elsewhere).
[x] If there's any prior art (e.g., in other linters), please add references here. I mentioned the warning that we had for years.
[x] 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?) Those documents do not mention this rule (there was no need as long as it was a language specified warning).
[ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

dart-lang / sdk