proposal: `avoid_add_all_with_singleton_collection`

[natebosch]

avoid_add_all_with_singleton_collection

Description

Call add(foo) over addAll([foo]).

Details

Give a detailed description that should provide context and motivation. Ideally this could be used directly in the rule's documentation.

Kind

Style. Potentially performance with the unnecessary list allocation.

Bad Examples

collection.addAll([a]);

Good Examples

collection.add(a);

Discussion

This has around 1.5k occurrences (when checked by regex) internally. The number is likely much smaller if we only consider collection types from the SDK.

One potential downside to this lint is the editing cliff when going from 1 to multiple items, or multiple to 1 item.

The other factor that may make this not worthwhile, is whether we would apply it to the ListBuilder interface from package:built_collection. Internally I see this pattern used on ListBuilder quite a bit - and that class does not implement any interface from the SDK.

Discussion checklist

No prior discussion I can find.

• [ ] 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.

[natebosch]

@davidmorgan I am curious for your thoughts on the utility of this lint. Do you think it would be better ROI to implement this internally and target built value?

[davidmorgan]

Thanks Nate!

I looked at internal usage a bit, I noticed there are also a surprisingly large number of addAll([]), which if we follow the same reasoning, should be banned altogether.

On style I think add(foo) and addAll([foo]) are approximately even, I could prefer one or the other depending on context.

I'm hesitant about adding a lint that's supposed to improve performance, I think we'd need to do quite a bit of work to prove that it's worthwhile, e.g. fix across a real app and compare. The biggest downside to linting for performance would be that most violations are probably irrelevant for performance, you probably only care about a few violations that are on hot codepaths.

So broadly my guess is this is probably not worth pursuing, if there turn out to be large performance numbers involved I could be persuaded :)

Thanks.