proposal: `prefer_const_collection_literals`

[asashour]

prefer_const_collection_literals

PREFER using const for instantiating collection literals when possible.

Collection literals which are not assigned to a variable, are preferred to be const.

Description

PREFER using 'const' for instantiating collection literals when possible.

Details

It is preferred to use 'const' for instantiating collection literals when they are not assigned to a variable, and all the items are literals or have constant values.

Bad Examples

  for (var i in [1, 2, 3]) {
    print(i);
  }

  if (!['cat', 'dog'].contains(value)) {
    print(value);
  }

Good Examples

  for (var i in const [1, 2, 3]) {
    print(i);
  }

  if (!const ['cat', 'dog'].contains(value)) {
    print(value);
  }

Discussion

The main usage is List, but Set and Maps are also possible candidates.

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.

[srawlins]

What if it is passed as a function argument? Or given to a record literal?

What if .add or another modifying-method is called on it?

What if it is returned or yielded?

[bwilkerson]

I assume that most of those would be covered by the definition of "assigned", but the third question is the key question: how would the rule know which methods are non-modifying? I don't think we want a hard coded list, and there's no way to tell via static analysis (without a whole-world assumption that we can't make). I think this is the determining factor for how many false positives this rule would have.

[srawlins]

Why not a hard-coded list?

[bwilkerson]

A hard-coded list can't include any extension methods that users write, so we'd either allow modifying methods (false negative) or disallow non-modifying methods (false positive).

[asashour]

I guess the user extension methods would be ignored, so that would be false negative (for non-modifying user methods). Would users complain in this case? As the lint can explicitly mention, to support only the SDK methods, or specifically .contains for lists, as this probably represents the majority of cases.

Having the lint for definite known cases is better than not having it, even with the false negatives with user methods, I guess.

[srawlins]

Also, I think it would be hard/impossible to catch modifications of child elements, e.g.

void main() {
  const a = const [[1, 2, 3]];
  a[0].add(4); // runtime error
}

[asashour]

This is an assignment, which is to be ignored by the lint. Let's make it simple, the lint is reported only when the collection is immediately consumed with for in or a known non-modifying method is immediately invoked).

[srawlins]

Oh right, haha. My bad.

void main() {
  for (var e in const [[1, 2, 3]]) {
    e.add(4); // runtime error
  }
}

[lrhn]

User methods called normally on a collection literal can probably be assumed to be non-mutating. Mutating the collection only makes sense if you return it, and if you do that, you should have used a cascade instead.

Or if it can otherwise escape, which would be a method which captures the collection and returns another value containing the collection somehow. Like List.cast, only probably something more useful, and it will want to later modify the collection, otherwise it's not a problem. I can't come up with a good example, though. It's a weird pattern.