linter
linter copied to clipboard
`prefer_const_constructors` when mutating member
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
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
valuesis an immutable list when the instance ofFoowas created usingconst, but we'd have to build in special knowledge thataddthrows an exception when the receiver is immutable. That would be special information that the analyzer knew becauseListis 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 aconstobject 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.
Could we just not lint if the arguments are not constants?
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.
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.