linter icon indicating copy to clipboard operation
linter copied to clipboard

`prefer_const_constructors` when mutating member

Open DetachHead opened this issue 3 years ago • 4 comments

Describe the issue when instanciating a class with a const constructor, the prefer_const_constructors warning incorrectly occurs even when the instance is being mutated.

To Reproduce

class Foo {
  const Foo(this.values);
  final List<int> values;
}

void main() {
  final foo = Foo([1]); // warning: Prefer const with constant constructors.
  foo.values.add(2);
}

Expected behavior no warning, as changing it to const will cause the following runtime error:

Unsupported operation: addError: Unsupported operation: add

DetachHead avatar Aug 15 '22 11:08 DetachHead

While I agree that this is a false positive, the problem is that we can't reliably prevent it.

  • We could (as in, we don't currently but theoretically could) deduce that the field values is an immutable list when the instance of Foo was created using const, but we'd have to build in special knowledge that add throws an exception when the receiver is immutable. That would be special information that the analyzer knew because List is defined in the SDK, not something we could know about any other object's constraints. Given any other class with similar methods that can only be used when the instance isn't a const object we'd be completely unable to filter false positives.

  • Even if we could solve the problem above, we have the problem that we can't perform whole-world analysis to determine that we should not generate a lint diagnostic when the modification happens outside the library containing the constructor invocation.

  • And even if we could perform a whole-world analysis, data flow is limited and there would be plenty of places where we wouldn't be able to determine for sure whether the object being created would never be passed to a location where a modification might be made.

I think it would be better for us to not try to catch this case for the limited number of situations in which it would be possible and just document the situations where a false positive can occur.

bwilkerson avatar Oct 07 '22 16:10 bwilkerson

Could we just not lint if the arguments are not constants?

lrhn avatar Oct 07 '22 17:10 lrhn

I believe that the current criteria is that we don't lint if the arguments can't be constants, but do lint if they could be.

bwilkerson avatar Oct 07 '22 17:10 bwilkerson

This reminds me of the discussion about making const implicit for Dart 2.0. List literals were the case where there really was not good way to know if the user intended it as non-const.

So, in this case, I'd say that [] cannot be const, because then it's a different value. That's different from a const constructor invocation, where the only difference is whether you get canonicalization or not.

lrhn avatar Oct 07 '22 17:10 lrhn